Make --keep-alive-for-test respect the kBackgroundModeEnabled preference.

The motivation for this is that we may one day want to make background mode be
the default, which this flag currently achieves, but in that case we'd need to
respect the user's desire to disable background mode altogether.

TEST=Starting chrome with --keep-alive-for-test should display a chrome icon in the system tray unless background mode is disabled from the advanced tab in chrome://settings. Toggling that setting should toggle the presence of the icon.

Review URL: https://chromiumcodereview.appspot.com/10831254

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

chrome/browser/background/background_mode_manager.cc

@@ -183,12 +183,12 @@ BackgroundModeManager::BackgroundModeManager(
   }
 
   // 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.
-  if (command_line->HasSwitch(switches::kKeepAliveForTest)) {
+  // in the background until the user explicitly terminates it.
+  if (command_line->HasSwitch(switches::kKeepAliveForTest))
     keep_alive_for_test_ = true;
+
+  if (ShouldBeInBackgroundMode())
     StartBackgroundMode();
-  }
 
   // Listen for the application shutting down so we can decrement our KeepAlive
   // count.
@@ -288,10 +288,7 @@ void BackgroundModeManager::Observe(
     case chrome::NOTIFICATION_PREF_CHANGED:
       DCHECK(*content::Details<std::string>(details).ptr() ==
              prefs::kBackgroundModeEnabled);
-      if (IsBackgroundModePrefEnabled())
-        EnableBackgroundMode();
-      else
-        DisableBackgroundMode();
+      OnBackgroundModeEnabledPrefChanged();
       break;
     case chrome::NOTIFICATION_EXTENSIONS_READY:
       // Extensions are loaded, so we don't need to manually keep the browser
@@ -345,6 +342,13 @@ void BackgroundModeManager::Observe(
   }
 }
 
+void BackgroundModeManager::OnBackgroundModeEnabledPrefChanged() {
+  if (IsBackgroundModePrefEnabled())
+    EnableBackgroundMode();
+  else
+    DisableBackgroundMode();
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 //  BackgroundModeManager, BackgroundApplicationListModel::Observer overrides
 void BackgroundModeManager::OnApplicationDataChanged(
@@ -356,9 +360,6 @@ void BackgroundModeManager::OnApplicationListChanged(Profile* profile) {
   if (!IsBackgroundModePrefEnabled())
     return;
 
-  // Figure out what just happened based on the count of background apps.
-  int count = GetBackgroundAppCount();
-
   // Update the profile cache with the fact whether background apps are running
   // for this profile.
   size_t profile_index = profile_cache_->GetIndexOfProfileWithPath(
@@ -368,7 +369,7 @@ void BackgroundModeManager::OnApplicationListChanged(Profile* profile) {
         profile_index, GetBackgroundAppCountForProfile(profile) != 0);
   }
 
-  if (count == 0) {
+  if (!ShouldBeInBackgroundMode()) {
     // We've uninstalled our last background app, make sure we exit background
     // mode and no longer launch on startup.
     EnableLaunchOnStartup(false);
@@ -535,10 +536,10 @@ void BackgroundModeManager::EndKeepAliveForStartup() {
 }
 
 void BackgroundModeManager::StartBackgroundMode() {
+  DCHECK(ShouldBeInBackgroundMode());
   // Don't bother putting ourselves in background mode if we're already there
   // or if background mode is disabled.
-  if (in_background_mode_ ||
-      (!IsBackgroundModePrefEnabled() && !keep_alive_for_test_))
+  if (in_background_mode_)
     return;
 
   // Mark ourselves as running in background mode.
@@ -559,9 +560,8 @@ void BackgroundModeManager::StartBackgroundMode() {
 void BackgroundModeManager::InitStatusTrayIcon() {
   // Only initialize status tray icons for those profiles which actually
   // have a background app running.
-  if (keep_alive_for_test_ || GetBackgroundAppCount() > 0) {
+  if (ShouldBeInBackgroundMode())
     CreateStatusTrayIcon();
-  }
 }
 
 void BackgroundModeManager::EndBackgroundMode() {
@@ -582,8 +582,7 @@ void BackgroundModeManager::EndBackgroundMode() {
 void BackgroundModeManager::EnableBackgroundMode() {
   DCHECK(IsBackgroundModePrefEnabled());
   // If background mode should be enabled, but isn't, turn it on.
-  if (!in_background_mode_ &&
-      (GetBackgroundAppCount() > 0 || keep_alive_for_test_)) {
+  if (!in_background_mode_ && ShouldBeInBackgroundMode()) {
     StartBackgroundMode();
     EnableLaunchOnStartup(true);
   }
@@ -617,6 +616,11 @@ int BackgroundModeManager::GetBackgroundAppCountForProfile(
   return bmd->GetBackgroundAppCount();
 }
 
+bool BackgroundModeManager::ShouldBeInBackgroundMode() const {
+  return IsBackgroundModePrefEnabled() &&
+      (GetBackgroundAppCount() > 0 || keep_alive_for_test_);
+}
+
 void BackgroundModeManager::OnBackgroundAppInstalled(
     const Extension* extension) {
   // Background mode is disabled - don't do anything.

chrome/browser/background/background_mode_manager.h

@@ -73,6 +73,10 @@ class BackgroundModeManager
                            BackgroundLaunchOnStartup);
   FRIEND_TEST_ALL_PREFIXES(BackgroundModeManagerTest,
                            BackgroundAppInstallUninstallWhileDisabled);
+  FRIEND_TEST_ALL_PREFIXES(BackgroundModeManagerTest,
+                           BackgroundModeDisabledPreventsKeepAliveOnStartup);
+  FRIEND_TEST_ALL_PREFIXES(BackgroundModeManagerTest,
+                           DisableBackgroundModeUnderTestFlag);
   FRIEND_TEST_ALL_PREFIXES(BackgroundModeManagerTest,
                            EnableAfterBackgroundAppInstall);
   FRIEND_TEST_ALL_PREFIXES(BackgroundModeManagerTest,
@@ -154,6 +158,9 @@ class BackgroundModeManager
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
+  // Called when the kBackgroundModeEnabled preference changes.
+  void OnBackgroundModeEnabledPrefChanged();
+
   // BackgroundApplicationListModel::Observer implementation.
   virtual void OnApplicationDataChanged(const extensions::Extension* extension,
                                         Profile* profile) OVERRIDE;
@@ -256,6 +263,9 @@ class BackgroundModeManager
   // Returns the number of background apps for a profile.
   virtual int GetBackgroundAppCountForProfile(Profile* const profile) const;
 
+  // Returns true if we should be in background mode.
+  bool ShouldBeInBackgroundMode() const;
+
   // Reference to the profile info cache. It is used to update the background
   // app status of profiles when they open/close background apps.
   ProfileInfoCache* profile_cache_;

chrome/browser/background/background_mode_manager_unittest.cc

@@ -32,10 +32,10 @@ class BackgroundModeManagerTest : public testing::Test {
 
 class TestBackgroundModeManager : public BackgroundModeManager {
  public:
-  explicit TestBackgroundModeManager(
-      CommandLine* command_line, ProfileInfoCache* cache)
+  TestBackgroundModeManager(
+      CommandLine* command_line, ProfileInfoCache* cache, bool enabled)
       : BackgroundModeManager(command_line, cache),
-        enabled_(true),
+        enabled_(enabled),
         app_count_(0),
         profile_app_count_(0),
         have_status_tray_(false),
@@ -55,7 +55,10 @@ class TestBackgroundModeManager : public BackgroundModeManager {
   void SetBackgroundAppCountForProfile(int count) {
     profile_app_count_ = count;
   }
-  void SetEnabled(bool enabled) { enabled_ = enabled; }
+  void SetEnabled(bool enabled) {
+    enabled_ = enabled;
+    OnBackgroundModeEnabledPrefChanged();
+  }
   bool HaveStatusTray() const { return have_status_tray_; }
   bool IsLaunchOnStartup() const { return launch_on_startup_; }
  private:
@@ -85,7 +88,7 @@ static void AssertBackgroundModeInactive(
 TEST_F(BackgroundModeManagerTest, BackgroundAppLoadUnload) {
   TestingProfile* profile = profile_manager_.CreateTestingProfile("p1");
   TestBackgroundModeManager manager(
-      command_line_.get(), profile_manager_.profile_info_cache());
+      command_line_.get(), profile_manager_.profile_info_cache(), true);
   manager.RegisterProfile(profile);
   EXPECT_FALSE(browser::WillKeepAlive());
 
@@ -105,7 +108,7 @@ TEST_F(BackgroundModeManagerTest, BackgroundAppLoadUnload) {
 TEST_F(BackgroundModeManagerTest, BackgroundAppInstallUninstallWhileDisabled) {
   TestingProfile* profile = profile_manager_.CreateTestingProfile("p1");
   TestBackgroundModeManager manager(
-      command_line_.get(), profile_manager_.profile_info_cache());
+      command_line_.get(), profile_manager_.profile_info_cache(), true);
   manager.RegisterProfile(profile);
   // Turn off background mode.
   manager.SetEnabled(false);
@@ -135,7 +138,7 @@ TEST_F(BackgroundModeManagerTest, BackgroundAppInstallUninstallWhileDisabled) {
 TEST_F(BackgroundModeManagerTest, EnableAfterBackgroundAppInstall) {
   TestingProfile* profile = profile_manager_.CreateTestingProfile("p1");
   TestBackgroundModeManager manager(
-      command_line_.get(), profile_manager_.profile_info_cache());
+      command_line_.get(), profile_manager_.profile_info_cache(), true);
   manager.RegisterProfile(profile);
 
   // Install app, should show status tray icon.
@@ -168,7 +171,7 @@ TEST_F(BackgroundModeManagerTest, MultiProfile) {
   TestingProfile* profile1 = profile_manager_.CreateTestingProfile("p1");
   TestingProfile* profile2 = profile_manager_.CreateTestingProfile("p2");
   TestBackgroundModeManager manager(
-      command_line_.get(), profile_manager_.profile_info_cache());
+      command_line_.get(), profile_manager_.profile_info_cache(), true);
   manager.RegisterProfile(profile1);
   manager.RegisterProfile(profile2);
   EXPECT_FALSE(browser::WillKeepAlive());
@@ -209,7 +212,7 @@ TEST_F(BackgroundModeManagerTest, ProfileInfoCacheStorage) {
   TestingProfile* profile1 = profile_manager_.CreateTestingProfile("p1");
   TestingProfile* profile2 = profile_manager_.CreateTestingProfile("p2");
   TestBackgroundModeManager manager(
-      command_line_.get(), profile_manager_.profile_info_cache());
+      command_line_.get(), profile_manager_.profile_info_cache(), true);
   manager.RegisterProfile(profile1);
   manager.RegisterProfile(profile2);
   EXPECT_FALSE(browser::WillKeepAlive());
@@ -251,10 +254,11 @@ TEST_F(BackgroundModeManagerTest, ProfileInfoCacheStorage) {
   // profiles in the cache.
   EXPECT_EQ(2u, cache->GetNumberOfProfiles());
 }
+
 TEST_F(BackgroundModeManagerTest, ProfileInfoCacheObserver) {
   TestingProfile* profile1 = profile_manager_.CreateTestingProfile("p1");
   TestBackgroundModeManager manager(
-      command_line_.get(), profile_manager_.profile_info_cache());
+      command_line_.get(), profile_manager_.profile_info_cache(), true);
   manager.RegisterProfile(profile1);
   EXPECT_FALSE(browser::WillKeepAlive());
 
@@ -286,3 +290,24 @@ TEST_F(BackgroundModeManagerTest, ProfileInfoCacheObserver) {
   EXPECT_EQ(UTF8ToUTF16("p1"),
             manager.GetBackgroundModeData(profile1)->name());
 }
+
+TEST_F(BackgroundModeManagerTest, DisableBackgroundModeUnderTestFlag) {
+  TestingProfile* profile1 = profile_manager_.CreateTestingProfile("p1");
+  command_line_->AppendSwitch(switches::kKeepAliveForTest);
+  TestBackgroundModeManager manager(
+      command_line_.get(), profile_manager_.profile_info_cache(), true);
+  manager.RegisterProfile(profile1);
+  EXPECT_TRUE(manager.ShouldBeInBackgroundMode());
+  manager.SetEnabled(false);
+  EXPECT_FALSE(manager.ShouldBeInBackgroundMode());
+}
+
+TEST_F(BackgroundModeManagerTest,
+       BackgroundModeDisabledPreventsKeepAliveOnStartup) {
+  TestingProfile* profile1 = profile_manager_.CreateTestingProfile("p1");
+  command_line_->AppendSwitch(switches::kKeepAliveForTest);
+  TestBackgroundModeManager manager(
+      command_line_.get(), profile_manager_.profile_info_cache(), false);
+  manager.RegisterProfile(profile1);
+  EXPECT_FALSE(manager.ShouldBeInBackgroundMode());
+}