Don't send onLaunched to apps with only hidden windows when they reload.

This also stops apps with only hidden windows getting sent the
onLaunched if they don't listen to onRestarted and the system is
restarted.

BUG=268755

Review URL: https://codereview.chromium.org/270273002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@270614 0039d316-1c4b-4281-b951-d872f2087c98

apps/app_lifetime_monitor.cc

@@ -77,25 +77,24 @@ void AppLifetimeMonitor::Observe(int type,
   }
 }
 
-void AppLifetimeMonitor::OnAppWindowAdded(AppWindow* app_window) {
+void AppLifetimeMonitor::OnAppWindowRemoved(AppWindow* app_window) {
+  if (!HasVisibleAppWindows(app_window))
+    NotifyAppDeactivated(app_window->extension_id());
+}
+
+void AppLifetimeMonitor::OnAppWindowHidden(AppWindow* app_window) {
+  if (!HasVisibleAppWindows(app_window))
+    NotifyAppDeactivated(app_window->extension_id());
+}
+
+void AppLifetimeMonitor::OnAppWindowShown(AppWindow* app_window) {
   if (app_window->window_type() != AppWindow::WINDOW_TYPE_DEFAULT)
     return;
 
-  AppWindowRegistry::AppWindowList windows =
-      AppWindowRegistry::Get(app_window->browser_context())
-          ->GetAppWindowsForApp(app_window->extension_id());
-  if (windows.size() == 1)
+  if (HasVisibleAppWindows(app_window))
     NotifyAppActivated(app_window->extension_id());
 }
 
-void AppLifetimeMonitor::OnAppWindowRemoved(AppWindow* app_window) {
-  AppWindowRegistry::AppWindowList windows =
-      AppWindowRegistry::Get(app_window->browser_context())
-          ->GetAppWindowsForApp(app_window->extension_id());
-  if (windows.empty())
-    NotifyAppDeactivated(app_window->extension_id());
-}
-
 void AppLifetimeMonitor::Shutdown() {
   AppWindowRegistry* app_window_registry =
       AppWindowRegistry::Factory::GetForBrowserContext(profile_,
@@ -104,6 +103,20 @@ void AppLifetimeMonitor::Shutdown() {
     app_window_registry->RemoveObserver(this);
 }
 
+bool AppLifetimeMonitor::HasVisibleAppWindows(AppWindow* app_window) const {
+  AppWindowRegistry::AppWindowList windows =
+      AppWindowRegistry::Get(app_window->browser_context())
+          ->GetAppWindowsForApp(app_window->extension_id());
+
+  for (AppWindowRegistry::AppWindowList::const_iterator i = windows.begin();
+       i != windows.end();
+       ++i) {
+    if (!(*i)->is_hidden())
+      return true;
+  }
+  return false;
+}
+
 void AppLifetimeMonitor::NotifyAppStart(const std::string& app_id) {
   FOR_EACH_OBSERVER(Observer, observers_, OnAppStart(profile_, app_id));
 }

apps/app_lifetime_monitor.h

@@ -62,12 +62,15 @@ class AppLifetimeMonitor : public KeyedService,
                        const content::NotificationDetails& details) OVERRIDE;
 
   // AppWindowRegistry::Observer overrides:
-  virtual void OnAppWindowAdded(AppWindow* app_window) OVERRIDE;
   virtual void OnAppWindowRemoved(AppWindow* app_window) OVERRIDE;
+  virtual void OnAppWindowHidden(apps::AppWindow* app_window) OVERRIDE;
+  virtual void OnAppWindowShown(apps::AppWindow* app_window) OVERRIDE;
 
   // KeyedService overrides:
   virtual void Shutdown() OVERRIDE;
 
+  bool HasVisibleAppWindows(apps::AppWindow* app_window) const;
+
   void NotifyAppStart(const std::string& app_id);
   void NotifyAppActivated(const std::string& app_id);
   void NotifyAppDeactivated(const std::string& app_id);

apps/app_load_service.cc

@@ -115,8 +115,10 @@ void AppLoadService::Observe(int type,
       if (!unload_info->extension->is_platform_app())
         break;
 
+      extensions::ExtensionPrefs* extension_prefs =
+          extensions::ExtensionPrefs::Get(profile_);
       if (WasUnloadedForReload(*unload_info) &&
-          HasAppWindows(unload_info->extension->id()) &&
+          extension_prefs->IsActive(unload_info->extension->id()) &&
           !HasPostReloadAction(unload_info->extension->id())) {
         post_reload_actions_[unload_info->extension->id()].action_type = LAUNCH;
       }
@@ -127,12 +129,6 @@ void AppLoadService::Observe(int type,
   }
 }
 
-bool AppLoadService::HasAppWindows(const std::string& extension_id) {
-  return !AppWindowRegistry::Get(profile_)
-              ->GetAppWindowsForApp(extension_id)
-              .empty();
-}
-
 bool AppLoadService::WasUnloadedForReload(
     const extensions::UnloadedExtensionInfo& unload_info) {
   if (unload_info.reason == extensions::UnloadedExtensionInfo::REASON_DISABLE) {

apps/app_load_service.h

@@ -64,7 +64,6 @@ class AppLoadService : public KeyedService,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
-  bool HasAppWindows(const std::string& extension_id);
   bool WasUnloadedForReload(
       const extensions::UnloadedExtensionInfo& unload_info);
   bool HasPostReloadAction(const std::string& extension_id);

apps/app_restore_service_browsertest.cc

@@ -55,6 +55,60 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, RunningAppsAreRecorded) {
   restart_listener.WaitUntilSatisfied();
 }
 
+// Tests that apps are recorded in the preferences as active when and only when
+// they have visible windows.
+IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, ActiveAppsAreRecorded) {
+  ExtensionTestMessageListener ready_listener("ready", true);
+  const Extension* extension =
+      LoadExtension(test_data_dir_.AppendASCII("platform_apps/active_test"));
+  ASSERT_TRUE(extension);
+  ExtensionPrefs* extension_prefs = ExtensionPrefs::Get(browser()->profile());
+  ASSERT_TRUE(ready_listener.WaitUntilSatisfied());
+
+  // Open a visible window and check the app is marked active.
+  ready_listener.Reply("create");
+  ready_listener.Reset();
+  ASSERT_TRUE(ready_listener.WaitUntilSatisfied());
+  ASSERT_TRUE(extension_prefs->IsActive(extension->id()));
+
+  // Close the window, then open a minimized window and check the app is active.
+  ready_listener.Reply("closeLastWindow");
+  ready_listener.Reset();
+  ASSERT_TRUE(ready_listener.WaitUntilSatisfied());
+  ready_listener.Reply("createMinimized");
+  ready_listener.Reset();
+  ASSERT_TRUE(ready_listener.WaitUntilSatisfied());
+  ASSERT_TRUE(extension_prefs->IsActive(extension->id()));
+
+  // Close the window, then open a hidden window and check the app is not
+  // marked active.
+  ready_listener.Reply("closeLastWindow");
+  ready_listener.Reset();
+  ASSERT_TRUE(ready_listener.WaitUntilSatisfied());
+  ready_listener.Reply("createHidden");
+  ready_listener.Reset();
+  ASSERT_TRUE(ready_listener.WaitUntilSatisfied());
+  ASSERT_FALSE(extension_prefs->IsActive(extension->id()));
+
+  // Open another window and check the app is marked active.
+  ready_listener.Reply("create");
+  ready_listener.Reset();
+  ASSERT_TRUE(ready_listener.WaitUntilSatisfied());
+  ASSERT_TRUE(extension_prefs->IsActive(extension->id()));
+
+  // Close the visible window and check the app has been marked inactive.
+  ready_listener.Reply("closeLastWindow");
+  ready_listener.Reset();
+  ASSERT_TRUE(ready_listener.WaitUntilSatisfied());
+  ASSERT_FALSE(extension_prefs->IsActive(extension->id()));
+
+  // Close the last window and exit.
+  ready_listener.Reply("closeLastWindow");
+  ready_listener.Reset();
+  ASSERT_TRUE(ready_listener.WaitUntilSatisfied());
+  ready_listener.Reply("exit");
+}
+
 IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, FileAccessIsSavedToPrefs) {
   content::WindowedNotificationObserver extension_suspended(
       chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED,

apps/app_window.cc

@@ -238,6 +238,7 @@ AppWindow::AppWindow(BrowserContext* context,
       fullscreen_types_(FULLSCREEN_TYPE_NONE),
       show_on_first_paint_(false),
       first_paint_complete_(false),
+      is_hidden_(false),
       cached_always_on_top_(false) {
   extensions::ExtensionsBrowserClient* client =
       extensions::ExtensionsBrowserClient::Get();
@@ -279,6 +280,11 @@ void AppWindow::Init(const GURL& url,
 
   native_app_window_.reset(delegate_->CreateNativeAppWindow(this, new_params));
 
+  // Prevent the browser process from shutting down while this window exists.
+  AppsClient::Get()->IncrementKeepAliveCount();
+  UpdateExtensionAppIcon();
+  AppWindowRegistry::Get(browser_context_)->AddAppWindow(this);
+
   if (new_params.hidden) {
     // Although the window starts hidden by default, calling Hide() here
     // notifies observers of the window being hidden.
@@ -330,13 +336,6 @@ void AppWindow::Init(const GURL& url,
     initial_bounds.Inset(frame_insets);
     apps::ResizeWebContents(web_contents, initial_bounds.size());
   }
-
-  // Prevent the browser process from shutting down while this window is open.
-  AppsClient::Get()->IncrementKeepAliveCount();
-
-  UpdateExtensionAppIcon();
-
-  AppWindowRegistry::Get(browser_context_)->AddAppWindow(this);
 }
 
 AppWindow::~AppWindow() {
@@ -678,6 +677,8 @@ void AppWindow::SetContentSizeConstraints(const gfx::Size& min_size,
 }
 
 void AppWindow::Show(ShowType show_type) {
+  is_hidden_ = false;
+
   if (CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableAppsShowOnFirstPaint)) {
     show_on_first_paint_ = true;
@@ -704,6 +705,7 @@ void AppWindow::Hide() {
   // there was a non-empty paint. It should have no effect in a non-racy
   // scenario where the application is hiding then showing a window: the second
   // show will not be delayed.
+  is_hidden_ = true;
   show_on_first_paint_ = false;
   GetBaseWindow()->Hide();
   AppWindowRegistry::Get(browser_context_)->AppWindowHidden(this);

apps/app_window.h

@@ -262,6 +262,7 @@ class AppWindow : public content::NotificationObserver,
   const GURL& app_icon_url() const { return app_icon_url_; }
   const gfx::Image& badge_icon() const { return badge_icon_; }
   const GURL& badge_icon_url() const { return badge_icon_url_; }
+  bool is_hidden() const { return is_hidden_; }
 
   const extensions::Extension* GetExtension() const;
   NativeAppWindow* GetBaseWindow();
@@ -518,6 +519,14 @@ class AppWindow : public content::NotificationObserver,
   // The first visually non-empty paint has completed.
   bool first_paint_complete_;
 
+  // Whether the window is hidden or not. Hidden in this context means actively
+  // by the chrome.app.window API, not in an operating system context. For
+  // example windows which are minimized are not hidden, and windows which are
+  // part of a hidden app on OS X are not hidden. Windows which were created
+  // with the |hidden| flag set to true, or which have been programmatically
+  // hidden, are considered hidden.
+  bool is_hidden_;
+
   // Whether the delayed Show() call was for an active or inactive window.
   ShowType delayed_show_type_;
 

chrome/browser/ui/cocoa/apps/native_app_window_cocoa.h

@@ -181,9 +181,6 @@ class NativeAppWindowCocoa : public apps::NativeAppWindow,
 
   bool has_frame_;
 
-  // Whether this window is hidden according to the app.window API. This is set
-  // by Hide, Show, and ShowInactive.
-  bool is_hidden_;
   // Whether this window last became hidden due to a request to hide the entire
   // app, e.g. via the dock menu or Cmd+H. This is set by Hide/ShowWithApp.
   bool is_hidden_with_app_;

chrome/browser/ui/cocoa/apps/native_app_window_cocoa.mm

@@ -298,7 +298,6 @@ - (void)setMouseDownCanMoveWindow:(BOOL)can_move;
     const AppWindow::CreateParams& params)
     : app_window_(app_window),
       has_frame_(params.frame == AppWindow::FRAME_CHROME),
-      is_hidden_(false),
       is_hidden_with_app_(false),
       is_maximized_(false),
       is_fullscreen_(false),
@@ -535,8 +534,6 @@ - (void)setMouseDownCanMoveWindow:(BOOL)can_move;
 }
 
 void NativeAppWindowCocoa::Show() {
-  is_hidden_ = false;
-
   if (is_hidden_with_app_) {
     // If there is a shim to gently request attention, return here. Otherwise
     // show the window as usual.
@@ -551,12 +548,10 @@ - (void)setMouseDownCanMoveWindow:(BOOL)can_move;
 }
 
 void NativeAppWindowCocoa::ShowInactive() {
-  is_hidden_ = false;
   [window() orderFront:window_controller_];
 }
 
 void NativeAppWindowCocoa::Hide() {
-  is_hidden_ = true;
   HideWithoutMarkingHidden();
 }
 
@@ -868,7 +863,7 @@ - (void)setMouseDownCanMoveWindow:(BOOL)can_move;
 
 void NativeAppWindowCocoa::ShowWithApp() {
   is_hidden_with_app_ = false;
-  if (!is_hidden_)
+  if (!app_window_->is_hidden())
     ShowInactive();
 }
 

chrome/browser/ui/cocoa/apps/native_app_window_cocoa_browsertest.mm

@@ -52,58 +52,63 @@ void SetUpAppWithWindows(int num_windows) {
   SetUpAppWithWindows(2);
   apps::AppWindowRegistry::AppWindowList windows =
       apps::AppWindowRegistry::Get(profile())->app_windows();
-  apps::NativeAppWindow* window = windows.front()->GetBaseWindow();
-  NSWindow* ns_window = window->GetNativeWindow();
-  apps::NativeAppWindow* other_window = windows.back()->GetBaseWindow();
-  NSWindow* other_ns_window = other_window->GetNativeWindow();
+
+  apps::AppWindow* app_window = windows.front();
+  apps::NativeAppWindow* native_window = app_window->GetBaseWindow();
+  NSWindow* ns_window = native_window->GetNativeWindow();
+
+  apps::AppWindow* other_app_window = windows.back();
+  apps::NativeAppWindow* other_native_window =
+      other_app_window->GetBaseWindow();
+  NSWindow* other_ns_window = other_native_window->GetNativeWindow();
 
   // Normal Hide/Show.
-  window->Hide();
+  app_window->Hide();
   EXPECT_FALSE([ns_window isVisible]);
-  window->Show();
+  app_window->Show(apps::AppWindow::SHOW_ACTIVE);
   EXPECT_TRUE([ns_window isVisible]);
 
   // Normal Hide/ShowWithApp.
-  window->HideWithApp();
+  native_window->HideWithApp();
   EXPECT_FALSE([ns_window isVisible]);
-  window->ShowWithApp();
+  native_window->ShowWithApp();
   EXPECT_TRUE([ns_window isVisible]);
 
   // HideWithApp, Hide, ShowWithApp does not show.
-  window->HideWithApp();
-  window->Hide();
-  window->ShowWithApp();
+  native_window->HideWithApp();
+  app_window->Hide();
+  native_window->ShowWithApp();
   EXPECT_FALSE([ns_window isVisible]);
 
   // Hide, HideWithApp, ShowWithApp does not show.
-  window->HideWithApp();
-  window->ShowWithApp();
+  native_window->HideWithApp();
+  native_window->ShowWithApp();
   EXPECT_FALSE([ns_window isVisible]);
 
   // Return to shown state.
-  window->Show();
+  app_window->Show(apps::AppWindow::SHOW_ACTIVE);
   EXPECT_TRUE([ns_window isVisible]);
 
   // HideWithApp the other window.
   EXPECT_TRUE([other_ns_window isVisible]);
-  other_window->HideWithApp();
+  other_native_window->HideWithApp();
   EXPECT_FALSE([other_ns_window isVisible]);
 
   // HideWithApp, Show shows all windows for this app.
-  window->HideWithApp();
+  native_window->HideWithApp();
   EXPECT_FALSE([ns_window isVisible]);
-  window->Show();
+  app_window->Show(apps::AppWindow::SHOW_ACTIVE);
   EXPECT_TRUE([ns_window isVisible]);
   EXPECT_TRUE([other_ns_window isVisible]);
 
   // Hide the other window.
-  other_window->Hide();
+  other_app_window->Hide();
   EXPECT_FALSE([other_ns_window isVisible]);
 
   // HideWithApp, ShowWithApp does not show the other window.
-  window->HideWithApp();
+  native_window->HideWithApp();
   EXPECT_FALSE([ns_window isVisible]);
-  window->ShowWithApp();
+  native_window->ShowWithApp();
   EXPECT_TRUE([ns_window isVisible]);
   EXPECT_FALSE([other_ns_window isVisible]);
 }

chrome/test/data/extensions/platform_apps/active_test/empty.html

@@ -0,0 +1 @@
+<!-- This file intentionally left blank. -->

chrome/test/data/extensions/platform_apps/active_test/manifest.json

@@ -0,0 +1,10 @@
+{
+  "name": "Platform App Active Test",
+  "version": "1",
+  "manifest_version": 2,
+  "app": {
+    "background": {
+      "scripts": ["test.js"]
+    }
+  }
+}