Hidden windows should not keep Chrome alive.

chrome/browser/apps/app_window_interactive_uitest.cc

Index: chrome/browser/apps/app_window_interactive_uitest.cc
diff --git a/chrome/browser/apps/app_window_interactive_uitest.cc b/chrome/browser/apps/app_window_interactive_uitest.cc
index 1212c206c288a42012b828fc3dd48eb2c6b4d06f..dd0c9803dd1fc8aacc239a1fe077e3bd75cd03ee 100644
--- a/chrome/browser/apps/app_window_interactive_uitest.cc
+++ b/chrome/browser/apps/app_window_interactive_uitest.cc
@@ -3,6 +3,8 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/apps/app_browsertest_util.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 +459,57 @@ IN_PROC_BROWSER_TEST_F(AppWindowInteractiveTest, MAYBE_TestShow) {
 IN_PROC_BROWSER_TEST_F(AppWindowInteractiveTest, TestDrawAttention) {
   ASSERT_TRUE(RunAppWindowInteractiveTest("testDrawAttention")) << message_;
 }
+
+// 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");
+  GetFirstAppWindow()->Hide();
+
+  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();
+}
+
+// An window that is created hidden and stays hidden should not keep Chrome

[benwells, 2015/02/10 07:25:10]

Nit: A window...

[jackhou1, 2015/02/10 23:33:20]

Done.

[benwells, 2015/02/12 22:55:08]

Nit: no it's not...

[jackhou1, 2015/02/15 23:00:27]

Oops, I fixed the one below and not this one. Done now.

+// 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();
+}
+
+// An window that is created hidden but shown soon after should keep Chrome
+// alive.
+IN_PROC_BROWSER_TEST_F(AppWindowHiddenKeepAliveTest, HiddenThenShown) {

[benwells, 2015/02/10 07:25:10]

Have you verified this test fails if the keepalive isn't added after a window
goes from hidden to shown?

[jackhou1, 2015/02/10 23:33:20]

In this case, the ChromeAppDelegate just avoids removing the keep-alive if the
window has never been shown. Added a new test to cover the case you suggested.

+  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());
+}