Revamp the MigrateBrowserPrefs and MigrateUserPrefs code.

chrome/browser/prefs/browser_prefs.cc

 // Copyright 2012 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/prefs/browser_prefs.h"
 
 #include <string>
 
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_registry_simple.h"
@@ skipping 539 matching lines @@
 void RegisterLoginProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
   RegisterProfilePrefs(registry);
 
   chromeos::PowerPrefs::RegisterLoginProfilePrefs(registry);
 }
 #endif
 
 void MigrateUserPrefs(Profile* profile) {
   PrefService* prefs = profile->GetPrefs();
 
-  // Cleanup prefs from now-removed protector feature.
-  prefs->ClearPref(kBackupPref);

[gab, 2015/02/20 20:33:57]

Remove kBackupPref's definition higher up in this file.

[rkaplow, 2015/02/20 21:58:08]

Am I good to remove this initialization of the pref?


  // 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);
}

I'm guessing this is fine to remove but want to double check

[gab, 2015/02/23 16:09:37]

Yes, prefs that are still cleared will need to be registered (and in fact now
that I recall, this is why they had to be declared at least at file scope rather
than cleanup method scope, sadly).

[rkaplow, 2015/02/23 23:27:52]

Done.

-
-#if !defined(OS_ANDROID)
+  // 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.
+#if !defined(OS_ANDROID)

[gab, 2015/02/20 20:33:57]

I liked having the comment inside the ifdef, please switch it back.

(same below)

[rkaplow, 2015/02/20 21:58:08]

Done.

I rearranged everything to be ordered by date so here is an obvious
addition/removal flow

   chrome_browser_net::MigrateNetworkPredictionUserPrefs(prefs);
 #endif
 
-  PromoResourceService::MigrateUserPrefs(prefs);
+  // Added 07/2014.
   translate::TranslatePrefs::MigrateUserPrefs(prefs, prefs::kAcceptLanguages);
 
+  // Added 06/2014.
 #if defined(OS_MACOSX) && !defined(OS_IOS)
   autofill::AutofillManager::MigrateUserPrefs(prefs);
 #endif  // defined(OS_MACOSX) && !defined(OS_IOS)
 }
 
 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);

[gab, 2015/02/20 20:33:57]

Also remove all constants cleaned up below from the codebase (all of them should
only have been referred to here).

[rkaplow, 2015/02/23 23:27:52]

should be done.

-  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 08/2014.
 #if !defined(OS_ANDROID)
   local_state->ClearPref(kLegacyProfileResetPromptMemento);

[gab, 2015/02/20 20:33:57]

For prefs that are cleared inline (like this one), please inline the constant in
this method as a static const char[] -- e.g., there is no point leaving globally
defined constants around just to clean them up here.

[rkaplow, 2015/02/20 21:58:08]

This gets used elsewhere,
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/pre...

I think I need to remove it there as well. Can you confirm?

[gab, 2015/02/23 16:09:37]

Oops as mentioned above we'll need keep the file scope definition to be able to
register it, it would make sense however to have a local
RegisterDeprecatedPrefs() method call from RegisterUserPrefs() for all of those
registrations to be grouped together (with similar dates + removal comments).

[rkaplow, 2015/02/23 22:55:03]

There's actually a comment for kLegacyProfileResetPromptMemento at least.
For now I think I will leave to try to cut this CL at once place. We can do what
you're suggesting in a follow up

 #endif
 
-#if defined(OS_CHROMEOS)
-  chromeos::default_pinned_apps_field_trial::MigratePrefs(local_state);
-#endif
-
+  // Added 05/2014.
 #if defined(TOOLKIT_VIEWS)
   MigrateBrowserTabStripPrefs(local_state);
 #endif
 }
 
 // As part of the migration from per-profile to per-partition HostZoomMaps,
 // we need to detect if an existing per-profile set of preferences exist, and
 // if so convert them to be per-partition. We migrate any per-profile zoom
 // level prefs via zoom_level_prefs.
 // Code that updates zoom prefs in the profile prefs store has been removed,
@@ skipping 39 matching lines @@
       host_zoom_dictionary, false /* sanitize_partition_host_zoom_levels */);
 
   // We're done migrating the profile per-host zoom level values, so we clear
   // them all.
   DictionaryPrefUpdate host_zoom_dictionary_update(
       prefs, prefs::kPerHostZoomLevelsDeprecated);
   host_zoom_dictionary_update->Clear();
 }
 
 }  // namespace chrome