Do not create login items if the user deleted a previous one.

Added a new kUserRemovedLoginItem pref that is used to track if the user has
previously removed a login item for chrome. This is so we don't keep re-creating
the login item every time the user enables/installs a background app if they
have indicated that they don't want chrome to open at login.

BUG=140017
TEST=Install background app on mac, uncheck "Open at Login" checkbox in dock menu, disable and re-enable the app via Tools->Extension, make sure Open at Login checkbox does not get re-enabled.


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

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

chrome/browser/background/background_mode_manager.cc

@@ -216,6 +216,7 @@ BackgroundModeManager::~BackgroundModeManager() {
 // static
 void BackgroundModeManager::RegisterPrefs(PrefService* prefs) {
   prefs->RegisterBooleanPref(prefs::kUserCreatedLoginItem, false);
+  prefs->RegisterBooleanPref(prefs::kUserRemovedLoginItem, false);
   prefs->RegisterBooleanPref(prefs::kBackgroundModeEnabled, true);
 }
 

chrome/browser/background/background_mode_manager_mac.mm

@@ -17,14 +17,42 @@
 using content::BrowserThread;
 
 namespace {
+#if !defined(NDEBUG)
+// The code to remove a login item has a potential race (because the code to
+// set and check the kUserRemovedLoginItem pref runs on the UI thread, while
+// the code that checks for a login item runs on the IO thread). We add this
+// flag which should always match the value of the pref to see if we ever hit
+// this race in practice.
+static bool login_item_removed = false;
+#endif
+
+void SetUserRemovedLoginItemPrefCallback() {
+  PrefService* service = g_browser_process->local_state();
+  service->SetBoolean(prefs::kUserRemovedLoginItem, true);
+}
 
 void DisableLaunchOnStartupCallback() {
   // Check if Chrome is not a login Item, or is a Login Item but w/o 'hidden'
   // flag - most likely user has modified the setting, don't override it.
   bool is_hidden = false;
-  if (!base::mac::CheckLoginItemStatus(&is_hidden) || !is_hidden)
+  if (!base::mac::CheckLoginItemStatus(&is_hidden)) {
+    // No login item - this means the user must have already removed it, so
+    // call back to the UI thread to set a preference so we don't try to
+    // recreate it the next time they enable/install a background app.
+#if !defined(NDEBUG)
+    login_item_removed = true;
+#endif
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                            base::Bind(SetUserRemovedLoginItemPrefCallback));
+    return;
+  }
+
+  // If the login item does not have the "hidden" flag set, just leave it there
+  // since it means the user must have created it.
+  if (!is_hidden)
     return;
 
+  // Remove the login item we created.
   base::mac::RemoveFromLoginItems();
 }
 
@@ -33,8 +61,8 @@ void SetUserCreatedLoginItemPrefCallback() {
   service->SetBoolean(prefs::kUserCreatedLoginItem, true);
 }
 
-void EnableLaunchOnStartupCallback() {
-  // Return if Chrome is already a Login Item (avoid overriding user choice).
+void EnableLaunchOnStartupCallback(bool should_add_login_item) {
+  // Check if Chrome is already a Login Item (avoid overriding user choice).
   if (base::mac::CheckLoginItemStatus(NULL)) {
     // Call back to the UI thread to set our preference so we don't delete the
     // user's login item when we disable launch on startup. There's a race
@@ -46,7 +74,12 @@ void EnableLaunchOnStartupCallback() {
     return;
   }
 
-  base::mac::AddToLoginItems(true);  // Hide on startup.
+  if (should_add_login_item)
+    base::mac::AddToLoginItems(true);  // Hide on startup.
+#if !defined(NDEBUG)
+  else
+    DCHECK(!login_item_removed); // Check for race condition (see above).
+#endif
 }
 
 }  // namespace
@@ -57,13 +90,26 @@ void BackgroundModeManager::EnableLaunchOnStartup(bool should_launch) {
     return;
 
   if (should_launch) {
+    PrefService* service = g_browser_process->local_state();
+    // Create a login item if the user did not remove our login item
+    // previously. We call out to the FILE thread either way since we
+    // want to check for a user-created login item.
+    bool should_add_login_item =
+        !service->GetBoolean(prefs::kUserRemovedLoginItem);
     BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-                            base::Bind(EnableLaunchOnStartupCallback));
+                            base::Bind(EnableLaunchOnStartupCallback,
+                                       should_add_login_item));
   } else {
     PrefService* service = g_browser_process->local_state();
     if (service->GetBoolean(prefs::kUserCreatedLoginItem)) {
-      // We didn't create the login item, so nothing to do here.
+      // We didn't create the login item, so nothing to do here. Clear our
+      // prefs so if the user removes the login item before installing a
+      // background app, we will revert to the default behavior.
       service->ClearPref(prefs::kUserCreatedLoginItem);
+      service->ClearPref(prefs::kUserRemovedLoginItem);
+#if !defined(NDEBUG)
+      login_item_removed = false;
+#endif
       return;
     }
     // Call to the File thread to remove the login item since it requires

chrome/common/pref_names.cc

@@ -1934,6 +1934,10 @@ const char kManagedAutoSelectCertificateForUrls[] =
 // uninstalling background apps.
 const char kUserCreatedLoginItem[] = "background_mode.user_created_login_item";
 
+// Set to true if the user removed our login item so we should not create a new
+// one when uninstalling background apps.
+const char kUserRemovedLoginItem[] = "background_mode.user_removed_login_item";
+
 // Set to true if background mode is enabled on this browser.
 const char kBackgroundModeEnabled[] = "background_mode.enabled";
 

chrome/common/pref_names.h

@@ -704,6 +704,7 @@ extern const char kIgnoredProtocolHandlers[];
 extern const char kCustomHandlersEnabled[];
 
 extern const char kUserCreatedLoginItem[];
+extern const char kUserRemovedLoginItem[];
 extern const char kBackgroundModeEnabled[];
 
 extern const char kDevicePolicyRefreshRate[];