Disable install of new default_apps for existing users

chrome/browser/extensions/default_apps.cc

Index: chrome/browser/extensions/default_apps.cc
diff --git a/chrome/browser/extensions/default_apps.cc b/chrome/browser/extensions/default_apps.cc
index 31762906b121cdb5b6cc70d9f204879bd31ced62..7a831d12878922282721d632aeb14bae9474c346 100644
--- a/chrome/browser/extensions/default_apps.cc
+++ b/chrome/browser/extensions/default_apps.cc
@@ -17,9 +17,36 @@
 #include "chrome/common/pref_names.h"
 #include "ui/base/l10n/l10n_util.h"
 
+namespace {
+
+// Returns true if the app was a default app in Chrome 22
+bool IsOldDefaultApp(const std::string& extension_id) {
+  const std::string gmail_id = "pjkljhegncpnkpknbcohdijeoejaedia";

[Mihai Parparita -not on Chrome, 2012/08/28 18:22:59]

These can be moved to const char[]'s directly in the anonymous namespace,
instead of being inside the function.

+  const std::string search_id = "coobgpohoikkiipiblmjeljniedjpjpf";
+  const std::string youtube_id = "blpcfgokakmgnkcojhhkbfbldkacnbeo";
+  return extension_id == gmail_id || extension_id == search_id
+      || extension_id == youtube_id;
+}
+
+bool IsLocaleSupported() {
+  // Don't bother installing default apps in locales where it is known that
+  // they don't work.
+  // TODO(rogerta): Do this check dynamically once the webstore can expose
+  // an API. See http://crbug.com/101357
+  const std::string& locale = g_browser_process->GetApplicationLocale();
+  static const char* unsupported_locales[] = {"CN", "TR", "IR"};
+  for (size_t i = 0; i < arraysize(unsupported_locales); ++i) {
+    if (EndsWith(locale, unsupported_locales[i], false)) {
+      return false;
+    }
+  }
+  return true;
+}

[Mihai Parparita -not on Chrome, 2012/08/28 18:22:59]

Add blank newline after this.

+}

[Mihai Parparita -not on Chrome, 2012/08/28 18:22:59]

It's customary to add a "  // namespace" end marker here.

+
 namespace default_apps {
 
-bool ShouldInstallInProfile(Profile* profile) {
+bool Provider::ShouldInstallInProfile() {

[Mihai Parparita -not on Chrome, 2012/08/28 18:22:59]

Move this down to be next to the other Provider methods.

   // We decide to install or not install default apps based on the following
   // criteria, from highest priority to lowest priority:
   //
@@ -31,18 +58,20 @@ bool ShouldInstallInProfile(Profile* profile) {
   // - The kDefaultApps preferences value in the profile.  This value is
   //   usually set in the master_preferences file.
   bool install_apps =
-      profile->GetPrefs()->GetString(prefs::kDefaultApps) == "install";
+      profile_->GetPrefs()->GetString(prefs::kDefaultApps) == "install";
 
-  default_apps::InstallState state =
-      static_cast<default_apps::InstallState>(profile->GetPrefs()->GetInteger(
+  InstallState state =
+      static_cast<InstallState>(profile_->GetPrefs()->GetInteger(
           prefs::kDefaultAppsInstallState));
 
+  isMigration_ = (state == kProvideLegacyDefaultApps);
+
   switch (state) {
-    case default_apps::kUnknown: {
+    case kUnknown: {
       // This is the first time the default apps feature runs on this profile.
       // Determine if we want to install them or not.
       chrome::VersionInfo version_info;
-      if (!profile->WasCreatedByVersionOrLater(version_info.Version().c_str()))
+      if (!profile_->WasCreatedByVersionOrLater(version_info.Version().c_str()))
         install_apps = false;
       break;
     }
@@ -52,21 +81,21 @@ bool ShouldInstallInProfile(Profile* profile) {
     // apps affected all users. Migrate old default apps to new mechanism where
     // they are installed only once as INTERNAL.
     // TODO(grv) : remove after Q1-2013.
-    case default_apps::kProvideLegacyDefaultApps:
-      profile->GetPrefs()->SetInteger(
+    case kProvideLegacyDefaultApps:
+      profile_->GetPrefs()->SetInteger(
           prefs::kDefaultAppsInstallState,
-          default_apps::kAlreadyInstalledDefaultApps);
+          kAlreadyInstalledDefaultApps);
       break;
 
-    case default_apps::kAlreadyInstalledDefaultApps:
-    case default_apps::kNeverInstallDefaultApps:
+    case kAlreadyInstalledDefaultApps:
+    case kNeverInstallDefaultApps:
       install_apps = false;
       break;
     default:
       NOTREACHED();
   }
 
-  if (install_apps && !isLocaleSupported()) {
+  if (install_apps && !IsLocaleSupported()) {
     install_apps = false;
   }
 
@@ -82,14 +111,14 @@ bool ShouldInstallInProfile(Profile* profile) {
 
   // Default apps are only installed on profile creation or a new chrome
   // download.
-  if (state == default_apps::kUnknown) {
+  if (state == kUnknown) {
     if (install_apps) {
-      profile->GetPrefs()->SetInteger(
+      profile_->GetPrefs()->SetInteger(
           prefs::kDefaultAppsInstallState,
-          default_apps::kAlreadyInstalledDefaultApps);
+          kAlreadyInstalledDefaultApps);
     } else {
-      profile->GetPrefs()->SetInteger(prefs::kDefaultAppsInstallState,
-                                      default_apps::kNeverInstallDefaultApps);
+      profile_->GetPrefs()->SetInteger(prefs::kDefaultAppsInstallState,
+                                      kNeverInstallDefaultApps);
     }
   }
 
@@ -109,13 +138,13 @@ Provider::Provider(Profile* profile,
                    int creation_flags)
     : extensions::ExternalProviderImpl(service, loader, crx_location,
                                        download_location, creation_flags),
-      profile_(profile) {
+      profile_(profile), isMigration_(false) {
   DCHECK(profile);
   set_auto_acknowledge(true);
 }
 
 void Provider::VisitRegisteredExtension() {
-  if (!profile_ || !ShouldInstallInProfile(profile_)) {
+  if (!profile_ || !ShouldInstallInProfile()) {
     base::DictionaryValue* prefs = new base::DictionaryValue;
     SetPrefs(prefs);
     return;
@@ -124,19 +153,23 @@ void Provider::VisitRegisteredExtension() {
   extensions::ExternalProviderImpl::VisitRegisteredExtension();
 }
 
-bool isLocaleSupported() {
-  // Don't bother installing default apps in locales where it is known that
-  // they don't work.
-  // TODO(rogerta): Do this check dynamically once the webstore can expose
-  // an API. See http://crbug.com/101357
-  const std::string& locale = g_browser_process->GetApplicationLocale();
-  static const char* unsupported_locales[] = {"CN", "TR", "IR"};
-  for (size_t i = 0; i < arraysize(unsupported_locales); ++i) {
-    if (EndsWith(locale, unsupported_locales[i], false)) {
-      return false;
+void Provider::SetPrefs(base::DictionaryValue* prefs) {
+  if (isMigration_) {
+    std::set<std::string> new_default_apps;
+    for (base::DictionaryValue::key_iterator i = prefs->begin_keys();
+         i != prefs->end_keys(); ++i) {
+      if (!IsOldDefaultApp(*i)) {
+        new_default_apps.insert(*i);
+      }
+    }
+    // Filter out the new default apps for migrating users.
+    for (std::set<std::string>::iterator it = new_default_apps.begin();
+         it != new_default_apps.end(); ++it) {
+      prefs->Remove(*it, NULL);
     }
   }
-  return true;
+
+  ExternalProviderImpl::SetPrefs(prefs);
 }
 
 }  // namespace default_apps