From ddb1ab17fe48895fa8d1ef920dfe9150ae664715 Mon Sep 17 00:00:00 2001 From: Xi Han Date: Mon, 27 Aug 2018 22:18:54 +0000 Subject: [PATCH] [FeatureList] Load local state file into a pref_store, and use the pref 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 Reviewed-by: John Abd-El-Malek Commit-Queue: Xi Han Cr-Commit-Position: refs/heads/master@{#586445} --- chrome/app/chrome_main_delegate.cc | 15 +++++- chrome/app/chrome_main_delegate.h | 15 +++++- chrome/browser/BUILD.gn | 2 + chrome/browser/browser_process_impl.cc | 8 ++-- chrome/browser/browser_process_impl.h | 12 ++++- .../browser/browser_process_impl_unittest.cc | 13 ++++- chrome/browser/chrome_browser_main.cc | 22 +++++---- chrome/browser/chrome_browser_main.h | 8 +++- chrome/browser/chrome_browser_main_android.cc | 7 ++- chrome/browser/chrome_browser_main_android.h | 5 +- chrome/browser/chrome_browser_main_linux.cc | 7 ++- chrome/browser/chrome_browser_main_linux.h | 5 +- chrome/browser/chrome_browser_main_mac.h | 5 +- chrome/browser/chrome_browser_main_mac.mm | 7 ++- chrome/browser/chrome_browser_main_posix.cc | 7 ++- chrome/browser/chrome_browser_main_posix.h | 5 +- chrome/browser/chrome_browser_main_win.cc | 7 ++- chrome/browser/chrome_browser_main_win.h | 5 +- .../browser/chrome_content_browser_client.cc | 30 +++++++----- .../browser/chrome_content_browser_client.h | 6 ++- chrome/browser/chrome_feature_list_creator.cc | 47 +++++++++++++++++++ chrome/browser/chrome_feature_list_creator.h | 38 +++++++++++++++ .../chromeos/chrome_browser_main_chromeos.cc | 7 ++- .../chromeos/chrome_browser_main_chromeos.h | 5 +- .../prefs/chrome_pref_service_factory.cc | 20 +++++--- .../prefs/chrome_pref_service_factory.h | 5 +- content/app/content_main_runner_impl.cc | 3 +- content/public/app/content_main_delegate.h | 10 ++-- content/shell/app/shell_main_delegate.cc | 2 +- content/shell/app/shell_main_delegate.h | 2 +- extensions/shell/app/shell_main_delegate.h | 2 +- .../shell/app/shell_main_delegate_mac.mm | 2 +- headless/lib/headless_content_main_delegate.h | 2 +- .../lib/headless_content_main_delegate_mac.mm | 2 +- 34 files changed, 263 insertions(+), 75 deletions(-) create mode 100644 chrome/browser/chrome_feature_list_creator.cc create mode 100644 chrome/browser/chrome_feature_list_creator.h diff --git a/chrome/app/chrome_main_delegate.cc b/chrome/app/chrome_main_delegate.cc index 131e0fa62b03db..b4c6d9bc5b8335 100644 --- a/chrome/app/chrome_main_delegate.cc +++ b/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(); + chrome_content_browser_client_ = std::make_unique( - 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() diff --git a/chrome/app/chrome_main_delegate.h b/chrome/app/chrome_main_delegate.h index 8931381b45002e..59d8d1b8db9255 100644 --- a/chrome/app/chrome_main_delegate.h +++ b/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 service_manifest_data_pack_; +#if !defined(CHROME_MULTIPLE_DLL_CHILD) + std::unique_ptr chrome_feature_list_creator_; +#endif + DISALLOW_COPY_AND_ASSIGN(ChromeMainDelegate); }; diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn index 2ac9ab73e9f15d..3e8b5fdb9bba50 100644 --- a/chrome/browser/BUILD.gn +++ b/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", diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc index a5c171d5856997..4be2ac1a1a59a1 100644 --- a/chrome/browser/browser_process_impl.cc +++ b/chrome/browser/browser_process_impl.cc @@ -215,8 +215,10 @@ rappor::RapporService* GetBrowserRapporService() { return nullptr; } -BrowserProcessImpl::BrowserProcessImpl() - : pref_service_factory_( +BrowserProcessImpl::BrowserProcessImpl( + scoped_refptr user_pref_store) + : user_pref_store_(std::move(user_pref_store)), + pref_service_factory_( std::make_unique()) { g_browser_process = this; platform_part_ = std::make_unique(); @@ -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()); diff --git a/chrome/browser/browser_process_impl.h b/chrome/browser/browser_process_impl.h index 6bfe753fb70a13..f2f961c30d2770 100644 --- a/chrome/browser/browser_process_impl.h +++ b/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 user_pref_store); ~BrowserProcessImpl() override; // Called to complete initialization. @@ -318,6 +323,11 @@ class BrowserProcessImpl : public BrowserProcess, scoped_refptr 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 user_pref_store_; + // Ensures that the observers of plugin/print disable/enable state // notifications are properly added and removed. PrefChangeRegistrar pref_change_registrar_; diff --git a/chrome/browser/browser_process_impl_unittest.cc b/chrome/browser/browser_process_impl_unittest.cc index 3a7af6450b0e22..75d5d38b690dd1 100644 --- a/chrome/browser/browser_process_impl_unittest.cc +++ b/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(); + chrome_feature_list_creator_->CreatePrefServiceForTesting(); + + browser_process_impl_ = std::make_unique( + chrome_feature_list_creator_->GetPrefStore()); browser_process_impl_->SetApplicationLocale("en"); } @@ -98,6 +106,7 @@ class BrowserProcessImplTest : public ::testing::Test { std::unique_ptr io_thread_; base::CommandLine command_line_; std::unique_ptr service_manager_context_; + std::unique_ptr chrome_feature_list_creator_; std::unique_ptr browser_process_impl_; }; diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc index a1b23ae7dd2781..14e00dd7f9c833 100644 --- a/chrome/browser/chrome_browser_main.cc +++ b/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 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 data_pack) + std::unique_ptr 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(); + // Create BrowserProcess in PreEarlyInitialization() so that we can load + // field trials (and all it depends upon). + browser_process_ = std::make_unique( + chrome_feature_list_creator_->GetPrefStore()); bool failed_to_load_resource_bundle = false; const int load_local_state_result = diff --git a/chrome/browser/chrome_browser_main.h b/chrome/browser/chrome_browser_main.h index 4d0e815c2195d8..b31d95a4144258 100644 --- a/chrome/browser/chrome_browser_main.h +++ b/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 data_pack); + ChromeBrowserMainParts(const content::MainFunctionParams& parameters, + std::unique_ptr 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 service_manifest_data_pack_; + ChromeFeatureListCreator* chrome_feature_list_creator_; + DISALLOW_COPY_AND_ASSIGN(ChromeBrowserMainParts); }; diff --git a/chrome/browser/chrome_browser_main_android.cc b/chrome/browser/chrome_browser_main_android.cc index fa700a5b78b124..2c5148b7b1d6b4 100644 --- a/chrome/browser/chrome_browser_main_android.cc +++ b/chrome/browser/chrome_browser_main_android.cc @@ -35,8 +35,11 @@ ChromeBrowserMainPartsAndroid::ChromeBrowserMainPartsAndroid( const content::MainFunctionParams& parameters, - std::unique_ptr data_pack) - : ChromeBrowserMainParts(parameters, std::move(data_pack)) {} + std::unique_ptr data_pack, + ChromeFeatureListCreator* chrome_feature_list_creator) + : ChromeBrowserMainParts(parameters, + std::move(data_pack), + chrome_feature_list_creator) {} ChromeBrowserMainPartsAndroid::~ChromeBrowserMainPartsAndroid() { } diff --git a/chrome/browser/chrome_browser_main_android.h b/chrome/browser/chrome_browser_main_android.h index 7cf7c8cbccd7b9..74516a7cbf00cc 100644 --- a/chrome/browser/chrome_browser_main_android.h +++ b/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 data_pack); + std::unique_ptr data_pack, + ChromeFeatureListCreator* chrome_feature_list_creator); ~ChromeBrowserMainPartsAndroid() override; // content::BrowserMainParts overrides. diff --git a/chrome/browser/chrome_browser_main_linux.cc b/chrome/browser/chrome_browser_main_linux.cc index ab9bf625af11df..829f522f6be781 100644 --- a/chrome/browser/chrome_browser_main_linux.cc +++ b/chrome/browser/chrome_browser_main_linux.cc @@ -35,8 +35,11 @@ ChromeBrowserMainPartsLinux::ChromeBrowserMainPartsLinux( const content::MainFunctionParams& parameters, - std::unique_ptr data_pack) - : ChromeBrowserMainPartsPosix(parameters, std::move(data_pack)) {} + std::unique_ptr data_pack, + ChromeFeatureListCreator* chrome_feature_list_creator) + : ChromeBrowserMainPartsPosix(parameters, + std::move(data_pack), + chrome_feature_list_creator) {} ChromeBrowserMainPartsLinux::~ChromeBrowserMainPartsLinux() { } diff --git a/chrome/browser/chrome_browser_main_linux.h b/chrome/browser/chrome_browser_main_linux.h index cfbdda63d02c33..a52adddd65f6f8 100644 --- a/chrome/browser/chrome_browser_main_linux.h +++ b/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 data_pack); + std::unique_ptr data_pack, + ChromeFeatureListCreator* chrome_feature_list_creator); ~ChromeBrowserMainPartsLinux() override; // ChromeBrowserMainParts overrides. diff --git a/chrome/browser/chrome_browser_main_mac.h b/chrome/browser/chrome_browser_main_mac.h index 5cd8d8e0239cc3..8bc804c1e6d9fe 100644 --- a/chrome/browser/chrome_browser_main_mac.h +++ b/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 data_pack); + std::unique_ptr data_pack, + ChromeFeatureListCreator* chrome_feature_list_creator); ~ChromeBrowserMainPartsMac() override; // BrowserParts overrides. diff --git a/chrome/browser/chrome_browser_main_mac.mm b/chrome/browser/chrome_browser_main_mac.mm index 689f151aaa1b23..616f8f8273a677 100644 --- a/chrome/browser/chrome_browser_main_mac.mm +++ b/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 data_pack) - : ChromeBrowserMainPartsPosix(parameters, std::move(data_pack)) {} + std::unique_ptr data_pack, + ChromeFeatureListCreator* chrome_feature_list_creator) + : ChromeBrowserMainPartsPosix(parameters, + std::move(data_pack), + chrome_feature_list_creator) {} ChromeBrowserMainPartsMac::~ChromeBrowserMainPartsMac() { } diff --git a/chrome/browser/chrome_browser_main_posix.cc b/chrome/browser/chrome_browser_main_posix.cc index 94b0a0b2b983b1..a7a9b05d9ec127 100644 --- a/chrome/browser/chrome_browser_main_posix.cc +++ b/chrome/browser/chrome_browser_main_posix.cc @@ -109,8 +109,11 @@ void ExitHandler::Exit() { ChromeBrowserMainPartsPosix::ChromeBrowserMainPartsPosix( const content::MainFunctionParams& parameters, - std::unique_ptr data_pack) - : ChromeBrowserMainParts(parameters, std::move(data_pack)) {} + std::unique_ptr 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(); diff --git a/chrome/browser/chrome_browser_main_posix.h b/chrome/browser/chrome_browser_main_posix.h index 334a2fc6567e8d..f5d497ae358693 100644 --- a/chrome/browser/chrome_browser_main_posix.h +++ b/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 data_pack); + std::unique_ptr data_pack, + ChromeFeatureListCreator* chrome_feature_list_creator); // content::BrowserMainParts overrides. int PreEarlyInitialization() override; diff --git a/chrome/browser/chrome_browser_main_win.cc b/chrome/browser/chrome_browser_main_win.cc index cc39c3d7e54029..ea4e613ed3d454 100644 --- a/chrome/browser/chrome_browser_main_win.cc +++ b/chrome/browser/chrome_browser_main_win.cc @@ -457,8 +457,11 @@ int DoUninstallTasks(bool chrome_still_running) { ChromeBrowserMainPartsWin::ChromeBrowserMainPartsWin( const content::MainFunctionParams& parameters, - std::unique_ptr data_pack) - : ChromeBrowserMainParts(parameters, std::move(data_pack)) {} + std::unique_ptr data_pack, + ChromeFeatureListCreator* chrome_feature_list_creator) + : ChromeBrowserMainParts(parameters, + std::move(data_pack), + chrome_feature_list_creator) {} ChromeBrowserMainPartsWin::~ChromeBrowserMainPartsWin() { } diff --git a/chrome/browser/chrome_browser_main_win.h b/chrome/browser/chrome_browser_main_win.h index 4dc9464dcf85e4..2865cdf4cf51dc 100644 --- a/chrome/browser/chrome_browser_main_win.h +++ b/chrome/browser/chrome_browser_main_win.h @@ -26,9 +26,10 @@ int DoUninstallTasks(bool chrome_still_running); class ChromeBrowserMainPartsWin : public ChromeBrowserMainParts { public: - explicit ChromeBrowserMainPartsWin( + ChromeBrowserMainPartsWin( const content::MainFunctionParams& parameters, - std::unique_ptr data_pack); + std::unique_ptr data_pack, + ChromeFeatureListCreator* chrome_feature_list_creator); ~ChromeBrowserMainPartsWin() override; diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc index fe334be866e239..a794342b6885bc 100644 --- a/chrome/browser/chrome_content_browser_client.cc +++ b/chrome/browser/chrome_content_browser_client.cc @@ -988,8 +988,11 @@ std::unique_ptr CreatePrefService( } // namespace ChromeContentBrowserClient::ChromeContentBrowserClient( - std::unique_ptr data_pack) - : weak_factory_(this) { + std::unique_ptr data_pack, + ChromeFeatureListCreator* chrome_feature_list_creator) + : service_manifest_data_pack_(std::move(data_pack)), + chrome_feature_list_creator_(chrome_feature_list_creator), + weak_factory_(this) { #if BUILDFLAG(ENABLE_PLUGINS) for (size_t i = 0; i < arraysize(kPredefinedAllowedDevChannelOrigins); ++i) allowed_dev_channel_origins_.insert(kPredefinedAllowedDevChannelOrigins[i]); @@ -1017,8 +1020,6 @@ ChromeContentBrowserClient::ChromeContentBrowserClient( gpu_binder_registry_.AddInterface( base::Bind(&metrics::CallStackProfileCollector::Create, metrics::CallStackProfileParams::GPU_PROCESS)); - - service_manifest_data_pack_ = std::move(data_pack); } ChromeContentBrowserClient::~ChromeContentBrowserClient() { @@ -1080,26 +1081,33 @@ content::BrowserMainParts* ChromeContentBrowserClient::CreateBrowserMainParts( // Construct the Main browser parts based on the OS type. #if defined(OS_WIN) main_parts = new ChromeBrowserMainPartsWin( - parameters, std::move(service_manifest_data_pack_)); + parameters, std::move(service_manifest_data_pack_), + chrome_feature_list_creator_); #elif defined(OS_MACOSX) main_parts = new ChromeBrowserMainPartsMac( - parameters, std::move(service_manifest_data_pack_)); + parameters, std::move(service_manifest_data_pack_), + chrome_feature_list_creator_); #elif defined(OS_CHROMEOS) main_parts = new chromeos::ChromeBrowserMainPartsChromeos( - parameters, std::move(service_manifest_data_pack_)); + parameters, std::move(service_manifest_data_pack_), + chrome_feature_list_creator_); #elif defined(OS_LINUX) main_parts = new ChromeBrowserMainPartsLinux( - parameters, std::move(service_manifest_data_pack_)); + parameters, std::move(service_manifest_data_pack_), + chrome_feature_list_creator_); #elif defined(OS_ANDROID) main_parts = new ChromeBrowserMainPartsAndroid( - parameters, std::move(service_manifest_data_pack_)); + parameters, std::move(service_manifest_data_pack_), + chrome_feature_list_creator_); #elif defined(OS_POSIX) main_parts = new ChromeBrowserMainPartsPosix( - parameters, std::move(service_manifest_data_pack_)); + parameters, std::move(service_manifest_data_pack_), + chrome_feature_list_creator_); #else NOTREACHED(); main_parts = new ChromeBrowserMainParts( - parameters, std::move(service_manifest_data_pack_)); + parameters, std::move(service_manifest_data_pack_), + chrome_feature_list_creator_); #endif chrome::AddProfilesExtraParts(main_parts); diff --git a/chrome/browser/chrome_content_browser_client.h b/chrome/browser/chrome_content_browser_client.h index 2ab8cbb42c827c..a063582d9d4333 100644 --- a/chrome/browser/chrome_content_browser_client.h +++ b/chrome/browser/chrome_content_browser_client.h @@ -18,6 +18,7 @@ #include "base/memory/ref_counted.h" #include "base/memory/weak_ptr.h" #include "build/build_config.h" +#include "chrome/browser/chrome_feature_list_creator.h" #include "chrome/browser/chrome_service.h" #include "content/public/browser/content_browser_client.h" #include "content/public/common/resource_type.h" @@ -67,7 +68,8 @@ class Origin; class ChromeContentBrowserClient : public content::ContentBrowserClient { public: explicit ChromeContentBrowserClient( - std::unique_ptr data_pack = nullptr); + std::unique_ptr data_pack = nullptr, + ChromeFeatureListCreator* chrome_feature_list_creator = nullptr); ~ChromeContentBrowserClient() override; // TODO(https://crbug.com/787567): This file is about calls from content/ out @@ -575,6 +577,8 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient { std::unique_ptr service_manifest_data_pack_; + ChromeFeatureListCreator* chrome_feature_list_creator_; + base::WeakPtrFactory weak_factory_; DISALLOW_COPY_AND_ASSIGN(ChromeContentBrowserClient); diff --git a/chrome/browser/chrome_feature_list_creator.cc b/chrome/browser/chrome_feature_list_creator.cc new file mode 100644 index 00000000000000..44ba341d99db21 --- /dev/null +++ b/chrome/browser/chrome_feature_list_creator.cc @@ -0,0 +1,47 @@ +// Copyright 2018 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/chrome_feature_list_creator.h" + +#include "base/files/file_path.h" +#include "base/files/file_util.h" +#include "base/path_service.h" +#include "chrome/common/chrome_paths.h" +#include "components/prefs/json_pref_store.h" +#include "components/prefs/pref_registry.h" +#include "components/prefs/pref_registry_simple.h" +#include "components/prefs/pref_service_factory.h" +#include "components/variations/pref_names.h" + +ChromeFeatureListCreator::ChromeFeatureListCreator() = default; +ChromeFeatureListCreator::~ChromeFeatureListCreator() = default; + +void ChromeFeatureListCreator::CreatePrefService() { + base::FilePath local_state_file; + bool result = + base::PathService::Get(chrome::FILE_LOCAL_STATE, &local_state_file); + DCHECK(result); + + pref_store_ = base::MakeRefCounted(local_state_file); + pref_store_->ReadPrefs(); + + PrefServiceFactory factory; + factory.set_user_prefs(pref_store_); + + scoped_refptr registry = new PrefRegistrySimple(); + simple_local_state_ = factory.Create(registry); +} + +scoped_refptr ChromeFeatureListCreator::GetPrefStore() { + return pref_store_; +} + +void ChromeFeatureListCreator::CreateFeatureList() { + CreatePrefService(); + // TODO(hanxi): Add implementation to create feature list. +} + +void ChromeFeatureListCreator::CreatePrefServiceForTesting() { + CreatePrefService(); +} diff --git a/chrome/browser/chrome_feature_list_creator.h b/chrome/browser/chrome_feature_list_creator.h new file mode 100644 index 00000000000000..9d8b0f80ec16ba --- /dev/null +++ b/chrome/browser/chrome_feature_list_creator.h @@ -0,0 +1,38 @@ +// Copyright 2018 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. + +#ifndef CHROME_BROWSER_CHROME_FEATURE_LIST_CREATOR_H_ +#define CHROME_BROWSER_CHROME_FEATURE_LIST_CREATOR_H_ + +#include "base/macros.h" +#include "components/prefs/pref_service.h" + +// Responsible for creating feature list and all its necessary parameters. +// This class is currently WIP and doesn't do what it's meant to do. +// TODO(hanxi): Finish implementation, https://crbug.com/848615. +class ChromeFeatureListCreator { + public: + ChromeFeatureListCreator(); + ~ChromeFeatureListCreator(); + + // Gets the pref store that is used to create feature list. + scoped_refptr GetPrefStore(); + + // Initializes all necessary parameters to create the feature list and calls + // base::FeatureList::SetInstance() to set the global instance. + void CreateFeatureList(); + + void CreatePrefServiceForTesting(); + + private: + void CreatePrefService(); + + scoped_refptr pref_store_; + + std::unique_ptr simple_local_state_; + + DISALLOW_COPY_AND_ASSIGN(ChromeFeatureListCreator); +}; + +#endif // CHROME_BROWSER_CHROME_FEATURE_LIST_CREATOR_H_ diff --git a/chrome/browser/chromeos/chrome_browser_main_chromeos.cc b/chrome/browser/chromeos/chrome_browser_main_chromeos.cc index 2ef649e859dfdd..9b622394215754 100644 --- a/chrome/browser/chromeos/chrome_browser_main_chromeos.cc +++ b/chrome/browser/chromeos/chrome_browser_main_chromeos.cc @@ -537,8 +537,11 @@ class SystemTokenCertDBInitializer { ChromeBrowserMainPartsChromeos::ChromeBrowserMainPartsChromeos( const content::MainFunctionParams& parameters, - std::unique_ptr data_pack) - : ChromeBrowserMainPartsLinux(parameters, std::move(data_pack)) {} + std::unique_ptr data_pack, + ChromeFeatureListCreator* chrome_feature_list_creator) + : ChromeBrowserMainPartsLinux(parameters, + std::move(data_pack), + chrome_feature_list_creator) {} ChromeBrowserMainPartsChromeos::~ChromeBrowserMainPartsChromeos() { // To be precise, logout (browser shutdown) is not yet done, but the diff --git a/chrome/browser/chromeos/chrome_browser_main_chromeos.h b/chrome/browser/chromeos/chrome_browser_main_chromeos.h index 02effbebb2cf90..8180bbd15a73a3 100644 --- a/chrome/browser/chromeos/chrome_browser_main_chromeos.h +++ b/chrome/browser/chromeos/chrome_browser_main_chromeos.h @@ -68,9 +68,10 @@ class UserActivityController; // src/ash or chrome/browser/ui/ash. class ChromeBrowserMainPartsChromeos : public ChromeBrowserMainPartsLinux { public: - explicit ChromeBrowserMainPartsChromeos( + ChromeBrowserMainPartsChromeos( const content::MainFunctionParams& parameters, - std::unique_ptr data_pack); + std::unique_ptr data_pack, + ChromeFeatureListCreator* chrome_feature_list_creator); ~ChromeBrowserMainPartsChromeos() override; // ChromeBrowserMainParts overrides. diff --git a/chrome/browser/prefs/chrome_pref_service_factory.cc b/chrome/browser/prefs/chrome_pref_service_factory.cc index bc3f0abdef7f83..c22257afc995a8 100644 --- a/chrome/browser/prefs/chrome_pref_service_factory.cc +++ b/chrome/browser/prefs/chrome_pref_service_factory.cc @@ -456,14 +456,20 @@ std::unique_ptr CreateLocalState( policy::PolicyService* policy_service, scoped_refptr pref_registry, bool async, - std::unique_ptr delegate) { + std::unique_ptr delegate, + scoped_refptr user_pref_store) { + if (!user_pref_store) { + // The new JsonPrefStore will read content from |pref_filename| in + // PrefServiceSyncableFactory. Errors are ignored. + user_pref_store = base::MakeRefCounted( + pref_filename, std::unique_ptr()); + } sync_preferences::PrefServiceSyncableFactory factory; - PrepareFactory( - &factory, pref_filename, policy_service, - nullptr, // supervised_user_settings - new JsonPrefStore(pref_filename, std::unique_ptr()), - nullptr, // extension_prefs - async); + PrepareFactory(&factory, pref_filename, policy_service, + nullptr, // supervised_user_settings + std::move(user_pref_store), + nullptr, // extension_prefs + async); return factory.Create(std::move(pref_registry), std::move(delegate)); } diff --git a/chrome/browser/prefs/chrome_pref_service_factory.h b/chrome/browser/prefs/chrome_pref_service_factory.h index 8020d4631ecb86..3318afc9909f7f 100644 --- a/chrome/browser/prefs/chrome_pref_service_factory.h +++ b/chrome/browser/prefs/chrome_pref_service_factory.h @@ -64,12 +64,15 @@ extern const char kSettingsEnforcementGroupEnforceAlwaysWithExtensionsAndDSE[]; // guaranteed that in asynchronous version initialization happens after this // function returned. +// |user_pref_store| instance is an existing pref store that the local state +// PrefService uses as its persistent user pref store. std::unique_ptr CreateLocalState( const base::FilePath& pref_filename, policy::PolicyService* policy_service, scoped_refptr pref_registry, bool async, - std::unique_ptr delegate); + std::unique_ptr delegate, + scoped_refptr user_pref_store); std::unique_ptr CreateProfilePrefs( const base::FilePath& pref_filename, diff --git a/content/app/content_main_runner_impl.cc b/content/app/content_main_runner_impl.cc index 58e5e03f3f6585..145dc521064f61 100644 --- a/content/app/content_main_runner_impl.cc +++ b/content/app/content_main_runner_impl.cc @@ -877,7 +877,7 @@ int ContentMainRunnerImpl::Run(bool start_service_manager_only) { base::TaskScheduler::Create("Browser"); } - delegate_->PreContentInitialization(); + delegate_->PreCreateMainMessageLoop(); // Create a MessageLoop if one does not already exist for the current // thread. This thread won't be promoted as BrowserThread::UI until @@ -885,6 +885,7 @@ int ContentMainRunnerImpl::Run(bool start_service_manager_only) { if (!base::MessageLoopCurrentForUI::IsSet()) main_message_loop_ = std::make_unique(); + delegate_->PostEarlyInitialization(); return RunBrowserProcessMain(main_params, delegate_); } #endif // !defined(CHROME_MULTIPLE_DLL_CHILD) diff --git a/content/public/app/content_main_delegate.h b/content/public/app/content_main_delegate.h index 979e25d1c1b9ff..4231027909279b 100644 --- a/content/public/app/content_main_delegate.h +++ b/content/public/app/content_main_delegate.h @@ -125,10 +125,14 @@ class CONTENT_EXPORT ContentMainDelegate { const base::Closure& quit_closure, service_manager::BackgroundServiceManager* service_manager); + // Allows the embedder to perform platform-specific initializatioion before + // creating the main message loop. + virtual void PreCreateMainMessageLoop() {} + // Allows the embedder to perform platform-specific initializatioion. For - // example, things that should be done immediately before the creation of the - // main message loop. - virtual void PreContentInitialization() {} + // example, things that should be done right after TaskScheduler starts and + // the main MessageLoop was installed. + virtual void PostEarlyInitialization() {} protected: friend class ContentClientInitializer; diff --git a/content/shell/app/shell_main_delegate.cc b/content/shell/app/shell_main_delegate.cc index b1e09521e8ad8b..2cd909585ccb3d 100644 --- a/content/shell/app/shell_main_delegate.cc +++ b/content/shell/app/shell_main_delegate.cc @@ -414,7 +414,7 @@ void ShellMainDelegate::InitializeResourceBundle() { #endif } -void ShellMainDelegate::PreContentInitialization() { +void ShellMainDelegate::PreCreateMainMessageLoop() { #if defined(OS_ANDROID) base::CommandLine& command_line = *base::CommandLine::ForCurrentProcess(); if (command_line.HasSwitch(switches::kRunWebTests) || diff --git a/content/shell/app/shell_main_delegate.h b/content/shell/app/shell_main_delegate.h index 8be636128699f9..97373efe723001 100644 --- a/content/shell/app/shell_main_delegate.h +++ b/content/shell/app/shell_main_delegate.h @@ -36,7 +36,7 @@ class ShellMainDelegate : public ContentMainDelegate { #if defined(OS_LINUX) void ZygoteForked() override; #endif - void PreContentInitialization() override; + void PreCreateMainMessageLoop() override; ContentBrowserClient* CreateContentBrowserClient() override; ContentGpuClient* CreateContentGpuClient() override; ContentRendererClient* CreateContentRendererClient() override; diff --git a/extensions/shell/app/shell_main_delegate.h b/extensions/shell/app/shell_main_delegate.h index 9a0776801c1077..1a0a6c1e0db9ac 100644 --- a/extensions/shell/app/shell_main_delegate.h +++ b/extensions/shell/app/shell_main_delegate.h @@ -41,7 +41,7 @@ class ShellMainDelegate : public content::ContentMainDelegate { void ZygoteForked() override; #endif #if defined(OS_MACOSX) - void PreContentInitialization() override; + void PreCreateMainMessageLoop() override; #endif private: diff --git a/extensions/shell/app/shell_main_delegate_mac.mm b/extensions/shell/app/shell_main_delegate_mac.mm index a3bb4a330ab773..b8f12a42ba7d5e 100644 --- a/extensions/shell/app/shell_main_delegate_mac.mm +++ b/extensions/shell/app/shell_main_delegate_mac.mm @@ -8,7 +8,7 @@ namespace extensions { -void ShellMainDelegate::PreContentInitialization() { +void ShellMainDelegate::PreCreateMainMessageLoop() { // Force the NSApplication subclass to be used. [ShellCrApplication sharedApplication]; } diff --git a/headless/lib/headless_content_main_delegate.h b/headless/lib/headless_content_main_delegate.h index a5b63e7812814f..540bb685ae323d 100644 --- a/headless/lib/headless_content_main_delegate.h +++ b/headless/lib/headless_content_main_delegate.h @@ -44,7 +44,7 @@ class HEADLESS_EXPORT HeadlessContentMainDelegate const std::string& process_type, const content::MainFunctionParams& main_function_params) override; #if defined(OS_MACOSX) - void PreContentInitialization() override; + void PreCreateMainMessageLoop() override; #endif content::ContentBrowserClient* CreateContentBrowserClient() override; content::ContentUtilityClient* CreateContentUtilityClient() override; diff --git a/headless/lib/headless_content_main_delegate_mac.mm b/headless/lib/headless_content_main_delegate_mac.mm index afc99df7301152..39211500af0d77 100644 --- a/headless/lib/headless_content_main_delegate_mac.mm +++ b/headless/lib/headless_content_main_delegate_mac.mm @@ -8,7 +8,7 @@ namespace headless { -void HeadlessContentMainDelegate::PreContentInitialization() { +void HeadlessContentMainDelegate::PreCreateMainMessageLoop() { // Force the NSApplication subclass to be used. [HeadlessShellCrApplication sharedApplication]; }