Hidden windows should not keep Chrome alive.

For each app window, there is a ChromeAppDelegate which
holds a ScopedKeepAlive. This CL resets the ScopedKeepAlive
while the app window is hidden.

BUG=450710

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

Cr-Commit-Position: refs/heads/master@{#316413}
(cherry picked from commit 39d75d5b5f11525e384745fd313d1d77ed7d0a88)

TBR=benwells@chromium.org

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

Cr-Commit-Position: refs/branch-heads/2272@{#304}
Cr-Branched-From: 827a380cfdb31aa54c8d56e63ce2c3fd8c3ba4d4-refs/heads/master@{#310958}

diff --git a/chrome/browser/apps/app_window_interactive_uitest.cc b/chrome/browser/apps/app_window_interactive_uitest.cc
index 1212c206..30247d7 100644
--- a/chrome/browser/apps/app_window_interactive_uitest.cc
+++ b/chrome/browser/apps/app_window_interactive_uitest.cc

@@ -3,6 +3,9 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/apps/app_browsertest_util.h"
+#include "chrome/browser/lifetime/application_lifetime.h"
+#include "chrome/browser/ui/browser_iterator.h"
+#include "chrome/browser/ui/browser_window.h"
 #include "chrome/test/base/interactive_test_utils.h"
 #include "extensions/browser/app_window/native_app_window.h"
 #include "extensions/test/extension_test_message_listener.h"
@@ -457,3 +460,78 @@
 IN_PROC_BROWSER_TEST_F(AppWindowInteractiveTest, TestDrawAttention) {
   ASSERT_TRUE(RunAppWindowInteractiveTest("testDrawAttention")) << message_;
 }
+
+// Only Linux and Windows use keep-alive to determine when to shut down.
+#if defined(OS_LINUX) || defined(OS_WIN)
+
+// In general, hidden windows should not keep Chrome alive. The exception is
+// when windows are created hidden, we allow the app some time to show the
+// the window.
+class AppWindowHiddenKeepAliveTest : public extensions::PlatformAppBrowserTest {
+ protected:
+  AppWindowHiddenKeepAliveTest() {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(AppWindowHiddenKeepAliveTest);
+};
+
+// A window that becomes hidden should not keep Chrome alive.
+IN_PROC_BROWSER_TEST_F(AppWindowHiddenKeepAliveTest, ShownThenHidden) {
+  LoadAndLaunchPlatformApp("minimal", "Launched");
+  for (chrome::BrowserIterator it; !it.done(); it.Next())
+    it->window()->Close();
+
+  EXPECT_TRUE(chrome::WillKeepAlive());
+  GetFirstAppWindow()->Hide();
+  EXPECT_FALSE(chrome::WillKeepAlive());
+}
+
+// A window that is hidden but re-shown should still keep Chrome alive.
+IN_PROC_BROWSER_TEST_F(AppWindowHiddenKeepAliveTest, ShownThenHiddenThenShown) {
+  LoadAndLaunchPlatformApp("minimal", "Launched");
+  AppWindow* app_window = GetFirstAppWindow();
+  app_window->Hide();
+  app_window->Show(AppWindow::SHOW_ACTIVE);
+
+  EXPECT_TRUE(chrome::WillKeepAlive());
+  for (chrome::BrowserIterator it; !it.done(); it.Next())
+    it->window()->Close();
+  EXPECT_TRUE(chrome::WillKeepAlive());
+  app_window->GetBaseWindow()->Close();
+}
+
+// A window that is created hidden and stays hidden should not keep Chrome
+// alive.
+IN_PROC_BROWSER_TEST_F(AppWindowHiddenKeepAliveTest, StaysHidden) {
+  LoadAndLaunchPlatformApp("hidden", "Launched");
+  AppWindow* app_window = GetFirstAppWindow();
+  EXPECT_TRUE(app_window->is_hidden());
+
+  for (chrome::BrowserIterator it; !it.done(); it.Next())
+    it->window()->Close();
+  // This will time out if the command above does not terminate Chrome.
+  content::RunMessageLoop();
+}
+
+// A window that is created hidden but shown soon after should keep Chrome
+// alive.
+IN_PROC_BROWSER_TEST_F(AppWindowHiddenKeepAliveTest, HiddenThenShown) {
+  ExtensionTestMessageListener launched_listener("Launched", true);
+  LoadAndLaunchPlatformApp("hidden_then_shown", &launched_listener);
+  AppWindow* app_window = GetFirstAppWindow();
+  EXPECT_TRUE(app_window->is_hidden());
+
+  // Close all browser windows.
+  for (chrome::BrowserIterator it; !it.done(); it.Next())
+    it->window()->Close();
+
+  // The app window will show after 3 seconds.
+  ExtensionTestMessageListener shown_listener("Shown", false);
+  launched_listener.Reply("");
+  EXPECT_TRUE(shown_listener.WaitUntilSatisfied());
+  EXPECT_FALSE(app_window->is_hidden());
+  EXPECT_TRUE(chrome::WillKeepAlive());
+  app_window->GetBaseWindow()->Close();
+}
+
+#endif

diff --git a/chrome/browser/ui/apps/chrome_app_delegate.cc b/chrome/browser/ui/apps/chrome_app_delegate.cc
index 4fbff22b..96ac0bf 100644
--- a/chrome/browser/ui/apps/chrome_app_delegate.cc
+++ b/chrome/browser/ui/apps/chrome_app_delegate.cc

@@ -25,6 +25,7 @@
 #include "chrome/common/extensions/chrome_extension_messages.h"
 #include "components/ui/zoom/zoom_controller.h"
 #include "content/public/browser/browser_context.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
@@ -46,6 +47,9 @@
 
 namespace {
 
+// Time to wait for an app window to show before allowing Chrome to quit.
+int kAppWindowFirstShowTimeoutSeconds = 10;
+
 bool disable_external_open_for_testing_ = false;
 
 // Opens a URL with Chromium (not external browser) with the right profile.
@@ -110,6 +114,16 @@
 
 }  // namespace
 
+// static
+void ChromeAppDelegate::RelinquishKeepAliveAfterTimeout(
+    const base::WeakPtr<ChromeAppDelegate>& chrome_app_delegate) {
+  // Resetting the ScopedKeepAlive may cause nested destruction of the
+  // ChromeAppDelegate which also resets the ScopedKeepAlive. To avoid this,
+  // move the ScopedKeepAlive out to here and let it fall out of scope.
+  if (chrome_app_delegate.get() && chrome_app_delegate->is_hidden_)
+    scoped_ptr<ScopedKeepAlive>(chrome_app_delegate->keep_alive_.Pass());
+}
+
 class ChromeAppDelegate::NewWindowContentsDelegate
     : public content::WebContentsDelegate {
  public:
@@ -150,8 +164,11 @@
 }
 
 ChromeAppDelegate::ChromeAppDelegate(scoped_ptr<ScopedKeepAlive> keep_alive)
-    : keep_alive_(keep_alive.Pass()),
-      new_window_contents_delegate_(new NewWindowContentsDelegate()) {
+    : has_been_shown_(false),
+      is_hidden_(true),
+      keep_alive_(keep_alive.Pass()),
+      new_window_contents_delegate_(new NewWindowContentsDelegate()),
+      weak_factory_(this) {
   registrar_.Add(this,
                  chrome::NOTIFICATION_APP_TERMINATING,
                  content::NotificationService::AllSources());
@@ -285,6 +302,28 @@
   terminating_callback_ = callback;
 }
 
+void ChromeAppDelegate::OnHide() {
+  is_hidden_ = true;
+  if (has_been_shown_) {
+    keep_alive_.reset();
+    return;
+  }
+
+  // Hold on to the keep alive for some time to give the app a chance to show
+  // the window.
+  content::BrowserThread::PostDelayedTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(&ChromeAppDelegate::RelinquishKeepAliveAfterTimeout,
+                 weak_factory_.GetWeakPtr()),
+      base::TimeDelta::FromSeconds(kAppWindowFirstShowTimeoutSeconds));
+}
+
+void ChromeAppDelegate::OnShow() {
+  has_been_shown_ = true;
+  is_hidden_ = false;
+  keep_alive_.reset(new ScopedKeepAlive);
+}
+
 void ChromeAppDelegate::Observe(int type,
                                 const content::NotificationSource& source,
                                 const content::NotificationDetails& details) {

diff --git a/chrome/browser/ui/apps/chrome_app_delegate.h b/chrome/browser/ui/apps/chrome_app_delegate.h
index e08e613..8501e64 100644
--- a/chrome/browser/ui/apps/chrome_app_delegate.h
+++ b/chrome/browser/ui/apps/chrome_app_delegate.h

@@ -7,6 +7,7 @@
 
 #include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "extensions/browser/app_window/app_delegate.h"
@@ -31,6 +32,9 @@
   static void DisableExternalOpenForTesting();
 
  private:
+  static void RelinquishKeepAliveAfterTimeout(
+      const base::WeakPtr<ChromeAppDelegate>& chrome_app_delegate);
+
   class NewWindowContentsDelegate;
 
   // extensions::AppDelegate:
@@ -66,16 +70,21 @@
                              bool blocked) override;
   bool IsWebContentsVisible(content::WebContents* web_contents) override;
   void SetTerminatingCallback(const base::Closure& callback) override;
+  void OnHide() override;
+  void OnShow() override;
 
   // content::NotificationObserver:
   void Observe(int type,
                const content::NotificationSource& source,
                const content::NotificationDetails& details) override;
 
+  bool has_been_shown_;
+  bool is_hidden_;
   scoped_ptr<ScopedKeepAlive> keep_alive_;
   scoped_ptr<NewWindowContentsDelegate> new_window_contents_delegate_;
   base::Closure terminating_callback_;
   content::NotificationRegistrar registrar_;
+  base::WeakPtrFactory<ChromeAppDelegate> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ChromeAppDelegate);
 };

diff --git a/chrome/test/data/extensions/platform_apps/hidden/test.js b/chrome/test/data/extensions/platform_apps/hidden/test.js
index c110227..15a1b5e7 100644
--- a/chrome/test/data/extensions/platform_apps/hidden/test.js
+++ b/chrome/test/data/extensions/platform_apps/hidden/test.js

@@ -3,7 +3,7 @@
 // found in the LICENSE file.
 
 chrome.app.runtime.onLaunched.addListener(function() {
-  chrome.app.window.create('main.html', {
+  chrome.app.window.create('empty.html', {
     hidden: true,
   }, function () {
     chrome.test.sendMessage('Launched');

diff --git a/chrome/test/data/extensions/platform_apps/hidden_then_shown/empty.html b/chrome/test/data/extensions/platform_apps/hidden_then_shown/empty.html
new file mode 100644
index 0000000..aabcd1b
--- /dev/null
+++ b/chrome/test/data/extensions/platform_apps/hidden_then_shown/empty.html

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

diff --git a/chrome/test/data/extensions/platform_apps/hidden_then_shown/manifest.json b/chrome/test/data/extensions/platform_apps/hidden_then_shown/manifest.json
new file mode 100644
index 0000000..59a8a94
--- /dev/null
+++ b/chrome/test/data/extensions/platform_apps/hidden_then_shown/manifest.json

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

diff --git a/chrome/test/data/extensions/platform_apps/hidden_then_shown/test.js b/chrome/test/data/extensions/platform_apps/hidden_then_shown/test.js
new file mode 100644
index 0000000..d1a7ff13
--- /dev/null
+++ b/chrome/test/data/extensions/platform_apps/hidden_then_shown/test.js

@@ -0,0 +1,16 @@
+// 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.
+
+chrome.app.runtime.onLaunched.addListener(function() {
+  chrome.app.window.create('empty.html', {
+    hidden: true,
+  }, function (win) {
+    chrome.test.sendMessage('Launched', function () {
+      setTimeout(function () {
+        win.show();
+        chrome.test.sendMessage('Shown');
+      }, 3000);
+    });
+  });
+});

diff --git a/extensions/browser/app_window/app_delegate.h b/extensions/browser/app_window/app_delegate.h
index 0e4477e..fd78d106 100644
--- a/extensions/browser/app_window/app_delegate.h
+++ b/extensions/browser/app_window/app_delegate.h

@@ -76,6 +76,10 @@
 
   // |callback| will be called when the process is about to terminate.
   virtual void SetTerminatingCallback(const base::Closure& callback) = 0;
+
+  // Called when the app is hidden or shown.
+  virtual void OnHide() = 0;
+  virtual void OnShow() = 0;
 };
 
 }  // namespace extensions

diff --git a/extensions/browser/app_window/app_window.cc b/extensions/browser/app_window/app_window.cc
index 7fdaeffb..83bd201 100644
--- a/extensions/browser/app_window/app_window.cc
+++ b/extensions/browser/app_window/app_window.cc

@@ -658,6 +658,7 @@
 }
 
 void AppWindow::Show(ShowType show_type) {
+  app_delegate_->OnShow();
   bool was_hidden = is_hidden_ || !has_been_shown_;
   is_hidden_ = false;
 
@@ -694,6 +695,7 @@
   show_on_first_paint_ = false;
   GetBaseWindow()->Hide();
   AppWindowRegistry::Get(browser_context_)->AppWindowHidden(this);
+  app_delegate_->OnHide();
 }
 
 void AppWindow::SetAlwaysOnTop(bool always_on_top) {

diff --git a/extensions/shell/browser/shell_app_delegate.h b/extensions/shell/browser/shell_app_delegate.h
index 8c9be2e..b458155 100644
--- a/extensions/shell/browser/shell_app_delegate.h
+++ b/extensions/shell/browser/shell_app_delegate.h

@@ -50,6 +50,8 @@
                              bool blocked) override;
   bool IsWebContentsVisible(content::WebContents* web_contents) override;
   void SetTerminatingCallback(const base::Closure& callback) override;
+  void OnHide() override {}
+  void OnShow() override {}
 
   // content::WebContentsObserver:
   void RenderViewCreated(content::RenderViewHost* render_view_host) override;