Read master preferences before create variations

to allow to migrate variations from master preferences to Local State

Change-Id: I25cca8a8265e2fae57d5d0a13b053e577a9bba11

Bug: 907434
Change-Id: I25cca8a8265e2fae57d5d0a13b053e577a9bba11
Reviewed-on: https://chromium-review.googlesource.com/c/1344097
Commit-Queue: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610635}

chrome/browser/chrome_browser_main.cc

@@ -878,7 +878,9 @@ int ChromeBrowserMainParts::PreCreateThreads() {
   if (result_code_ == service_manager::RESULT_CODE_NORMAL_EXIT) {
 #if !defined(OS_ANDROID)
     // These members must be initialized before exiting this function normally.
+#if !defined(OS_CHROMEOS)
     DCHECK(master_prefs_.get());
+#endif  // !defined(OS_CHROMEOS)
     DCHECK(browser_creator_.get());
 #endif  // !defined(OS_ANDROID)
     for (size_t i = 0; i < chrome_extra_parts_.size(); ++i)
@@ -912,16 +914,17 @@ int ChromeBrowserMainParts::OnLocalStateLoaded(
   }
 #endif  // defined(OS_WIN)
 
-#if !defined(OS_ANDROID)
-  master_prefs_ = std::make_unique<first_run::MasterPrefs>();
-#endif
-
   if (browser_process_->actual_locale().empty()) {
     *failed_to_load_resource_bundle = true;
     return chrome::RESULT_CODE_MISSING_DATA;
   }
 
-  const int apply_first_run_result = ApplyFirstRunPrefs();
+#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
+  master_prefs_ = chrome_feature_list_creator_->TakeMasterPrefs();
+#endif
+
+  const int apply_first_run_result =
+      chrome_feature_list_creator_->master_prefs_apply_result();
   if (apply_first_run_result != service_manager::RESULT_CODE_NORMAL_EXIT)
     return apply_first_run_result;
 
@@ -935,59 +938,6 @@ int ChromeBrowserMainParts::OnLocalStateLoaded(
   return service_manager::RESULT_CODE_NORMAL_EXIT;
 }
 
-int ChromeBrowserMainParts::ApplyFirstRunPrefs() {
-// Android does first run in Java instead of native.
-// Chrome OS has its own out-of-box-experience code.
-#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
-  // On first run, we need to process the predictor preferences before the
-  // browser's profile_manager object is created, but after ResourceBundle
-  // is initialized.
-  if (!first_run::IsChromeFirstRun())
-    return service_manager::RESULT_CODE_NORMAL_EXIT;
-
-  first_run::ProcessMasterPreferencesResult pmp_result =
-      first_run::ProcessMasterPreferences(user_data_dir_, master_prefs_.get());
-  if (pmp_result == first_run::EULA_EXIT_NOW)
-    return chrome::RESULT_CODE_EULA_REFUSED;
-
-  // TODO(macourteau): refactor preferences that are copied from
-  // master_preferences into local_state, as a "local_state" section in
-  // master preferences. If possible, a generic solution would be preferred
-  // over a copy one-by-one of specific preferences. Also see related TODO
-  // in first_run.h.
-
-  PrefService* local_state = g_browser_process->local_state();
-  // Store the initial VariationsService seed in local state, if it exists
-  // in master prefs.
-  if (!master_prefs_->compressed_variations_seed.empty()) {
-    local_state->SetString(variations::prefs::kVariationsCompressedSeed,
-                           master_prefs_->compressed_variations_seed);
-    if (!master_prefs_->variations_seed_signature.empty()) {
-      local_state->SetString(variations::prefs::kVariationsSeedSignature,
-                             master_prefs_->variations_seed_signature);
-    }
-    // Set the variation seed date to the current system time. If the user's
-    // clock is incorrect, this may cause some field trial expiry checks to
-    // not do the right thing until the next seed update from the server,
-    // when this value will be updated.
-    local_state->SetInt64(variations::prefs::kVariationsSeedDate,
-                          base::Time::Now().ToInternalValue());
-  }
-
-  if (!master_prefs_->suppress_default_browser_prompt_for_version.empty()) {
-    local_state->SetString(
-        prefs::kBrowserSuppressDefaultBrowserPrompt,
-        master_prefs_->suppress_default_browser_prompt_for_version);
-  }
-
-#if defined(OS_WIN)
-  if (!master_prefs_->welcome_page_on_os_upgrade_enabled)
-    local_state->SetBoolean(prefs::kWelcomePageOnOSUpgradeEnabled, false);
-#endif
-#endif  // !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
-  return service_manager::RESULT_CODE_NORMAL_EXIT;
-}
-
 int ChromeBrowserMainParts::PreCreateThreadsImpl() {
   TRACE_EVENT0("startup", "ChromeBrowserMainParts::PreCreateThreadsImpl")
   run_message_loop_ = false;

chrome/browser/chrome_browser_main.h

@@ -123,11 +123,6 @@ class ChromeBrowserMainParts : public content::BrowserMainParts {
   // if the ResourceBundle couldn't be loaded.
   int OnLocalStateLoaded(bool* failed_to_load_resource_bundle);
 
-  // Applies any preferences (to local state) needed for first run. This is
-  // always called and early outs if not first-run. Return value is an exit
-  // status, RESULT_CODE_NORMAL_EXIT indicates success.
-  int ApplyFirstRunPrefs();
-
   // Methods for Main Message Loop -------------------------------------------
 
   int PreCreateThreadsImpl();

chrome/browser/first_run/first_run_browsertest.cc

@@ -12,6 +12,7 @@
 #include "base/run_loop.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/metrics/histogram_tester.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/component_loader.h"
@@ -28,6 +29,9 @@
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/prefs/pref_service.h"
 #include "components/user_prefs/user_prefs.h"
+#include "components/variations/metrics.h"
+#include "components/variations/pref_names.h"
+#include "components/variations/variations_switches.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/test/browser_test_utils.h"
@@ -301,4 +305,56 @@ INSTANTIATE_TEST_CASE_P(
         chrome_prefs::internals::
             kSettingsEnforcementGroupEnforceAlwaysWithExtensionsAndDSE));
 
+#define COMPRESSED_SEED_TEST_VALUE \
+  "H4sICMNRYFcAA3NlZWRfYmluAOPSMEwxsjQxM0lLMk4xt0hLMzQ1NUs1TTI1NUw2MzExT05KNj" \
+  "dJNU1LMRDay8glH+rrqBual5mWX5SbWVKpG1KUmZija2igG5BalJyaVyLRMGfSUlYLRif2lNS0" \
+  "xNKcEi9uLhhTgNGLh4sjvSi/tCDewBCFZ4TCM0bhmaDwTFF4Zig8cxSeBQrPUoARAEVeJPrqAAAA"
+#define SEED_SIGNATURE_TEST_VALUE \
+  "MEQCIDD1IVxjzWYncun+9IGzqYjZvqxxujQEayJULTlbTGA/AiAr0oVmEgVUQZBYq5VLOSvy96" \
+  "JkMYgzTkHPwbv7K/CmgA=="
+
+constexpr char kCompressedSeedTestValue[] = COMPRESSED_SEED_TEST_VALUE;
+constexpr char kSignatureValue[] = SEED_SIGNATURE_TEST_VALUE;
+
+extern const char kWithVariationsPrefs[] =
+    "{\n"
+    "  \"variations_compressed_seed\": \"" COMPRESSED_SEED_TEST_VALUE "\",\n"
+    "  \"variations_seed_signature\": \"" SEED_SIGNATURE_TEST_VALUE "\"\n"
+    "}\n";
+
+class FirstRunMasterPrefsVariationsSeedTest
+    : public FirstRunMasterPrefsBrowserTestT<kWithVariationsPrefs> {
+ public:
+  FirstRunMasterPrefsVariationsSeedTest() {
+    base::CommandLine::ForCurrentProcess()->AppendSwitch(
+        variations::switches::kDisableFieldTrialTestingConfig);
+  }
+  ~FirstRunMasterPrefsVariationsSeedTest() override = default;
+
+ protected:
+  base::HistogramTester histogram_tester_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(FirstRunMasterPrefsVariationsSeedTest);
+};
+
+#undef COMPRESSED_SEED_TEST_VALUE
+#undef SEED_SIGNATURE_TEST_VALUE
+
+IN_PROC_BROWSER_TEST_F(FirstRunMasterPrefsVariationsSeedTest, Test) {
+  // Tests variation migration from master_preferences to local_state.
+  EXPECT_EQ(kCompressedSeedTestValue,
+            g_browser_process->local_state()->GetString(
+                variations::prefs::kVariationsCompressedSeed));
+  EXPECT_EQ(kSignatureValue, g_browser_process->local_state()->GetString(
+                                 variations::prefs::kVariationsSeedSignature));
+
+  // Verify variations loaded in VariationsService by metrics.
+  histogram_tester_.ExpectUniqueSample("Variations.SeedLoadResult",
+                                       variations::StoreSeedResult::SUCCESS, 1);
+  histogram_tester_.ExpectUniqueSample(
+      "Variations.LoadSeedSignature",
+      variations::VerifySignatureResult::VALID_SIGNATURE, 1);
+}
+
 #endif  // !defined(OS_CHROMEOS)

chrome/browser/metrics/chrome_feature_list_creator.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/metrics/chrome_feature_list_creator.h"
 
 #include <set>
+#include <vector>
 
 #include "base/base_switches.h"
 #include "base/feature_list.h"
@@ -23,6 +24,7 @@
 #include "chrome/browser/prefs/browser_prefs.h"
 #include "chrome/browser/prefs/chrome_pref_service_factory.h"
 #include "chrome/common/chrome_paths.h"
+#include "chrome/common/chrome_result_codes.h"
 #include "chrome/common/pref_names.h"
 #include "components/flags_ui/flags_ui_pref_names.h"
 #include "components/flags_ui/pref_service_flags_storage.h"
@@ -38,6 +40,7 @@
 #include "components/variations/pref_names.h"
 #include "components/variations/service/variations_service.h"
 #include "components/variations/variations_crash_keys.h"
+#include "services/service_manager/embedder/result_codes.h"
 #include "ui/base/resource/resource_bundle.h"
 
 #if defined(OS_CHROMEOS)
@@ -76,6 +79,7 @@ void ChromeFeatureListCreator::CreateFeatureList() {
   CreatePrefService();
   ConvertFlagsToSwitches();
   CreateMetricsServices();
+  SetupMasterPrefs();
   SetupFieldTrials();
 }
 
@@ -103,6 +107,13 @@ ChromeFeatureListCreator::TakePrefServiceFactory() {
   return std::move(pref_service_factory_);
 }
 
+#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
+std::unique_ptr<first_run::MasterPrefs>
+ChromeFeatureListCreator::TakeMasterPrefs() {
+  return std::move(master_prefs_);
+}
+#endif
+
 void ChromeFeatureListCreator::CreatePrefService() {
   base::FilePath local_state_file;
   bool result =
@@ -194,3 +205,64 @@ void ChromeFeatureListCreator::CreateMetricsServices() {
       std::make_unique<metrics_services_manager::MetricsServicesManager>(
           std::move(client));
 }
+
+void ChromeFeatureListCreator::SetupMasterPrefs() {
+  master_prefs_apply_result_ = ApplyFirstRunPrefs();
+}
+
+int ChromeFeatureListCreator::ApplyFirstRunPrefs() {
+// Android does first run in Java instead of native.
+// Chrome OS has its own out-of-box-experience code.
+#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
+  master_prefs_ = std::make_unique<first_run::MasterPrefs>();
+  // On first run, we need to process the predictor preferences before the
+  // browser's profile_manager object is created, but after ResourceBundle
+  // is initialized.
+  if (!first_run::IsChromeFirstRun())
+    return service_manager::RESULT_CODE_NORMAL_EXIT;
+
+  base::FilePath user_data_dir;
+  if (!base::PathService::Get(chrome::DIR_USER_DATA, &user_data_dir))
+    return chrome::RESULT_CODE_MISSING_DATA;
+
+  first_run::ProcessMasterPreferencesResult pmp_result =
+      first_run::ProcessMasterPreferences(user_data_dir, master_prefs_.get());
+  if (pmp_result == first_run::EULA_EXIT_NOW)
+    return chrome::RESULT_CODE_EULA_REFUSED;
+
+  // TODO(macourteau): refactor preferences that are copied from
+  // master_preferences into local_state, as a "local_state" section in
+  // master preferences. If possible, a generic solution would be preferred
+  // over a copy one-by-one of specific preferences. Also see related TODO
+  // in first_run.h.
+
+  // Store the initial VariationsService seed in local state, if it exists
+  // in master prefs.
+  if (!master_prefs_->compressed_variations_seed.empty()) {
+    local_state_->SetString(variations::prefs::kVariationsCompressedSeed,
+                            master_prefs_->compressed_variations_seed);
+    if (!master_prefs_->variations_seed_signature.empty()) {
+      local_state_->SetString(variations::prefs::kVariationsSeedSignature,
+                              master_prefs_->variations_seed_signature);
+    }
+    // Set the variation seed date to the current system time. If the user's
+    // clock is incorrect, this may cause some field trial expiry checks to
+    // not do the right thing until the next seed update from the server,
+    // when this value will be updated.
+    local_state_->SetInt64(variations::prefs::kVariationsSeedDate,
+                           base::Time::Now().ToInternalValue());
+  }
+
+  if (!master_prefs_->suppress_default_browser_prompt_for_version.empty()) {
+    local_state_->SetString(
+        prefs::kBrowserSuppressDefaultBrowserPrompt,
+        master_prefs_->suppress_default_browser_prompt_for_version);
+  }
+
+#if defined(OS_WIN)
+  if (!master_prefs_->welcome_page_on_os_upgrade_enabled)
+    local_state_->SetBoolean(prefs::kWelcomePageOnOSUpgradeEnabled, false);
+#endif
+#endif  // !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
+  return service_manager::RESULT_CODE_NORMAL_EXIT;
+}

chrome/browser/metrics/chrome_feature_list_creator.h

@@ -5,8 +5,13 @@
 #ifndef CHROME_BROWSER_METRICS_CHROME_FEATURE_LIST_CREATOR_H_
 #define CHROME_BROWSER_METRICS_CHROME_FEATURE_LIST_CREATOR_H_
 
+#include <memory>
+#include <string>
+
 #include "base/macros.h"
+#include "build/build_config.h"
 #include "chrome/browser/chrome_browser_field_trials.h"
+#include "chrome/browser/first_run/first_run.h"
 #include "chrome/browser/metrics/field_trial_synchronizer.h"
 #include "chrome/browser/policy/chrome_browser_policy_connector.h"
 #include "components/metrics_services_manager/metrics_services_manager.h"
@@ -44,6 +49,16 @@ class ChromeFeatureListCreator {
   std::unique_ptr<policy::ChromeBrowserPolicyConnector>
   TakeChromeBrowserPolicyConnector();
 
+#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
+  std::unique_ptr<first_run::MasterPrefs> TakeMasterPrefs();
+#endif
+
+  // Returns result of |ApplyFirstRunPrefs()|.
+  int master_prefs_apply_result() const {
+    DCHECK_NE(-1, master_prefs_apply_result_);
+    return master_prefs_apply_result_;
+  }
+
   // Passes ownership of the |pref_service_factory_| to the caller.
   std::unique_ptr<prefs::InProcessPrefServiceFactory> TakePrefServiceFactory();
 
@@ -62,6 +77,12 @@ class ChromeFeatureListCreator {
   void ConvertFlagsToSwitches();
   void SetupFieldTrials();
   void CreateMetricsServices();
+  void SetupMasterPrefs();
+
+  // Applies any preferences (to local state) needed for first run. This is
+  // always called and early outs if not first-run. Return value is an exit
+  // status, RESULT_CODE_NORMAL_EXIT indicates success.
+  int ApplyFirstRunPrefs();
 
   // If TakePrefService() is called, the caller will take the ownership
   // of this variable. Stop using this variable afterwards.
@@ -86,6 +107,13 @@ class ChromeFeatureListCreator {
 
   std::unique_ptr<prefs::InProcessPrefServiceFactory> pref_service_factory_;
 
+#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
+  std::unique_ptr<first_run::MasterPrefs> master_prefs_;
+#endif
+
+  // Keeps result of |ApplyFirstRunPrefs()|
+  int master_prefs_apply_result_ = -1;
+
   DISALLOW_COPY_AND_ASSIGN(ChromeFeatureListCreator);
 };