Revert "Read master preferences before create variations"

This reverts commit 18483d6a.

Reason for revert: Crash in official build due to resourcebundle not being initialized: crbug.com/908259

Original change's description:
> 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}

TBR=sky@chromium.org,asvitkine@chromium.org,hanxi@chromium.org,jochen@chromium.org,boriay@yandex-team.ru

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 907434
Change-Id: Ic48accb1c33bb5206dac265596628d8225ee2683
Reviewed-on: https://chromium-review.googlesource.com/c/1350201Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610737}

chrome/browser/chrome_browser_main.cc

@@ -878,9 +878,7 @@ 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)
@@ -914,17 +912,16 @@ 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;
   }
 
-#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();
+  const int apply_first_run_result = ApplyFirstRunPrefs();
   if (apply_first_run_result != service_manager::RESULT_CODE_NORMAL_EXIT)
     return apply_first_run_result;
 
@@ -938,6 +935,59 @@ 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,6 +123,11 @@ 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,7 +12,6 @@
 #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"
@@ -29,9 +28,6 @@
 #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"
@@ -305,56 +301,4 @@ 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,7 +5,6 @@
 #include "chrome/browser/metrics/chrome_feature_list_creator.h"
 
 #include <set>
-#include <vector>
 
 #include "base/base_switches.h"
 #include "base/feature_list.h"
@@ -24,7 +23,6 @@
 #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"
@@ -40,7 +38,6 @@
 #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)
@@ -79,7 +76,6 @@ void ChromeFeatureListCreator::CreateFeatureList() {
   CreatePrefService();
   ConvertFlagsToSwitches();
   CreateMetricsServices();
-  SetupMasterPrefs();
   SetupFieldTrials();
 }
 
@@ -107,13 +103,6 @@ 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 =
@@ -205,64 +194,3 @@ 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,13 +5,8 @@
 #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"
@@ -49,16 +44,6 @@ 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();
 
@@ -77,12 +62,6 @@ 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.
@@ -107,13 +86,6 @@ 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);
 };