Remove kDistroDict from Preferences.

chrome/browser/prefs/browser_prefs.cc

Index: chrome/browser/prefs/browser_prefs.cc
diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
index cef77d36b5d63da541ff25c222c5acbd520eda61..61aae71dd6bc97bef8e6775d14bb88ce90666e1e 100644
--- a/chrome/browser/prefs/browser_prefs.cc
+++ b/chrome/browser/prefs/browser_prefs.cc
@@ -112,6 +112,7 @@
 #include "net/http/http_server_properties_manager.h"
 #include "ppapi/features/features.h"
 #include "printing/features/features.h"
+#include "rlz/features/features.h"
 
 #if BUILDFLAG(ENABLE_APP_LIST)
 #include "chrome/browser/apps/drive/drive_app_mapping.h"
@@ -262,37 +263,49 @@ namespace {
 
 #if BUILDFLAG(ENABLE_GOOGLE_NOW)
 // Deprecated 3/2016
-const char kGoogleGeolocationAccessEnabled[] =
+constexpr char kGoogleGeolocationAccessEnabled[] =
     "googlegeolocationaccess.enabled";
 #endif
 
 // Deprecated 4/2016.
-const char kCheckDefaultBrowser[] = "browser.check_default_browser";
+constexpr char kCheckDefaultBrowser[] = "browser.check_default_browser";
 
 // Deprecated 5/2016.
-const char kDesktopSearchRedirectionInfobarShownPref[] =
+constexpr char kDesktopSearchRedirectionInfobarShownPref[] =
     "desktop_search_redirection_infobar_shown";
 
 // Deprecated 7/2016.
-const char kNetworkPredictionEnabled[] = "dns_prefetching.enabled";
-const char kDisableSpdy[] = "spdy.disabled";
+constexpr char kNetworkPredictionEnabled[] = "dns_prefetching.enabled";
+constexpr char kDisableSpdy[] = "spdy.disabled";
 
 // Deprecated 8/2016.
-const char kRecentlySelectedEncoding[] = "profile.recently_selected_encodings";
-const char kStaticEncodings[] = "intl.static_encodings";
+constexpr char kRecentlySelectedEncoding[] =
+    "profile.recently_selected_encodings";
+constexpr char kStaticEncodings[] = "intl.static_encodings";
 
 // Deprecated 9/2016.
-const char kWebKitUsesUniversalDetector[] =
+constexpr char kWebKitUsesUniversalDetector[] =
     "webkit.webprefs.uses_universal_detector";
-const char kWebKitAllowDisplayingInsecureContent[] =
+constexpr char kWebKitAllowDisplayingInsecureContent[] =
     "webkit.webprefs.allow_displaying_insecure_content";
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
 // Deprecated 2/2017.
-const char kToolbarMigratedComponentActionStatus[] =
+constexpr char kToolbarMigratedComponentActionStatus[] =
     "toolbar_migrated_component_action_status";
 #endif
 
+#if BUILDFLAG(ENABLE_RLZ)
+// Migrated out of kDistroDict as of 2/2017.
+constexpr char kDistroRlzPingDelay[] = "ping_delay";
+#endif  // BUILDFLAG(ENABLE_RLZ)
+
+// master_preferences used to be mapped as-is to Preferences on first run but
+// the "distribution" dictionary was never used beyond first run. It is now
+// stripped in first_run.cc prior to applying this mapping. Cleanup for existing
+// Preferences files added here 2/2017.
+constexpr char kDistroDict[] = "distribution";
+
 void DeleteWebRTCIdentityStoreDBOnFileThread(
     const base::FilePath& profile_path) {
   base::DeleteFile(profile_path.Append(
@@ -640,6 +653,8 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
 #if BUILDFLAG(ENABLE_EXTENSIONS)
   registry->RegisterDictionaryPref(kToolbarMigratedComponentActionStatus);
 #endif
+
+  registry->RegisterDictionaryPref(kDistroDict);
 }
 
 void RegisterUserProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
@@ -732,6 +747,20 @@ void MigrateObsoleteProfilePrefs(Profile* profile) {
   // Added 2/2017.
   profile_prefs->ClearPref(kToolbarMigratedComponentActionStatus);
 #endif
+
+  // Added 2/2017.
+  {
+#if BUILDFLAG(ENABLE_RLZ)

[grt (UTC plus 2), 2017/02/24 07:42:21]

i'm not convinced that the pref should not be migrated if !ENABLE_RLZ. a user
with a post-migrated pref won't lose it if we accidentally ship an rlz-free
build for a day, so why should a user that happens to be migrated via this code
on that day lose it?

[Roger Tawa OOO till Jul 10th, 2017/02/24 16:02:17]

What is the chance of that?  It's like saying we accidentally ship a chromium
build instead of a chrome build for a day.  But maybe better safe than sorry. 
Also need to remove ifdefs from master_preferences.cc and its unittest.

[gab, 2017/02/28 19:54:45]

Agreed that we shouldn't worry about shipping a non-RLZ build (and it's not like
this would be destructive if so either, it would just skip the migration for
that one build, which can't need it if it has RLZ disabled so...).

The reason it's easier to ifdef all is that prefs::kRlzPingDelay is behind
ENABLE_RLZ and so this code has to oblige (not ifdef'ing pref would be uncommon
and would allow non-ifdefed code that relies on RLZ being enabled anyways..).

[grt (UTC plus 2), 2017/02/28 20:09:07]

It is destructive -- kDistroDict is cleared on line 762. I agree that it's
difficult to imagine us somehow falling into this trap, but it is a change in
behavior. Prior to this change, ping_delay set via master prefs will be used
if/when a Chrome that uses RLZ picks up prefs created with it. After this
change, ping_delay vanishes during profile load if RLZ isn't supported by
whatever Chrome loads it, and then is gone thereafter.

[gab, 2017/02/28 22:30:41]

Ah right but what you're describing is caused by code in first_run.cc, not here.

I guess if we did ship a build without RLZ, we'd be losing custom ping delays
for users that either (1) go through first run with it or (2) go through one
time migration with it.

That is, it wouldn't affect nearly all users and it would just mean a fallback
to the default value. I'm willing to live with that being the result of skipping
a non RLZ build (which sounds unlikely as rogerta also said).

+    const base::DictionaryValue* distro_dict =
+        profile_prefs->GetDictionary(kDistroDict);
+    int rlz_ping_delay = 0;
+    if (distro_dict &&
+        distro_dict->GetInteger(kDistroRlzPingDelay, &rlz_ping_delay)) {
+      profile_prefs->SetInteger(prefs::kRlzPingDelay, rlz_ping_delay);
+    }
+#endif  // BUILDFLAG(ENABLE_RLZ)
+    profile_prefs->ClearPref(kDistroDict);
+  }
 }
 
 }  // namespace chrome