Revert r165772: Keep browser process alive while there are platform apps with background pages runn…

chrome/browser/extensions/platform_app_browsertest.cc

Index: chrome/browser/extensions/platform_app_browsertest.cc
===================================================================
--- chrome/browser/extensions/platform_app_browsertest.cc	(revision 166679)
+++ chrome/browser/extensions/platform_app_browsertest.cc	(working copy)
@@ -10,7 +10,6 @@
 #include "chrome/browser/automation/automation_util.h"
 #include "chrome/browser/debugger/devtools_window.h"
 #include "chrome/browser/extensions/api/permissions/permissions_api.h"
-#include "chrome/browser/lifetime/application_lifetime.h"

[davidjames, 2012/11/08 17:38:48]

There was a trivial conflict here in the context of the patch, which was easy to
resolve (I just manually applied the same change, deleting this line.)

 #include "chrome/browser/extensions/app_restore_service_factory.h"
 #include "chrome/browser/extensions/app_restore_service.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
@@ -23,7 +22,6 @@
 #include "chrome/browser/extensions/shell_window_registry.h"
 #include "chrome/browser/tab_contents/render_view_context_menu.h"
 #include "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/constrained_window_tab_helper.h"
 #include "chrome/browser/ui/extensions/application_launch.h"
@@ -147,8 +145,6 @@
 
 }  // namespace
 
-// TODO(benwells): Break up this file into some sensible smaller files.
-
 // Tests that CreateShellWindow doesn't crash if you close it straight away.
 // LauncherPlatformAppBrowserTest relies on this behaviour, but is only run for
 // ash, so we test that it works here.
@@ -156,7 +152,6 @@
   const Extension* extension = LoadAndLaunchPlatformApp("minimal");
   ShellWindow* window = CreateShellWindow(extension);
   CloseShellWindow(window);
-  CloseShellWindowsAndWaitForAppToExit();
 }
 
 // Tests that platform apps can be launched in incognito mode without crashing.
@@ -181,18 +176,8 @@
           NEW_WINDOW));
 
   ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
-  CloseShellWindowsAndWaitForAppToExit();
 }
 
-// Tests that the browser process is kept alive by the platform app's background
-// page.
-IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, AppEventPageKeepsBrowserAlive) {
-  LoadAndLaunchPlatformApp("minimal");
-  browser()->window()->Close();
-  ASSERT_TRUE(browser::WillKeepAlive());
-  CloseShellWindowsAndWaitForAppToExit();
-}
-
 // Tests that platform apps received the "launch" event when launched.
 IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, OnLaunchedEvent) {
   ASSERT_TRUE(RunPlatformAppTest("platform_apps/launch")) << message_;
@@ -225,7 +210,6 @@
       web_contents);
 
   ASSERT_TRUE(handler.WaitUntilReply());
-  CloseShellWindowsAndWaitForAppToExit();
 }
 
 // Tests that platform apps cannot use certain disabled window properties, but
@@ -236,8 +220,11 @@
 }
 
 IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, EmptyContextMenu) {
+  ExtensionTestMessageListener launched_listener("Launched", false);
   LoadAndLaunchPlatformApp("minimal");
 
+  ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
+
   // The empty app doesn't add any context menu items, so its menu should
   // only include the developer tools.
   WebContents* web_contents = GetFirstShellWindowWebContents();
@@ -253,12 +240,16 @@
   ASSERT_TRUE(menu->HasCommandWithId(IDC_CONTENT_CONTEXT_RELOAD_PACKAGED_APP));
   ASSERT_FALSE(menu->HasCommandWithId(IDC_BACK));
   ASSERT_FALSE(menu->HasCommandWithId(IDC_SAVE_PAGE));
-  CloseShellWindowsAndWaitForAppToExit();
 }
 
 IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, AppWithContextMenu) {
+  ExtensionTestMessageListener launched_listener("Launched", false);
   LoadAndLaunchPlatformApp("context_menu");
 
+  // Wait for the extension to tell us it's initialized its context menus and
+  // launched a window.
+  ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
+
   // The context_menu app has two context menu items. These, along with a
   // separator and the developer tools, is all that should be in the menu.
   WebContents* web_contents = GetFirstShellWindowWebContents();
@@ -277,12 +268,16 @@
   ASSERT_FALSE(menu->HasCommandWithId(IDC_BACK));
   ASSERT_FALSE(menu->HasCommandWithId(IDC_SAVE_PAGE));
   ASSERT_FALSE(menu->HasCommandWithId(IDC_CONTENT_CONTEXT_UNDO));
-  CloseShellWindowsAndWaitForAppToExit();
 }
 
 IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, InstalledAppWithContextMenu) {
+  ExtensionTestMessageListener launched_listener("Launched", false);
   InstallAndLaunchPlatformApp("context_menu");
 
+  // Wait for the extension to tell us it's initialized its context menus and
+  // launched a window.
+  ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
+
   // The context_menu app has two context menu items. For an installed app
   // these are all that should be in the menu.
   WebContents* web_contents = GetFirstShellWindowWebContents();
@@ -301,12 +296,16 @@
   ASSERT_FALSE(menu->HasCommandWithId(IDC_BACK));
   ASSERT_FALSE(menu->HasCommandWithId(IDC_SAVE_PAGE));
   ASSERT_FALSE(menu->HasCommandWithId(IDC_CONTENT_CONTEXT_UNDO));
-  CloseShellWindowsAndWaitForAppToExit();
 }
 
 IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, AppWithContextMenuTextField) {
+  ExtensionTestMessageListener launched_listener("Launched", false);
   LoadAndLaunchPlatformApp("context_menu");
 
+  // Wait for the extension to tell us it's initialized its context menus and
+  // launched a window.
+  ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
+
   // The context_menu app has one context menu item. This, along with a
   // separator and the developer tools, is all that should be in the menu.
   WebContents* web_contents = GetFirstShellWindowWebContents();
@@ -326,12 +325,16 @@
   ASSERT_TRUE(menu->HasCommandWithId(IDC_CONTENT_CONTEXT_COPY));
   ASSERT_FALSE(menu->HasCommandWithId(IDC_BACK));
   ASSERT_FALSE(menu->HasCommandWithId(IDC_SAVE_PAGE));
-  CloseShellWindowsAndWaitForAppToExit();
 }
 
 IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, AppWithContextMenuSelection) {
+  ExtensionTestMessageListener launched_listener("Launched", false);
   LoadAndLaunchPlatformApp("context_menu");
 
+  // Wait for the extension to tell us it's initialized its context menus and
+  // launched a window.
+  ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
+
   // The context_menu app has one context menu item. This, along with a
   // separator and the developer tools, is all that should be in the menu.
   WebContents* web_contents = GetFirstShellWindowWebContents();
@@ -351,12 +354,16 @@
   ASSERT_TRUE(menu->HasCommandWithId(IDC_CONTENT_CONTEXT_COPY));
   ASSERT_FALSE(menu->HasCommandWithId(IDC_BACK));
   ASSERT_FALSE(menu->HasCommandWithId(IDC_SAVE_PAGE));
-  CloseShellWindowsAndWaitForAppToExit();
 }
 
 IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, AppWithContextMenuClicked) {
+  ExtensionTestMessageListener launched_listener("Launched", false);
   LoadAndLaunchPlatformApp("context_menu_click");
 
+  // Wait for the extension to tell us it's initialized its context menus and
+  // launched a window.
+  ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
+
   // Test that the menu item shows up
   WebContents* web_contents = GetFirstShellWindowWebContents();
   ASSERT_TRUE(web_contents);
@@ -374,15 +381,13 @@
   menu->ExecuteCommand(IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST);
 
   ASSERT_TRUE(onclicked_listener.WaitUntilSatisfied());
-  CloseShellWindowsAndWaitForAppToExit();
 }
 
 IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, DisallowNavigation) {
   TabsAddedNotificationObserver observer(2);
 
   ASSERT_TRUE(StartTestServer());
-  ASSERT_TRUE(RunPlatformAppTestReturnImmediately("platform_apps/navigation"))
-      << message_;
+  ASSERT_TRUE(RunPlatformAppTest("platform_apps/navigation")) << message_;
 
   observer.Wait();
   ASSERT_EQ(2U, observer.tabs().size());
@@ -390,8 +395,6 @@
             observer.tabs()[0]->GetURL().spec());
   EXPECT_EQ("http://chromium.org/",
             observer.tabs()[1]->GetURL().spec());
-  browser()->window()->Close();
-  CloseShellWindowsAndWaitForAppToExit();
 }
 
 IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, Iframes) {
@@ -461,7 +464,9 @@
   ASSERT_EQ(0U, GetShellWindowCount());
 
   // Launch a platform app that shows a window.
+  ExtensionTestMessageListener launched_listener("Launched", false);
   LoadAndLaunchPlatformApp("minimal");
+  ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
   ASSERT_EQ(1U, GetShellWindowCount());
   ShellWindowRegistry::ShellWindowSet shell_windows =
       ShellWindowRegistry::Get(browser()->profile())->shell_windows();
@@ -478,12 +483,13 @@
   // to get a list of all the shell windows, so we can test this.
 
   // Launch another platform app that also shows a window.
+  ExtensionTestMessageListener launched_listener2("Launched", false);
   LoadAndLaunchPlatformApp("context_menu");
+  ASSERT_TRUE(launched_listener2.WaitUntilSatisfied());
 
   // There are two total shell windows, but each app can only see its own.
   ASSERT_EQ(2U, GetShellWindowCount());
   // TODO(jeremya): as above, this requires more extension functions.
-  CloseShellWindowsAndWaitForAppToExit();
 }
 
 // ChromeOS does not support passing arguments on the command line, so the tests
@@ -527,7 +533,6 @@
     message_ = catcher.message();
     ASSERT_TRUE(0);
   }
-  CloseShellWindowsAndWaitForAppToExit();
 }
 
 // Tests that no launch data is sent through if the file is of the wrong MIME
@@ -594,8 +599,6 @@
   LoadAndLaunchPlatformApp("open_link");
   observer.Wait();
   ASSERT_EQ(2, browser()->tab_count());
-  browser()->window()->Close();
-  CloseShellWindowsAndWaitForAppToExit();
 }
 
 IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, MutationEventsDisabled) {
@@ -670,7 +673,6 @@
   // Wait for javascript to verify that the third window got the restored size
   // and explicitly specified coordinates.
   ASSERT_TRUE(done3_listener.WaitUntilSatisfied());
-  CloseShellWindowsAndWaitForAppToExit();
 }
 
 // Tests that a running app is recorded in the preferences as such.
@@ -702,7 +704,6 @@
   AppRestoreServiceFactory::GetForProfile(browser()->profile())->
       HandleStartup(true);
   restart_listener.WaitUntilSatisfied();
-  CloseShellWindowsAndWaitForAppToExit();
 }
 
 namespace {
@@ -720,8 +721,10 @@
 void PlatformAppDevToolsBrowserTest::RunTestWithDevTools(
     const char* name, int test_flags) {
   using content::DevToolsAgentHostRegistry;
+  ExtensionTestMessageListener launched_listener("Launched", false);

[davidjames, 2012/11/08 17:38:48]

I added this line because we need to wait for the application to launch before
looking for the shell window.

   const Extension* extension = LoadAndLaunchPlatformApp(name);
   ASSERT_TRUE(extension);
+  ASSERT_TRUE(launched_listener.WaitUntilSatisfied());

[davidjames, 2012/11/08 17:38:48]

Same here.

   ShellWindow* window = GetFirstShellWindow();
   ASSERT_TRUE(window);
   ASSERT_EQ(window->window_key().empty(), (test_flags & HAS_ID) == 0);
@@ -759,7 +762,6 @@
     ASSERT_TRUE(rvh);
     ASSERT_TRUE(DevToolsAgentHostRegistry::HasDevToolsAgentHost(rvh));
   }
-  CloseShellWindowsAndWaitForAppToExit();

[davidjames, 2012/11/08 17:38:48]

The CL I reverted added the CloseShellWindowsAndWaitForAppToExit API and added
CloseShellWindowsAndWaitForAppToExit() calls at the bottom of a bunch of tests.
This call was added in a subsequent CL, but still needs to be removed, so I
removed it.

 }
 
 }  // namespace
@@ -801,19 +803,14 @@
   ExtensionTestMessageListener listener("PermissionRequestDone", false);
   constrained_window_tab_helper->CloseConstrainedWindows();
   ASSERT_TRUE(listener.WaitUntilSatisfied());
-
-  CloseShellWindowsAndWaitForAppToExit();

[davidjames, 2012/11/08 17:38:48]

Same here. The CL I reverted added the CloseShellWindowsAndWaitForAppToExit API
and added CloseShellWindowsAndWaitForAppToExit() calls at the bottom of a bunch
of tests. This call was added in a subsequent CL, but still needs to be removed,
so I removed it.

 }
 
 // Tests that an app calling chrome.runtime.reload will reload the app and
 // relaunch it if it was running.
 IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, ReloadRelaunches) {
   ExtensionTestMessageListener launched_listener("Launched", true);
-  const Extension* extension = LoadPlatformApp("reload");
+  const Extension* extension = LoadAndLaunchPlatformApp("reload");
   ASSERT_TRUE(extension);
-  application_launch::OpenApplication(application_launch::LaunchParams(

[davidjames, 2012/11/08 17:38:48]

The CL I reverted added the LoadPlatformApp API. I'm guessing that now that
we're switching back to LoadAndLaunchPlatformApp, we don't need to run this
OpenApplication call anymore?

-      browser()->profile(), extension, extension_misc::LAUNCH_NONE,
-      NEW_WINDOW));
   ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
   ASSERT_TRUE(GetFirstShellWindow());
 
@@ -822,7 +819,6 @@
   launched_listener.Reply("reload");
   ASSERT_TRUE(launched_listener2.WaitUntilSatisfied());
   ASSERT_TRUE(GetFirstShellWindow());
-  CloseShellWindowsAndWaitForAppToExit();

[davidjames, 2012/11/08 17:38:48]

The CL I reverted added the CloseShellWindowsAndWaitForAppToExit API and added
CloseShellWindowsAndWaitForAppToExit() calls at the bottom of a bunch of tests.
This call was added in a subsequent CL, but still needs to be removed, so I
removed it.

 }
 
 }  // namespace extensions