BackgroundModeManager is now responsible for KeepAlive for --no-startup-window

Updated BackgroundModeManager to keep the browser process alive until extensions finish loading.
This fixes a race condition with the previous implementation of --no-startup-window which
would let chrome exit if the message loop was re-entered before background apps were loaded.

BUG=58083
TEST=run with --no-startup-window, make sure chrome starts

Review URL: http://codereview.chromium.org/3664001

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

3 files changed, 46 insertions, 12 deletions

diff --git a/chrome/browser/background_mode_manager.cc b/chrome/browser/background_mode_manager.cc
index 64a5023..d7999da 100644
--- a/chrome/browser/background_mode_manager.cc
+++ b/chrome/browser/background_mode_manager.cc
@@ -132,13 +132,24 @@ BackgroundModeManager::BackgroundModeManager(Profile* profile,
     : profile_(profile),
       background_app_count_(0),
       in_background_mode_(false),
+      keep_alive_for_startup_(false),
       status_tray_(NULL),
       status_icon_(NULL) {
-  // If background mode is disabled, just exit - don't listen for
+  // If background mode or apps are disabled, just exit - don't listen for
   // any notifications.
-  if (!command_line->HasSwitch(switches::kEnableBackgroundMode))
+  if (!command_line->HasSwitch(switches::kEnableBackgroundMode) ||
+      command_line->HasSwitch(switches::kDisableExtensions))
     return;
 
+  // Keep the browser alive until extensions are done loading - this is needed
+  // by the --no-startup-window flag. We want to stay alive until we load
+  // extensions, at which point we should either run in background mode (if
+  // there are background apps) or exit if there are none.
+  if (command_line->HasSwitch(switches::kNoStartupWindow)) {
+    keep_alive_for_startup_ = true;
+    BrowserList::StartKeepAlive();
+  }
+
   // If the -keep-alive-for-test flag is passed, then always keep chrome running
   // in the background until the user explicitly terminates it, by acting as if
   // we loaded a background app.
@@ -215,9 +226,13 @@ void BackgroundModeManager::Observe(NotificationType type,
                                     const NotificationDetails& details) {
   switch (type.value) {
     case NotificationType::EXTENSIONS_READY:
-    // On a Mac, we use 'login items' mechanism which has user-facing UI so we
-    // don't want to stomp on user choice every time we start and load
-    // registered extensions.
+      // Extensions are loaded, so we don't need to manually keep the browser
+      // process alive any more when running in no-startup-window mode.
+      EndKeepAliveForStartup();
+
+      // On a Mac, we use 'login items' mechanism which has user-facing UI so we
+      // don't want to stomp on user choice every time we start and load
+      // registered extensions.
 #if !defined(OS_MACOSX)
       EnableLaunchOnStartup(IsBackgroundModeEnabled() &&
                             background_app_count_ > 0);
@@ -242,6 +257,9 @@ void BackgroundModeManager::Observe(NotificationType type,
         OnBackgroundAppUninstalled();
       break;
     case NotificationType::APP_TERMINATING:
+      // Make sure we aren't still keeping the app alive (only happens if we
+      // don't receive an EXTENSIONS_READY notification for some reason).
+      EndKeepAliveForStartup();
       // Performing an explicit shutdown, so exit background mode (does nothing
       // if we aren't in background mode currently).
       EndBackgroundMode();
@@ -260,6 +278,17 @@ void BackgroundModeManager::Observe(NotificationType type,
   }
 }
 
+void BackgroundModeManager::EndKeepAliveForStartup() {
+  if (keep_alive_for_startup_) {
+    keep_alive_for_startup_ = false;
+    // We call this via the message queue to make sure we don't try to end
+    // keep-alive (which can shutdown Chrome) before the message loop has
+    // started.
+    MessageLoop::current()->PostTask(
+        FROM_HERE, NewRunnableFunction(BrowserList::EndKeepAlive));
+  }
+}
+
 void BackgroundModeManager::OnBackgroundModePrefChanged() {
   // Background mode has been enabled/disabled in preferences, so update our
   // state accordingly.
diff --git a/chrome/browser/background_mode_manager.h b/chrome/browser/background_mode_manager.h
index 9c3de16..e662ca75 100644
--- a/chrome/browser/background_mode_manager.h
+++ b/chrome/browser/background_mode_manager.h
@@ -107,6 +107,12 @@ class BackgroundModeManager
   // the background and removes its status bar icon.
   void EndBackgroundMode();
 
+  // If --no-startup-window is passed, BackgroundModeManager will manually keep
+  // chrome running while waiting for apps to load. This is called when we no
+  // longer need to do this (either because the user has chosen to exit chrome
+  // manually, or all apps have been loaded).
+  void EndKeepAliveForStartup();
+
   // Create a status tray icon to allow the user to shutdown Chrome when running
   // in background mode. Virtual to enable testing.
   virtual void CreateStatusTrayIcon();
@@ -133,6 +139,11 @@ class BackgroundModeManager
   // user disables/enables background mode via preferences.
   bool in_background_mode_;
 
+  // Set when we are keeping chrome running during the startup process - this
+  // is required when running with the --no-startup-window flag, as otherwise
+  // chrome would immediately exit due to having no open windows.
+  bool keep_alive_for_startup_;
+
   // Reference to our status tray (owned by our parent profile). If null, the
   // platform doesn't support status icons.
   StatusTray* status_tray_;
diff --git a/chrome/browser/browser_init.cc b/chrome/browser/browser_init.cc
index bb8c5d0..c4c141f 100644
--- a/chrome/browser/browser_init.cc
+++ b/chrome/browser/browser_init.cc
@@ -638,14 +638,8 @@ void BrowserInit::LaunchWithProfile::ProcessLaunchURLs(
     const std::vector<GURL>& urls_to_open) {
   // If we're starting up in "background mode" (no open browser window) then
   // don't open any browser windows.
-  if (process_startup && command_line_.HasSwitch(switches::kNoStartupWindow)) {
-    BrowserList::StartKeepAlive();
-    // Keep the app alive while the system initializes, then allow it to
-    // shutdown if no other module wants to keep it running.
-    MessageLoop::current()->PostTask(
-        FROM_HERE, NewRunnableFunction(BrowserList::EndKeepAlive));
+  if (process_startup && command_line_.HasSwitch(switches::kNoStartupWindow))
     return;
-  }
 
   if (process_startup && ProcessStartupURLs(urls_to_open)) {
     // ProcessStartupURLs processed the urls, nothing else to do.