[FeatureList] Load local state file into a pref_store, and use the pr…

…ef store to init local state later. [1/3]

The reason to create local state in two steps is because we need to read some value from the
local state file first, and at that time it's too early to create the full local state object.

We read local state file by creating a simple pref_service from the pref_store, and when
local state is ready, reset all the objects that uses the simple pref_service to use the
full local state.

This is the first step to move feature list creation earlier on
content_main_runner. We need the simple pref service to create feature list.

This CL comes from https://crrev.com/c/1081759. See previous discussion there.

More info is available on design doc
https://docs.google.com/document/d/1czDvrWU5bE9okOiX-uMwJuZ09SzqxTEW3zHAJG11RVI/edit

Bug: 848615, 729596
Change-Id: I3e58768ab68e75d51b9a64155923565823e154e3
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Reviewed-on: https://chromium-review.googlesource.com/1148959
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Xi Han <hanxi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586445}

chrome/app/chrome_main_delegate.cc

@@ -505,6 +505,13 @@ ChromeMainDelegate::ChromeMainDelegate(base::TimeTicks exe_entry_point_ticks) {
 ChromeMainDelegate::~ChromeMainDelegate() {
 }
 
+#if !defined(CHROME_MULTIPLE_DLL_CHILD)
+void ChromeMainDelegate::PostEarlyInitialization() {
+  DCHECK(chrome_feature_list_creator_);
+  chrome_feature_list_creator_->CreateFeatureList();
+}
+#endif
+
 bool ChromeMainDelegate::BasicStartupComplete(int* exit_code) {
 #if defined(OS_CHROMEOS)
   chromeos::BootTimesRecorder::Get()->SaveChromeMainStats();
@@ -1054,9 +1061,13 @@ ChromeMainDelegate::CreateContentBrowserClient() {
 #else
   if (chrome_content_browser_client_ == nullptr) {
     DCHECK(service_manifest_data_pack_);
+    DCHECK(!chrome_feature_list_creator_);
+    chrome_feature_list_creator_ = std::make_unique<ChromeFeatureListCreator>();
+
     chrome_content_browser_client_ =
         std::make_unique<ChromeContentBrowserClient>(
-            std::move(service_manifest_data_pack_));
+            std::move(service_manifest_data_pack_),
+            chrome_feature_list_creator_.get());
   }
   return chrome_content_browser_client_.get();
 #endif
@@ -1139,7 +1150,7 @@ service_manager::ProcessType ChromeMainDelegate::OverrideProcessType() {
   return service_manager::ProcessType::kDefault;
 }
 
-void ChromeMainDelegate::PreContentInitialization() {
+void ChromeMainDelegate::PreCreateMainMessageLoop() {
 #if defined(OS_MACOSX)
   // Tell Cocoa to finish its initialization, which we want to do manually
   // instead of calling NSApplicationMain(). The primary reason is that NSAM()

chrome/app/chrome_main_delegate.h

@@ -15,6 +15,10 @@
 #include "content/public/app/content_main_delegate.h"
 #include "ui/base/resource/data_pack.h"
 
+#if !defined(CHROME_MULTIPLE_DLL_CHILD)
+#include "chrome/browser/chrome_feature_list_creator.h"
+#endif
+
 namespace base {
 class CommandLine;
 }
@@ -35,7 +39,7 @@ class ChromeMainDelegate : public content::ContentMainDelegate {
   ~ChromeMainDelegate() override;
 
  protected:
-  // content::ContentMainDelegate implementation:
+  // content::ContentMainDelegate:
   bool BasicStartupComplete(int* exit_code) override;
   void PreSandboxStartup() override;
   void SandboxInitialized(const std::string& process_type) override;
@@ -56,7 +60,10 @@ class ChromeMainDelegate : public content::ContentMainDelegate {
 #endif
   bool ShouldEnableProfilerRecording() override;
   service_manager::ProcessType OverrideProcessType() override;
-  void PreContentInitialization() override;
+  void PreCreateMainMessageLoop() override;
+#if !defined(CHROME_MULTIPLE_DLL_CHILD)
+  void PostEarlyInitialization() override;
+#endif
 
   content::ContentBrowserClient* CreateContentBrowserClient() override;
   content::ContentGpuClient* CreateContentGpuClient() override;
@@ -78,6 +85,10 @@ class ChromeMainDelegate : public content::ContentMainDelegate {
   // ContentBrowserClient in CreateContentBrowserClient()
   std::unique_ptr<ui::DataPack> service_manifest_data_pack_;
 
+#if !defined(CHROME_MULTIPLE_DLL_CHILD)
+  std::unique_ptr<ChromeFeatureListCreator> chrome_feature_list_creator_;
+#endif
+
   DISALLOW_COPY_AND_ASSIGN(ChromeMainDelegate);
 };
 

chrome/browser/BUILD.gn

@@ -255,6 +255,8 @@ jumbo_split_static_library("browser") {
     "chrome_content_browser_client_parts.h",
     "chrome_device_client.cc",
     "chrome_device_client.h",
+    "chrome_feature_list_creator.cc",
+    "chrome_feature_list_creator.h",
     "chrome_notification_types.h",
     "chrome_quota_permission_context.cc",
     "chrome_quota_permission_context.h",

chrome/browser/browser_process_impl.cc

@@ -215,8 +215,10 @@ rappor::RapporService* GetBrowserRapporService() {
   return nullptr;
 }
 
-BrowserProcessImpl::BrowserProcessImpl()
-    : pref_service_factory_(
+BrowserProcessImpl::BrowserProcessImpl(
+    scoped_refptr<PersistentPrefStore> user_pref_store)
+    : user_pref_store_(std::move(user_pref_store)),
+      pref_service_factory_(
           std::make_unique<prefs::InProcessPrefServiceFactory>()) {
   g_browser_process = this;
   platform_part_ = std::make_unique<BrowserProcessPlatformPart>();
@@ -1118,7 +1120,7 @@ void BrowserProcessImpl::CreateLocalState() {
   delegate->InitPrefRegistry(pref_registry.get());
   local_state_ = chrome_prefs::CreateLocalState(
       local_state_path, policy_service(), std::move(pref_registry), false,
-      std::move(delegate));
+      std::move(delegate), std::move(user_pref_store_));
   DCHECK(local_state_);
 
   sessions::SessionIdGenerator::GetInstance()->Init(local_state_.get());

chrome/browser/browser_process_impl.h

@@ -25,6 +25,7 @@
 #include "chrome/common/buildflags.h"
 #include "components/keep_alive_registry/keep_alive_state_observer.h"
 #include "components/nacl/common/buildflags.h"
+#include "components/prefs/persistent_pref_store.h"
 #include "components/prefs/pref_change_registrar.h"
 #include "extensions/buildflags/buildflags.h"
 #include "media/media_buildflags.h"
@@ -74,7 +75,11 @@ class TabLifecycleUnitSource;
 class BrowserProcessImpl : public BrowserProcess,
                            public KeepAliveStateObserver {
  public:
-  BrowserProcessImpl();
+  // |user_pref_store|: if non-null, will be used as the source (and
+  // destination) of user prefs for Local State instead of loading the JSON file
+  // from disk.
+  explicit BrowserProcessImpl(
+      scoped_refptr<PersistentPrefStore> user_pref_store);
   ~BrowserProcessImpl() override;
 
   // Called to complete initialization.
@@ -318,6 +323,11 @@ class BrowserProcessImpl : public BrowserProcess,
 
   scoped_refptr<DownloadRequestLimiter> download_request_limiter_;
 
+  // A pref store that is created from the Local State file. This is handed-off
+  // to |local_state_| when it's created. It will use it, if non-null, instead
+  // of loading the user prefs from disk.
+  scoped_refptr<PersistentPrefStore> user_pref_store_;
+
   // Ensures that the observers of plugin/print disable/enable state
   // notifications are properly added and removed.
   PrefChangeRegistrar pref_change_registrar_;

chrome/browser/browser_process_impl_unittest.cc

@@ -13,6 +13,7 @@
 #include "base/task/task_scheduler/task_scheduler.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
+#include "chrome/browser/chrome_feature_list_creator.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/webui/ntp/ntp_resource_cache_factory.h"
 #include "chrome/test/base/testing_profile.h"
@@ -30,12 +31,19 @@ class BrowserProcessImplTest : public ::testing::Test {
       : stashed_browser_process_(g_browser_process),
         loop_(base::MessageLoop::TYPE_UI),
         ui_thread_(content::BrowserThread::UI, &loop_),
-        command_line_(base::CommandLine::NO_PROGRAM),
-        browser_process_impl_(new BrowserProcessImpl()) {
+        command_line_(base::CommandLine::NO_PROGRAM) {
     // Create() and StartWithDefaultParams() TaskScheduler in seperate steps to
     // properly simulate the browser process' lifecycle.
     base::TaskScheduler::Create("BrowserProcessImplTest");
     base::SetRecordActionTaskRunner(loop_.task_runner());
+
+    // |chrome_feature_list_creator_->CreatePrefServiceForTesting()| creates a
+    // user pref store, and must occur before creating |browser_process_impl_|.
+    chrome_feature_list_creator_ = std::make_unique<ChromeFeatureListCreator>();
+    chrome_feature_list_creator_->CreatePrefServiceForTesting();
+
+    browser_process_impl_ = std::make_unique<BrowserProcessImpl>(
+        chrome_feature_list_creator_->GetPrefStore());
     browser_process_impl_->SetApplicationLocale("en");
   }
 
@@ -98,6 +106,7 @@ class BrowserProcessImplTest : public ::testing::Test {
   std::unique_ptr<content::TestBrowserThread> io_thread_;
   base::CommandLine command_line_;
   std::unique_ptr<content::TestServiceManagerContext> service_manager_context_;
+  std::unique_ptr<ChromeFeatureListCreator> chrome_feature_list_creator_;
   std::unique_ptr<BrowserProcessImpl> browser_process_impl_;
 };
 

chrome/browser/chrome_browser_main.cc

@@ -20,7 +20,6 @@
 #include "base/debug/crash_logging.h"
 #include "base/debug/debugger.h"
 #include "base/debug/leak_annotations.h"
-#include "base/deferred_sequenced_task_runner.h"
 #include "base/feature_list.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
@@ -32,8 +31,6 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
-#include "base/sequenced_task_runner.h"
-#include "base/single_thread_task_runner.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
@@ -57,6 +54,7 @@
 #include "chrome/browser/browser_process_platform_part.h"
 #include "chrome/browser/chrome_browser_field_trials.h"
 #include "chrome/browser/chrome_browser_main_extra_parts.h"
+#include "chrome/browser/chrome_feature_list_creator.h"
 #include "chrome/browser/component_updater/crl_set_component_installer.h"
 #include "chrome/browser/component_updater/file_type_policies_component_installer.h"
 #include "chrome/browser/component_updater/mei_preload_component_installer.h"
@@ -404,9 +402,9 @@ void InitializeLocalState() {
       registry->RegisterStringPref(language::prefs::kApplicationLocale,
                                    std::string());
       const std::unique_ptr<PrefService> parent_local_state =
-          chrome_prefs::CreateLocalState(parent_profile,
-                                         g_browser_process->policy_service(),
-                                         std::move(registry), false, nullptr);
+          chrome_prefs::CreateLocalState(
+              parent_profile, g_browser_process->policy_service(),
+              std::move(registry), false, nullptr, nullptr);
       // Right now, we only inherit the locale setting from the parent profile.
       local_state->SetString(
           language::prefs::kApplicationLocale,
@@ -782,7 +780,8 @@ const char kMissingLocaleDataMessage[] =
 
 ChromeBrowserMainParts::ChromeBrowserMainParts(
     const content::MainFunctionParams& parameters,
-    std::unique_ptr<ui::DataPack> data_pack)
+    std::unique_ptr<ui::DataPack> data_pack,
+    ChromeFeatureListCreator* chrome_feature_list_creator)
     : parameters_(parameters),
       parsed_command_line_(parameters.command_line),
       result_code_(service_manager::RESULT_CODE_NORMAL_EXIT),
@@ -791,7 +790,9 @@ ChromeBrowserMainParts::ChromeBrowserMainParts(
           !parameters.ui_task),
       profile_(NULL),
       run_message_loop_(true),
-      service_manifest_data_pack_(std::move(data_pack)) {
+      service_manifest_data_pack_(std::move(data_pack)),
+      chrome_feature_list_creator_(chrome_feature_list_creator) {
+  DCHECK(chrome_feature_list_creator_);
   // If we're running tests (ui_task is non-null).
   if (parameters.ui_task)
     browser_defaults::enable_help_app = false;
@@ -989,7 +990,10 @@ int ChromeBrowserMainParts::PreEarlyInitialization() {
   for (size_t i = 0; i < chrome_extra_parts_.size(); ++i)
     chrome_extra_parts_[i]->PreEarlyInitialization();
 
-  browser_process_ = std::make_unique<BrowserProcessImpl>();
+  // Create BrowserProcess in PreEarlyInitialization() so that we can load
+  // field trials (and all it depends upon).
+  browser_process_ = std::make_unique<BrowserProcessImpl>(
+      chrome_feature_list_creator_->GetPrefStore());
 
   bool failed_to_load_resource_bundle = false;
   const int load_local_state_result =

chrome/browser/chrome_browser_main.h

@@ -21,6 +21,7 @@
 
 class BrowserProcessImpl;
 class ChromeBrowserMainExtraParts;
+class ChromeFeatureListCreator;
 class FieldTrialSynchronizer;
 class PrefService;
 class Profile;
@@ -55,8 +56,9 @@ class ChromeBrowserMainParts : public content::BrowserMainParts {
 #endif
 
  protected:
-  explicit ChromeBrowserMainParts(const content::MainFunctionParams& parameters,
-                                  std::unique_ptr<ui::DataPack> data_pack);
+  ChromeBrowserMainParts(const content::MainFunctionParams& parameters,
+                         std::unique_ptr<ui::DataPack> data_pack,
+                         ChromeFeatureListCreator* chrome_feature_list_creator);
 
   // content::BrowserMainParts overrides.
   bool ShouldContentCreateFeatureList() override;
@@ -206,6 +208,8 @@ class ChromeBrowserMainParts : public content::BrowserMainParts {
   // resource bundle gets created.
   std::unique_ptr<ui::DataPack> service_manifest_data_pack_;
 
+  ChromeFeatureListCreator* chrome_feature_list_creator_;
+
   DISALLOW_COPY_AND_ASSIGN(ChromeBrowserMainParts);
 };
 

chrome/browser/chrome_browser_main_android.cc

@@ -35,8 +35,11 @@
 
 ChromeBrowserMainPartsAndroid::ChromeBrowserMainPartsAndroid(
     const content::MainFunctionParams& parameters,
-    std::unique_ptr<ui::DataPack> data_pack)
-    : ChromeBrowserMainParts(parameters, std::move(data_pack)) {}
+    std::unique_ptr<ui::DataPack> data_pack,
+    ChromeFeatureListCreator* chrome_feature_list_creator)
+    : ChromeBrowserMainParts(parameters,
+                             std::move(data_pack),
+                             chrome_feature_list_creator) {}
 
 ChromeBrowserMainPartsAndroid::~ChromeBrowserMainPartsAndroid() {
 }

chrome/browser/chrome_browser_main_android.h

@@ -11,9 +11,10 @@
 
 class ChromeBrowserMainPartsAndroid : public ChromeBrowserMainParts {
  public:
-  explicit ChromeBrowserMainPartsAndroid(
+  ChromeBrowserMainPartsAndroid(
       const content::MainFunctionParams& parameters,
-      std::unique_ptr<ui::DataPack> data_pack);
+      std::unique_ptr<ui::DataPack> data_pack,
+      ChromeFeatureListCreator* chrome_feature_list_creator);
   ~ChromeBrowserMainPartsAndroid() override;
 
   // content::BrowserMainParts overrides.

chrome/browser/chrome_browser_main_linux.cc

@@ -35,8 +35,11 @@
 
 ChromeBrowserMainPartsLinux::ChromeBrowserMainPartsLinux(
     const content::MainFunctionParams& parameters,
-    std::unique_ptr<ui::DataPack> data_pack)
-    : ChromeBrowserMainPartsPosix(parameters, std::move(data_pack)) {}
+    std::unique_ptr<ui::DataPack> data_pack,
+    ChromeFeatureListCreator* chrome_feature_list_creator)
+    : ChromeBrowserMainPartsPosix(parameters,
+                                  std::move(data_pack),
+                                  chrome_feature_list_creator) {}
 
 ChromeBrowserMainPartsLinux::~ChromeBrowserMainPartsLinux() {
 }

chrome/browser/chrome_browser_main_linux.h

@@ -13,9 +13,10 @@
 
 class ChromeBrowserMainPartsLinux : public ChromeBrowserMainPartsPosix {
  public:
-  explicit ChromeBrowserMainPartsLinux(
+  ChromeBrowserMainPartsLinux(
       const content::MainFunctionParams& parameters,
-      std::unique_ptr<ui::DataPack> data_pack);
+      std::unique_ptr<ui::DataPack> data_pack,
+      ChromeFeatureListCreator* chrome_feature_list_creator);
   ~ChromeBrowserMainPartsLinux() override;
 
   // ChromeBrowserMainParts overrides.

chrome/browser/chrome_browser_main_mac.h

@@ -10,9 +10,10 @@
 
 class ChromeBrowserMainPartsMac : public ChromeBrowserMainPartsPosix {
  public:
-  explicit ChromeBrowserMainPartsMac(
+  ChromeBrowserMainPartsMac(
       const content::MainFunctionParams& parameters,
-      std::unique_ptr<ui::DataPack> data_pack);
+      std::unique_ptr<ui::DataPack> data_pack,
+      ChromeFeatureListCreator* chrome_feature_list_creator);
   ~ChromeBrowserMainPartsMac() override;
 
   // BrowserParts overrides.

chrome/browser/chrome_browser_main_mac.mm

@@ -68,8 +68,11 @@ void EnsureMetadataNeverIndexFile(const base::FilePath& user_data_dir) {
 
 ChromeBrowserMainPartsMac::ChromeBrowserMainPartsMac(
     const content::MainFunctionParams& parameters,
-    std::unique_ptr<ui::DataPack> data_pack)
-    : ChromeBrowserMainPartsPosix(parameters, std::move(data_pack)) {}
+    std::unique_ptr<ui::DataPack> data_pack,
+    ChromeFeatureListCreator* chrome_feature_list_creator)
+    : ChromeBrowserMainPartsPosix(parameters,
+                                  std::move(data_pack),
+                                  chrome_feature_list_creator) {}
 
 ChromeBrowserMainPartsMac::~ChromeBrowserMainPartsMac() {
 }

chrome/browser/chrome_browser_main_posix.cc

@@ -109,8 +109,11 @@ void ExitHandler::Exit() {
 
 ChromeBrowserMainPartsPosix::ChromeBrowserMainPartsPosix(
     const content::MainFunctionParams& parameters,
-    std::unique_ptr<ui::DataPack> data_pack)
-    : ChromeBrowserMainParts(parameters, std::move(data_pack)) {}
+    std::unique_ptr<ui::DataPack> data_pack,
+    ChromeFeatureListCreator* chrome_feature_list_creator)
+    : ChromeBrowserMainParts(parameters,
+                             std::move(data_pack),
+                             chrome_feature_list_creator) {}
 
 int ChromeBrowserMainPartsPosix::PreEarlyInitialization() {
   const int result = ChromeBrowserMainParts::PreEarlyInitialization();

chrome/browser/chrome_browser_main_posix.h

@@ -10,9 +10,10 @@
 
 class ChromeBrowserMainPartsPosix : public ChromeBrowserMainParts {
  public:
-  explicit ChromeBrowserMainPartsPosix(
+  ChromeBrowserMainPartsPosix(
       const content::MainFunctionParams& parameters,
-      std::unique_ptr<ui::DataPack> data_pack);
+      std::unique_ptr<ui::DataPack> data_pack,
+      ChromeFeatureListCreator* chrome_feature_list_creator);
 
   // content::BrowserMainParts overrides.
   int PreEarlyInitialization() override;