Revamp the MigrateBrowserPrefs and MigrateUserPrefs code.

Restructure it so it is organized by date-added, and delete all migrations that are sufficiently old.

BUG=69995,165672

Review URL: https://codereview.chromium.org/944433002

Cr-Commit-Position: refs/heads/master@{#318780}

chrome/browser/chromeos/login/default_pinned_apps_field_trial.cc

-// Copyright 2013 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "chrome/browser/chromeos/login/default_pinned_apps_field_trial.h"
-
-#include "base/basictypes.h"
-#include "base/prefs/pref_registry_simple.h"
-#include "base/prefs/pref_service.h"
-
-namespace chromeos {
-namespace default_pinned_apps_field_trial {
-
-namespace {
-
-// Name of a local state pref to store the list of users that participate
-// the experiment.
-const char kExperimentUsers[] = "DefaultPinnedAppsExperimentUsers";
-
-}  // namespace
-
-void RegisterPrefs(PrefRegistrySimple* registry) {
-  registry->RegisterListPref(kExperimentUsers);
-}
-
-void MigratePrefs(PrefService* local_state) {
-  // TODO(xiyuan): Remove everything in M34.
-  local_state->ClearPref(kExperimentUsers);
-}
-
-}  // namespace default_pinned_apps_field_trial
-}  // namespace chromeos

chrome/browser/chromeos/login/default_pinned_apps_field_trial.h

-// Copyright 2013 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CHROME_BROWSER_CHROMEOS_LOGIN_DEFAULT_PINNED_APPS_FIELD_TRIAL_H_
-#define CHROME_BROWSER_CHROMEOS_LOGIN_DEFAULT_PINNED_APPS_FIELD_TRIAL_H_
-
-#include <string>
-
-namespace base {
-class ListValue;
-}
-
-class PrefRegistrySimple;
-class PrefService;
-
-namespace chromeos {
-namespace default_pinned_apps_field_trial {
-
-// Registers a local state that keeps track of users in experiment.
-void RegisterPrefs(PrefRegistrySimple* registry);
-
-// Migrate by removing the old local state.
-void MigratePrefs(PrefService* local_state);
-
-}  // namespace default_pinned_apps_field_trial
-}  // namespace chromeos
-
-#endif  // CHROME_BROWSER_CHROMEOS_LOGIN_DEFAULT_PINNED_APPS_FIELD_TRIAL_H_

chrome/browser/prefs/browser_prefs.cc

@@ -162,7 +162,6 @@
 #include "chrome/browser/chromeos/extensions/echo_private_api.h"
 #include "chrome/browser/chromeos/file_system_provider/registry.h"
 #include "chrome/browser/chromeos/first_run/first_run.h"
-#include "chrome/browser/chromeos/login/default_pinned_apps_field_trial.h"
 #include "chrome/browser/chromeos/login/saml/saml_offline_signin_limiter.h"
 #include "chrome/browser/chromeos/login/session/user_session_manager.h"
 #include "chrome/browser/chromeos/login/startup_utils.h"
@@ -223,19 +222,6 @@
 
 namespace {
 
-enum MigratedPreferences {
-  NO_PREFS = 0,
-  DNS_PREFS = 1 << 0,
-  WINDOWS_PREFS = 1 << 1,
-};
-
-// A previous feature (see
-// chrome/browser/protector/protected_prefs_watcher.cc in source
-// control history) used this string as a prefix for various prefs it
-// registered. We keep it here for now to clear out those old prefs in
-// MigrateUserPrefs.
-const char kBackupPref[] = "backup";
-
 #if !defined(OS_ANDROID)
 // The AutomaticProfileResetter service used this preference to save that the
 // profile reset prompt had already been shown, however, the preference has been
@@ -250,8 +236,6 @@ const char kLegacyProfileResetPromptMemento[] = "profile.reset_prompt_memento";
 namespace chrome {
 
 void RegisterLocalState(PrefRegistrySimple* registry) {
-  // Prefs in Local State.
-  registry->RegisterIntegerPref(prefs::kMultipleProfilePrefMigration, 0);
 
   // Please keep this list alphabetized.
   AppListService::RegisterPrefs(registry);
@@ -328,7 +312,6 @@ void RegisterLocalState(PrefRegistrySimple* registry) {
   chromeos::DataPromoNotification::RegisterPrefs(registry);
   chromeos::DeviceOAuth2TokenService::RegisterPrefs(registry);
   chromeos::device_settings_cache::RegisterPrefs(registry);
-  chromeos::default_pinned_apps_field_trial::RegisterPrefs(registry);
   chromeos::EnableDebuggingScreenHandler::RegisterPrefs(registry);
   chromeos::language_prefs::RegisterPrefs(registry);
   chromeos::KioskAppManager::RegisterPrefs(registry);
@@ -528,13 +511,6 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
 #if defined(USE_ASH)
   ash::RegisterChromeLauncherUserPrefs(registry);
 #endif
-
-  // Preferences registered only for migration (clearing or moving to a new key)
-  // go here.
-  registry->RegisterDictionaryPref(
-      kBackupPref,
-      new base::DictionaryValue(),
-      user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
 }
 
 void RegisterUserProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
@@ -557,79 +533,44 @@ void RegisterLoginProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
 }
 #endif
 
-void MigrateUserPrefs(Profile* profile) {
-  PrefService* prefs = profile->GetPrefs();
-
-  // Cleanup prefs from now-removed protector feature.
-  prefs->ClearPref(kBackupPref);
+// This method should be periodically pruned of year+ old migrations.
+void MigrateObsoleteBrowserPrefs(Profile* profile, PrefService* local_state) {
+#if defined(TOOLKIT_VIEWS)
+  // Added 05/2014.
+  MigrateBrowserTabStripPrefs(local_state);
+#endif
 
 #if !defined(OS_ANDROID)
-  // Migrate kNetworkPredictionEnabled to kNetworkPredictionOptions when not on
-  // Android.  On Android, platform-specific code performs preference migration.
-  // TODO(bnc): https://crbug.com/401970  Remove migration code one year after
-  // M38.
-  chrome_browser_net::MigrateNetworkPredictionUserPrefs(prefs);
+  // Added 08/2014.
+  local_state->ClearPref(kLegacyProfileResetPromptMemento);
 #endif
+}
 
-  PromoResourceService::MigrateUserPrefs(prefs);
-  translate::TranslatePrefs::MigrateUserPrefs(prefs, prefs::kAcceptLanguages);
+// This method should be periodically pruned of year+ old migrations.
+void MigrateObsoleteProfilePrefs(Profile* profile) {
+  PrefService* profile_prefs = profile->GetPrefs();
 
 #if defined(OS_MACOSX) && !defined(OS_IOS)
-  autofill::AutofillManager::MigrateUserPrefs(prefs);
+  // Added 06/2014.
+  autofill::AutofillManager::MigrateUserPrefs(profile_prefs);
 #endif  // defined(OS_MACOSX) && !defined(OS_IOS)
 
-#if defined(OS_CHROMEOS) && defined(ENABLE_APP_LIST)
-  MigrateGoogleNowPrefs(profile);
-#endif
-}
-
-void MigrateBrowserPrefs(Profile* profile, PrefService* local_state) {
-  // Copy pref values which have been migrated to user_prefs from local_state,
-  // or remove them from local_state outright, if copying is not required.
-  int current_version =
-      local_state->GetInteger(prefs::kMultipleProfilePrefMigration);
-  PrefRegistrySimple* registry = static_cast<PrefRegistrySimple*>(
-      local_state->DeprecatedGetPrefRegistry());
-
-  if (!(current_version & DNS_PREFS)) {
-    registry->RegisterListPref(prefs::kDnsStartupPrefetchList);
-    local_state->ClearPref(prefs::kDnsStartupPrefetchList);
-
-    registry->RegisterListPref(prefs::kDnsHostReferralList);
-    local_state->ClearPref(prefs::kDnsHostReferralList);
-
-    current_version |= DNS_PREFS;
-    local_state->SetInteger(prefs::kMultipleProfilePrefMigration,
-                            current_version);
-  }
-
-  PrefService* user_prefs = profile->GetPrefs();
-  if (!(current_version & WINDOWS_PREFS)) {
-    registry->RegisterDictionaryPref(prefs::kBrowserWindowPlacement);
-    if (local_state->HasPrefPath(prefs::kBrowserWindowPlacement)) {
-      const PrefService::Preference* pref =
-          local_state->FindPreference(prefs::kBrowserWindowPlacement);
-      DCHECK(pref);
-      user_prefs->Set(prefs::kBrowserWindowPlacement,
-                      *(pref->GetValue()));
-    }
-    local_state->ClearPref(prefs::kBrowserWindowPlacement);
-
-    current_version |= WINDOWS_PREFS;
-    local_state->SetInteger(prefs::kMultipleProfilePrefMigration,
-                            current_version);
-  }
+  // Added 07/2014.
+  translate::TranslatePrefs::MigrateUserPrefs(profile_prefs,
+                                              prefs::kAcceptLanguages);
 
 #if !defined(OS_ANDROID)
-  local_state->ClearPref(kLegacyProfileResetPromptMemento);
-#endif
-
-#if defined(OS_CHROMEOS)
-  chromeos::default_pinned_apps_field_trial::MigratePrefs(local_state);
+  // Added 08/2014.
+  // Migrate kNetworkPredictionEnabled to kNetworkPredictionOptions when not on
+  // Android.  On Android, platform-specific code performs preference migration.
+  // TODO(bnc): https://crbug.com/401970  Remove migration code one year after
+  // M38.
+  chrome_browser_net::MigrateNetworkPredictionUserPrefs(profile_prefs);
 #endif
 
-#if defined(TOOLKIT_VIEWS)
-  MigrateBrowserTabStripPrefs(local_state);
+#if defined(OS_CHROMEOS) && defined(ENABLE_APP_LIST)
+  // Added 02/2015.
+  MigrateGoogleNowPrefs(profile);
 #endif
 }
 

chrome/browser/prefs/browser_prefs.h

@@ -30,11 +30,17 @@ void RegisterUserProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
 void RegisterLoginProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
 #endif
 
-// Migrates prefs from |local_state| to |profile|'s pref store.
-void MigrateBrowserPrefs(Profile* profile, PrefService* local_state);
-
-// Migrates prefs in |profile|'s pref store.
-void MigrateUserPrefs(Profile* profile);
+// Migrate/cleanup deprecated prefs in |local_state|. Over time, long deprecated
+// prefs should be removed as new ones are added, but this call should never go
+// away (even if it becomes an empty call for some time) as it should remain
+// *the* place to drop deprecated browser prefs at.
+void MigrateObsoleteBrowserPrefs(Profile* profile, PrefService* local_state);
+
+// Migrate/cleanup deprecated prefs in |profile|'s pref store. Over time, long
+// deprecated prefs should be removed as new ones are added, but this call
+// should never go away (even if it becomes an empty call for some time) as it
+// should remain *the* place to drop deprecated profile prefs at.
+void MigrateObsoleteProfilePrefs(Profile* profile);
 
 // Migrates zoom level prefs in |profile|'s pref store to a per-StoragePartition
 // set of prefs.

chrome/browser/prefs/pref_service_browsertest.cc

@@ -52,41 +52,29 @@ IN_PROC_BROWSER_TEST_F(PreservedWindowPlacement, MAYBE_Test) {
 
 class PreferenceServiceTest : public InProcessBrowserTest {
  public:
-  explicit PreferenceServiceTest(bool new_profile) : new_profile_(new_profile) {
-  }
-
   bool SetUpUserDataDirectory() override {
     base::FilePath user_data_directory;
     PathService::Get(chrome::DIR_USER_DATA, &user_data_directory);
 
-    if (new_profile_) {
-      original_pref_file_ = ui_test_utils::GetTestFilePath(
-          base::FilePath().AppendASCII("profiles").
-                     AppendASCII("window_placement").
-                     AppendASCII("Default"),
-          base::FilePath().Append(chrome::kPreferencesFilename));
-      tmp_pref_file_ =
-          user_data_directory.AppendASCII(TestingProfile::kTestUserProfileDir);
-      CHECK(base::CreateDirectory(tmp_pref_file_));
-      tmp_pref_file_ = tmp_pref_file_.Append(chrome::kPreferencesFilename);
-    } else {
-      original_pref_file_ = ui_test_utils::GetTestFilePath(
-          base::FilePath().AppendASCII("profiles").
-                     AppendASCII("window_placement"),
-          base::FilePath().Append(chrome::kLocalStateFilename));
-      tmp_pref_file_ = user_data_directory.Append(chrome::kLocalStateFilename);
-    }
-
-    CHECK(base::PathExists(original_pref_file_));
-    // Copy only the Preferences file if |new_profile_|, or Local State if not,
-    // and the rest will be automatically created.
-    CHECK(base::CopyFile(original_pref_file_, tmp_pref_file_));
+    original_pref_file_ = ui_test_utils::GetTestFilePath(
+        base::FilePath()
+            .AppendASCII("profiles")
+            .AppendASCII("window_placement")
+            .AppendASCII("Default"),
+        base::FilePath().Append(chrome::kPreferencesFilename));
+    tmp_pref_file_ =
+        user_data_directory.AppendASCII(TestingProfile::kTestUserProfileDir);
+    EXPECT_TRUE(base::CreateDirectory(tmp_pref_file_));
+    tmp_pref_file_ = tmp_pref_file_.Append(chrome::kPreferencesFilename);
+
+    EXPECT_TRUE(base::PathExists(original_pref_file_));
+    EXPECT_TRUE(base::CopyFile(original_pref_file_, tmp_pref_file_));
 
 #if defined(OS_WIN)
     // Make the copy writable.  On POSIX we assume the umask allows files
     // we create to be writable.
-    CHECK(::SetFileAttributesW(tmp_pref_file_.value().c_str(),
-        FILE_ATTRIBUTE_NORMAL));
+    EXPECT_TRUE(::SetFileAttributesW(tmp_pref_file_.value().c_str(),
+                                     FILE_ATTRIBUTE_NORMAL));
 #endif
     return true;
   }
@@ -94,9 +82,6 @@ class PreferenceServiceTest : public InProcessBrowserTest {
  protected:
   base::FilePath original_pref_file_;
   base::FilePath tmp_pref_file_;
-
- private:
-  bool new_profile_;
 };
 
 #if defined(OS_WIN) || defined(OS_MACOSX)
@@ -106,13 +91,7 @@ class PreferenceServiceTest : public InProcessBrowserTest {
 // might be able to make this work on buildbots.
 // TODO(port): revisit this.
 
-class PreservedWindowPlacementIsLoaded : public PreferenceServiceTest {
- public:
-  PreservedWindowPlacementIsLoaded() : PreferenceServiceTest(true) {
-  }
-};
-
-IN_PROC_BROWSER_TEST_F(PreservedWindowPlacementIsLoaded, Test) {
+IN_PROC_BROWSER_TEST_F(PreferenceServiceTest, Test) {
 #if defined(OS_WIN) && defined(USE_ASH)
   // Disable this test in Metro+Ash for now (http://crbug.com/262796).
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
@@ -164,69 +143,3 @@ IN_PROC_BROWSER_TEST_F(PreservedWindowPlacementIsLoaded, Test) {
   EXPECT_EQ(is_maximized, is_window_maximized);
 }
 #endif
-
-#if defined(OS_WIN) || defined(OS_MACOSX)
-
-class PreservedWindowPlacementIsMigrated : public PreferenceServiceTest {
- public:
-  PreservedWindowPlacementIsMigrated() : PreferenceServiceTest(false) {
-  }
-};
-
-IN_PROC_BROWSER_TEST_F(PreservedWindowPlacementIsMigrated, Test) {
-#if defined(OS_WIN) && defined(USE_ASH)
-  // Disable this test in Metro+Ash for now (http://crbug.com/262796).
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kAshBrowserTests))
-    return;
-#endif
-
-  // The window should open with the old reference profile, with window
-  // placement values stored in Local State.
-
-  JSONFileValueSerializer deserializer(original_pref_file_);
-  scoped_ptr<base::Value> root(deserializer.Deserialize(NULL, NULL));
-
-  ASSERT_TRUE(root.get());
-  ASSERT_TRUE(root->IsType(base::Value::TYPE_DICTIONARY));
-
-  // Retrieve the screen rect for the launched window
-  gfx::Rect bounds = browser()->window()->GetRestoredBounds();
-
-  // Values from old reference profile in Local State should have been
-  // correctly migrated to the user's Preferences -- if so, the window
-  // should be set to values taken from the user's Local State.
-  base::DictionaryValue* root_dict =
-      static_cast<base::DictionaryValue*>(root.get());
-
-  // Retrieve the expected rect values from User Preferences, where they
-  // should have been migrated from Local State.
-  int bottom = 0;
-  std::string kBrowserWindowPlacement(prefs::kBrowserWindowPlacement);
-  EXPECT_TRUE(root_dict->GetInteger(kBrowserWindowPlacement + ".bottom",
-      &bottom));
-  EXPECT_EQ(bottom, bounds.y() + bounds.height());
-
-  int top = 0;
-  EXPECT_TRUE(root_dict->GetInteger(kBrowserWindowPlacement + ".top",
-      &top));
-  EXPECT_EQ(top, bounds.y());
-
-  int left = 0;
-  EXPECT_TRUE(root_dict->GetInteger(kBrowserWindowPlacement + ".left",
-      &left));
-  EXPECT_EQ(left, bounds.x());
-
-  int right = 0;
-  EXPECT_TRUE(root_dict->GetInteger(kBrowserWindowPlacement + ".right",
-      &right));
-  EXPECT_EQ(right, bounds.x() + bounds.width());
-
-  // Find if launched window is maximized.
-  bool is_window_maximized = browser()->window()->IsMaximized();
-  bool is_maximized = false;
-  EXPECT_TRUE(root_dict->GetBoolean(kBrowserWindowPlacement + ".maximized",
-      &is_maximized));
-  EXPECT_EQ(is_maximized, is_window_maximized);
-}
-#endif

chrome/browser/profiles/profile_impl.cc

@@ -851,11 +851,10 @@ void ProfileImpl::OnPrefsLoaded(bool success) {
     return;
   }
 
-  // TODO(mirandac): remove migration code after 6 months (crbug.com/69995).
+  // Migrate obsolete prefs.
   if (g_browser_process->local_state())
-    chrome::MigrateBrowserPrefs(this, g_browser_process->local_state());
-  // TODO(ivankr): remove cleanup code eventually (crbug.com/165672).
-  chrome::MigrateUserPrefs(this);
+    chrome::MigrateObsoleteBrowserPrefs(this, g_browser_process->local_state());
+  chrome::MigrateObsoleteProfilePrefs(this);
 
   // |kSessionExitType| was added after |kSessionExitedCleanly|. If the pref
   // value is empty fallback to checking for |kSessionExitedCleanly|.

chrome/chrome_browser_chromeos.gypi

@@ -421,8 +421,6 @@
         'browser/chromeos/login/auth/chrome_login_performer.h',
         'browser/chromeos/login/chrome_restart_request.cc',
         'browser/chromeos/login/chrome_restart_request.h',
-        'browser/chromeos/login/default_pinned_apps_field_trial.cc',
-        'browser/chromeos/login/default_pinned_apps_field_trial.h',
         'browser/chromeos/login/demo_mode/demo_app_launcher.cc',
         'browser/chromeos/login/demo_mode/demo_app_launcher.h',
         'browser/chromeos/login/easy_unlock/easy_unlock_create_keys_operation.cc',

chrome/common/pref_names.cc

@@ -361,14 +361,6 @@ const char kContextualSearchEnabled[] = "search.contextual_search_enabled";
 // window when the user is attempting to quit. Mac only.
 const char kConfirmToQuitEnabled[] = "browser.confirm_to_quit";
 
-// OBSOLETE.  Enum that specifies whether to enforce a third-party cookie
-// blocking policy.  This has been superseded by kDefaultContentSettings +
-// kBlockThirdPartyCookies.
-// 0 - allow all cookies.
-// 1 - block third-party cookies
-// 2 - block all cookies
-const char kCookieBehavior[] = "security.cookie_behavior";
-
 // Boolean which specifies whether we should ask the user if we should download
 // a file (true) or just download it automatically.
 const char kPromptForDownload[] = "download.prompt_for_download";
@@ -376,18 +368,10 @@ const char kPromptForDownload[] = "download.prompt_for_download";
 // A boolean pref set to true if we're using Link Doctor error pages.
 const char kAlternateErrorPagesEnabled[] = "alternate_error_pages.enabled";
 
-// OBSOLETE: new pref now stored with user prefs instead of profile, as
-// kDnsPrefetchingStartupList.
-const char kDnsStartupPrefetchList[] = "StartupDNSPrefetchList";
-
 // An adaptively identified list of domain names to be pre-fetched during the
 // next startup, based on what was actually needed during this startup.
 const char kDnsPrefetchingStartupList[] = "dns_prefetching.startup_list";
 
-// OBSOLETE: new pref now stored with user prefs instead of profile, as
-// kDnsPrefetchingHostReferralList.
-const char kDnsHostReferralList[] = "HostReferralList";
-
 // A list of host names used to fetch web pages, and their commonly used
 // sub-resource hostnames (and expected latency benefits from pre-resolving, or
 // preconnecting to, such sub-resource hostnames).
@@ -421,16 +405,6 @@ const char kLastPolicyCheckTime[] = "policy.last_policy_check_time";
 const char kInstantUIZeroSuggestUrlPrefix[] =
     "instant_ui.zero_suggest_url_prefix";
 
-// Used to migrate preferences from local state to user preferences to
-// enable multiple profiles.
-// BITMASK with possible values (see browser_prefs.cc for enum):
-// 0: No preferences migrated.
-// 1: DNS preferences migrated: kDnsPrefetchingStartupList and HostReferralList
-// 2: Browser window preferences migrated: kDevToolsSplitLocation and
-//    kBrowserWindowPlacement
-const char kMultipleProfilePrefMigration[] =
-    "local_state.multiple_profile_prefs_version";
-
 // A boolean pref set to true if prediction of network actions is allowed.
 // Actions include DNS prefetching, TCP and SSL preconnection, prerendering
 // of web pages, and resource prefetching.

chrome/common/pref_names.h

@@ -149,12 +149,9 @@ extern const char kSearchSuggestEnabled[];
 extern const char kContextualSearchEnabled[];
 #endif
 extern const char kConfirmToQuitEnabled[];
-extern const char kCookieBehavior[];  // OBSOLETE
 extern const char kPromptForDownload[];
 extern const char kAlternateErrorPagesEnabled[];
-extern const char kDnsStartupPrefetchList[];  // OBSOLETE
 extern const char kDnsPrefetchingStartupList[];
-extern const char kDnsHostReferralList[];  // OBSOLETE
 extern const char kDnsPrefetchingHostReferralList[];
 extern const char kDisableSpdy[];
 extern const char kHttpServerProperties[];
@@ -165,7 +162,6 @@ extern const char kDisabledSchemes[];
 extern const char kLastPolicyCheckTime[];
 #endif
 extern const char kInstantUIZeroSuggestUrlPrefix[];
-extern const char kMultipleProfilePrefMigration[];
 extern const char kNetworkPredictionEnabled[];
 extern const char kNetworkPredictionOptions[];
 extern const char kDefaultAppsInstallState[];