Remove kDistroDict from Preferences.

chrome/browser/first_run/first_run.cc

 // Copyright (c) 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/first_run/first_run.h"
 
 #include <algorithm>
+#include <memory>
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/lazy_instance.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
@@ skipping 44 matching lines @@
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/common/one_shot_event.h"
 #include "google_apis/gaia/gaia_auth_util.h"
+#include "rlz/features/features.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "url/gurl.h"
 
 namespace content {
 class BrowserContext;
 }
 
 using base::UserMetricsAction;
 
 namespace {
@@ skipping 361 matching lines @@
     = LAZY_INSTANCE_INITIALIZER;
 
 // Loads master preferences from the master preference file into the installer
 // master preferences. Returns the pointer to installer::MasterPreferences
 // object if successful; otherwise, returns NULL.
 installer::MasterPreferences* LoadMasterPrefs() {
   base::FilePath master_prefs_path;
   if (!master_prefs_path_for_testing.Get().empty())
     master_prefs_path = master_prefs_path_for_testing.Get();
   else
     master_prefs_path = base::FilePath(first_run::internal::MasterPrefsPath());

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

nit: should have curly braces in on this if/else

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

Done.

   if (master_prefs_path.empty())
     return NULL;
   installer::MasterPreferences* install_prefs =
       new installer::MasterPreferences(master_prefs_path);
   if (!install_prefs->read_from_file()) {
     delete install_prefs;
     return NULL;
   }
 
   return install_prefs;
@@ skipping 23 matching lines @@
 
 namespace first_run {
 namespace internal {
 
 void SetupMasterPrefsFromInstallPrefs(
     const installer::MasterPreferences& install_prefs,
     MasterPrefs* out_prefs) {
   ConvertStringVectorToGURLVector(
       install_prefs.GetFirstRunTabs(), &out_prefs->new_tabs);
 
-  install_prefs.GetInt(installer::master_preferences::kDistroPingDelay,
-                       &out_prefs->ping_delay);
-
   bool value = false;
   if (install_prefs.GetBool(
           installer::master_preferences::kDistroImportSearchPref, &value)) {
     if (value) {
       out_prefs->do_import_items |= importer::SEARCH_ENGINES;
     } else {
       out_prefs->dont_import_items |= importer::SEARCH_ENGINES;
     }
   }
 
@@ skipping 102 matching lines @@
                                      bool force_first_run,
                                      bool no_first_run) {
   return (force_first_run || (!has_sentinel && !no_first_run))
              ? FIRST_RUN_TRUE
              : FIRST_RUN_FALSE;
 }
 
 }  // namespace internal
 
 MasterPrefs::MasterPrefs()
-    : ping_delay(0),
-      homepage_defined(false),
+    : homepage_defined(false),
       do_import_items(0),
       dont_import_items(0),
       make_chrome_default_for_user(false),
       suppress_first_run_default_browser_prompt(false),
-      welcome_page_on_os_upgrade_enabled(true) {
-}
+      welcome_page_on_os_upgrade_enabled(true) {}
 
 MasterPrefs::~MasterPrefs() {}
 
 bool IsChromeFirstRun() {
   if (g_first_run == internal::FIRST_RUN_UNKNOWN) {
     const base::CommandLine* command_line =
         base::CommandLine::ForCurrentProcess();
     g_first_run = internal::DetermineFirstRunState(
         internal::IsFirstRunSentinelPresent(),
         command_line->HasSwitch(switches::kForceFirstRun),
@@ skipping 13 matching lines @@
   // However, user will have chance to modify metrics reporting state during
   // first run.
   return false;
 }
 
 void CreateSentinelIfNeeded() {
   if (IsChromeFirstRun())
     internal::CreateSentinel();
 }
 
-std::string GetPingDelayPrefName() {
-  return base::StringPrintf("%s.%s",
-                            installer::master_preferences::kDistroDict,
-                            installer::master_preferences::kDistroPingDelay);
-}
-
 void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
-  registry->RegisterIntegerPref(GetPingDelayPrefName().c_str(), 0);
+#if BUILDFLAG(ENABLE_RLZ)
+  registry->RegisterIntegerPref(prefs::kRlzPingDelay, 90);

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

this doesn't feel like the right place. rlz isn't specific to first_run. could
you look for a more general place to register the pref? also, i wonder if
local_state isn't a better place for it. maybe outside of scope of this cl, but
how about a comment/todo for later?

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

while 90 has been the default, should it really be 90000 (see other comment
about seconds vs milliseconds)? +rogerga to confirm.

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

See my other comment.  I believe this is a bug that has been around for a long
time.  Since all the docs talk about seconds and existing master prefs use
seconds, lets keep seconds and fix the code to handle it correctly.

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

Could be moved to c/b/rlz/chrome_rlz_tracker_delegate.cc

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

Done.

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

I think it's intentionally tied to the default profile? Either way, I'm
unwilling to do anything to change that...

+#endif
 }
 
 bool SetShowFirstRunBubblePref(FirstRunBubbleOptions show_bubble_option) {
   PrefService* local_state = g_browser_process->local_state();
   if (!local_state)
     return false;
   if (local_state->GetInteger(
           prefs::kShowFirstRunBubbleOption) != FIRST_RUN_BUBBLE_SUPPRESS) {
     // Set the new state as long as the bubble wasn't explicitly suppressed
     // already.
@@ skipping 54 matching lines @@
 }
 
 ProcessMasterPreferencesResult ProcessMasterPreferences(
     const base::FilePath& user_data_dir,
     MasterPrefs* out_prefs) {
   DCHECK(!user_data_dir.empty());
 
   std::unique_ptr<installer::MasterPreferences> install_prefs(
       LoadMasterPrefs());
 
-  // Default value in case master preferences is missing or corrupt, or
-  // ping_delay is missing.
-  out_prefs->ping_delay = 90;
   if (install_prefs.get()) {
     if (!internal::ShowPostInstallEULAIfNeeded(install_prefs.get()))
       return EULA_EXIT_NOW;
 
+    std::unique_ptr<base::DictionaryValue> master_dictionary =
+        install_prefs->master_dictionary().CreateDeepCopy();
+    // The distribution dictionary (and any prefs below it) are never registered
+    // for use in Chrome's PrefService. Strip them from the master dictionary
+    // before mapping it to prefs.
+    master_dictionary->RemoveWithoutPathExpansion(
+        installer::master_preferences::kDistroDict, nullptr);
+
     if (!chrome_prefs::InitializePrefsFromMasterPrefs(
             profiles::GetDefaultProfileDir(user_data_dir),
-            install_prefs->master_dictionary())) {
+            std::move(master_dictionary))) {
       DLOG(ERROR) << "Failed to initialize from master_preferences.";
     }
 
     DoDelayedInstallExtensionsIfNeeded(install_prefs.get());
 
     internal::SetupMasterPrefsFromInstallPrefs(*install_prefs, out_prefs);
   }
 
   return FIRST_RUN_PROCEED;
 }
@@ skipping 94 matching lines @@
   SetShouldDoPersonalDataManagerFirstRun();
 
   internal::DoPostImportPlatformSpecificTasks(profile);
 }
 
 uint16_t auto_import_state() {
   return g_auto_import_state;
 }
 
 }  // namespace first_run