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

13 files changed, 185 insertions, 58 deletions

diff --git a/apps/app_lifetime_monitor.cc b/apps/app_lifetime_monitor.cc
index 83a3c56..0d258c6 100644
--- a/apps/app_lifetime_monitor.cc
+++ b/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));
 }
diff --git a/apps/app_lifetime_monitor.h b/apps/app_lifetime_monitor.h
index a9fed9c..9e4aa56 100644
--- a/apps/app_lifetime_monitor.h
+++ b/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);
diff --git a/apps/app_load_service.cc b/apps/app_load_service.cc
index cfcef3e..8d816f5 100644
--- a/apps/app_load_service.cc
+++ b/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) {
diff --git a/apps/app_load_service.h b/apps/app_load_service.h
index 2ccd3e0..c79915d 100644
--- a/apps/app_load_service.h
+++ b/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);
diff --git a/apps/app_restore_service_browsertest.cc b/apps/app_restore_service_browsertest.cc
index 833cee5..1ba87e3 100644
--- a/apps/app_restore_service_browsertest.cc
+++ b/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,
diff --git a/apps/app_window.cc b/apps/app_window.cc
index a48f79d..2071892 100644
--- a/apps/app_window.cc
+++ b/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);
diff --git a/apps/app_window.h b/apps/app_window.h
index 3fd5757..9b922c8 100644
--- a/apps/app_window.h
+++ b/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_;
 
diff --git a/chrome/browser/ui/cocoa/apps/native_app_window_cocoa.h b/chrome/browser/ui/cocoa/apps/native_app_window_cocoa.h
index 716c573..e7b6c05 100644
--- a/chrome/browser/ui/cocoa/apps/native_app_window_cocoa.h
+++ b/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_;
diff --git a/chrome/browser/ui/cocoa/apps/native_app_window_cocoa.mm b/chrome/browser/ui/cocoa/apps/native_app_window_cocoa.mm
index ecd390e..3af4ff8 100644
--- a/chrome/browser/ui/cocoa/apps/native_app_window_cocoa.mm
+++ b/chrome/browser/ui/cocoa/apps/native_app_window_cocoa.mm
@@ -298,7 +298,6 @@ NativeAppWindowCocoa::NativeAppWindowCocoa(
     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 @@ gfx::Rect NativeAppWindowCocoa::GetBounds() const {
 }
 
 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 NativeAppWindowCocoa::Show() {
 }
 
 void NativeAppWindowCocoa::ShowInactive() {
-  is_hidden_ = false;
   [window() orderFront:window_controller_];
 }
 
 void NativeAppWindowCocoa::Hide() {
-  is_hidden_ = true;
   HideWithoutMarkingHidden();
 }
 
@@ -868,7 +863,7 @@ bool NativeAppWindowCocoa::HandledByExtensionCommand(NSEvent* event) {
 
 void NativeAppWindowCocoa::ShowWithApp() {
   is_hidden_with_app_ = false;
-  if (!is_hidden_)
+  if (!app_window_->is_hidden())
     ShowInactive();
 }
 
diff --git a/chrome/browser/ui/cocoa/apps/native_app_window_cocoa_browsertest.mm b/chrome/browser/ui/cocoa/apps/native_app_window_cocoa_browsertest.mm
index 1976806..519ac53 100644
--- a/chrome/browser/ui/cocoa/apps/native_app_window_cocoa_browsertest.mm
+++ b/chrome/browser/ui/cocoa/apps/native_app_window_cocoa_browsertest.mm
@@ -52,58 +52,63 @@ IN_PROC_BROWSER_TEST_F(NativeAppWindowCocoaBrowserTest, HideShowWithApp) {
   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]);
 }
diff --git a/chrome/test/data/extensions/platform_apps/active_test/empty.html b/chrome/test/data/extensions/platform_apps/active_test/empty.html
new file mode 100644
index 0000000..aabcd1b
--- /dev/null
+++ b/chrome/test/data/extensions/platform_apps/active_test/empty.html
@@ -0,0 +1 @@
+<!-- This file intentionally left blank. -->
diff --git a/chrome/test/data/extensions/platform_apps/active_test/manifest.json b/chrome/test/data/extensions/platform_apps/active_test/manifest.json
new file mode 100644
index 0000000..1b49987
--- /dev/null
+++ b/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"]
+    }
+  }
+}
diff --git a/chrome/test/data/extensions/platform_apps/active_test/test.js b/chrome/test/data/extensions/platform_apps/active_test/test.js
new file mode 100644
index 0000000..984c497
--- /dev/null
+++ b/chrome/test/data/extensions/platform_apps/active_test/test.js
@@ -0,0 +1,43 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// This test gets sent commands to execute, which it is sent by the
+// controlling C++ code. This code then checks that the apps' active state
+// is being tracked correctly.
+var windows = [];
+
+function windowClosed() {
+  processNextCommand();
+}
+
+function processNextCommand() {
+  chrome.test.sendMessage("ready", function(response) {
+    if (response == 'exit')
+      return;
+
+    if (response == 'closeLastWindow') {
+      windowToClose = windows.pop();
+      windowToClose.close();
+      return;
+    }
+
+    // Otherwise we are creating a window.
+    createOptions = {};
+
+    if (response == 'createMinimized')
+      createOptions.state = 'minimized';
+
+    if (response == 'createHidden')
+      createOptions.hidden = true;
+
+    chrome.app.window.create('empty.html', createOptions,
+        function(createdWindow) {
+      createdWindow.onClosed.addListener(windowClosed);
+      windows.push(createdWindow);
+      processNextCommand();
+    });
+  });
+}
+
+processNextCommand();
\ No newline at end of file