Remove |local_state_task_runner| from BrowserProcessImpl.

The default JsonPrefStore task runner is used instead of injecting one.
Remaining use cases didn't require sharing the task runner except one
(to flush), but CommitPendingWrite()'s async reply API already provides
this functionality and we use it in this CL instead of exposing the
entire task runner (which is also more readable then implicitly
depending on the impl flushing right away on its task runner).

This is another take on hanxi's https://crrev.com/c/1153632
after debugging it locally I realized RundownTaskCounter needs to
observe the notification synchronously (as the WaitableEvent prevents
observing the reply). A nested Runloop is also not suitable (ref.
https://crbug.com/318527 and in code comments).

Note: JsonPrefStore's optional constructor params were flipped since the task
runner is now the most likely optional parameter.

This CL is a precursor to:
https://crrev.com/c/1148959.

R=sky@chromium.org

Bug: 848615, 318527
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;luci.chromium.try:linux_mojo;master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I24c03cb2e49ec667e592d7a78722cdaf0884af36
Reviewed-on: https://chromium-review.googlesource.com/1163628
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581343}

chrome/browser/browser_process_impl.cc

@@ -215,10 +215,8 @@ rappor::RapporService* GetBrowserRapporService() {
   return nullptr;
 }
 
-BrowserProcessImpl::BrowserProcessImpl(
-    base::SequencedTaskRunner* local_state_task_runner)
-    : local_state_task_runner_(local_state_task_runner),
-      pref_service_factory_(
+BrowserProcessImpl::BrowserProcessImpl()
+    : pref_service_factory_(
           std::make_unique<prefs::InProcessPrefServiceFactory>()) {
   g_browser_process = this;
   platform_part_ = std::make_unique<BrowserProcessPlatformPart>();
@@ -471,15 +469,15 @@ class RundownTaskCounter :
  public:
   RundownTaskCounter();
 
-  // Posts a rundown task to |task_runner|, can be invoked an arbitrary number
-  // of times before calling TimedWait.
-  void Post(base::SequencedTaskRunner* task_runner);
+  // Increments |count_| and returns a closure bound to Decrement(). All
+  // closures returned by this RundownTaskCounter's GetRundownClosure() method
+  // must be invoked for TimedWait() to complete its wait without timing
+  // out.
+  base::OnceClosure GetRundownClosure();
 
-  // Waits until the count is zero or |end_time| is reached.
-  // This can only be called once per instance. Returns true if a count of zero
-  // is reached or false if the |end_time| is reached. It is valid to pass an
-  // |end_time| in the past.
-  bool TimedWaitUntil(const base::TimeTicks& end_time);
+  // Waits until the count is zero or |timeout| expires.
+  // This can only be called once per instance.
+  void TimedWait(base::TimeDelta timeout);
 
  private:
   friend class base::RefCountedThreadSafe<RundownTaskCounter>;
@@ -491,28 +489,22 @@ class RundownTaskCounter :
 
   // The count starts at one to defer the possibility of one->zero transitions
   // until TimedWait is called.
-  base::AtomicRefCount count_;
+  base::AtomicRefCount count_{1};
   base::WaitableEvent waitable_event_;
 
   DISALLOW_COPY_AND_ASSIGN(RundownTaskCounter);
 };
 
-RundownTaskCounter::RundownTaskCounter()
-    : count_(1),
-      waitable_event_(base::WaitableEvent::ResetPolicy::MANUAL,
-                      base::WaitableEvent::InitialState::NOT_SIGNALED) {}
+RundownTaskCounter::RundownTaskCounter() = default;
 
-void RundownTaskCounter::Post(base::SequencedTaskRunner* task_runner) {
+base::OnceClosure RundownTaskCounter::GetRundownClosure() {
   // As the count starts off at one, it should never get to zero unless
   // TimedWait has been called.
   DCHECK(!count_.IsZero());
 
   count_.Increment();
 
-  // The task must be non-nestable to guarantee that it runs after all tasks
-  // currently scheduled on |task_runner| have completed.
-  task_runner->PostNonNestableTask(
-      FROM_HERE, base::BindOnce(&RundownTaskCounter::Decrement, this));
+  return base::BindOnce(&RundownTaskCounter::Decrement, this);
 }
 
 void RundownTaskCounter::Decrement() {
@@ -520,20 +512,25 @@ void RundownTaskCounter::Decrement() {
     waitable_event_.Signal();
 }
 
-bool RundownTaskCounter::TimedWaitUntil(const base::TimeTicks& end_time) {
+void RundownTaskCounter::TimedWait(base::TimeDelta timeout) {
   // Decrement the excess count from the constructor.
   Decrement();
 
-  return waitable_event_.TimedWaitUntil(end_time);
+  // RundownTaskCounter::TimedWait() could return
+  // |waitable_event_.TimedWait()|'s result if any user ever cared about whether
+  // it returned per success or timeout. Currently no user of this API cares and
+  // as such this return value is ignored.
+  waitable_event_.TimedWait(timeout);
 }
 
 }  // namespace
 
 void BrowserProcessImpl::FlushLocalStateAndReply(base::OnceClosure reply) {
-  if (local_state_)
-    local_state_->CommitPendingWrite();
-  local_state_task_runner_->PostTaskAndReply(FROM_HERE, base::DoNothing(),
-                                             std::move(reply));
+  if (local_state_) {
+    local_state_->CommitPendingWrite(std::move(reply));
+    return;
+  }
+  base::SequencedTaskRunnerHandle::Get()->PostTask(FROM_HERE, std::move(reply));
 }
 
 void BrowserProcessImpl::EndSession() {
@@ -546,8 +543,8 @@ void BrowserProcessImpl::EndSession() {
     Profile* profile = profiles[i];
     profile->SetExitType(Profile::EXIT_SESSION_ENDED);
     if (profile->GetPrefs()) {
-      profile->GetPrefs()->CommitPendingWrite();
-      rundown_counter->Post(profile->GetIOTaskRunner().get());
+      profile->GetPrefs()->CommitPendingWrite(
+          base::OnceClosure(), rundown_counter->GetRundownClosure());
     }
   }
 
@@ -559,9 +556,8 @@ void BrowserProcessImpl::EndSession() {
     // MetricsService lazily writes to prefs, force it to write now.
     // On ChromeOS, chrome gets killed when hangs, so no need to
     // commit metrics::prefs::kStabilitySessionEndCompleted change immediately.
-    local_state_->CommitPendingWrite();
-
-    rundown_counter->Post(local_state_task_runner_.get());
+    local_state_->CommitPendingWrite(base::OnceClosure(),
+                                     rundown_counter->GetRundownClosure());
 #endif
   }
 
@@ -588,8 +584,7 @@ void BrowserProcessImpl::EndSession() {
   // GPU process synchronously. Because the system may not be allowing
   // processes to launch, this can result in a hang. See
   // http://crbug.com/318527.
-  const base::TimeTicks end_time = base::TimeTicks::Now() + kEndSessionTimeout;
-  rundown_counter->TimedWaitUntil(end_time);
+  rundown_counter->TimedWait(kEndSessionTimeout);
 #else
   NOTIMPLEMENTED();
 #endif
@@ -1117,8 +1112,8 @@ void BrowserProcessImpl::CreateLocalState() {
   auto delegate = pref_service_factory_->CreateDelegate();
   delegate->InitPrefRegistry(pref_registry.get());
   local_state_ = chrome_prefs::CreateLocalState(
-      local_state_path, local_state_task_runner_.get(), policy_service(),
-      std::move(pref_registry), false, std::move(delegate));
+      local_state_path, policy_service(), std::move(pref_registry), false,
+      std::move(delegate));
   DCHECK(local_state_);
 
   sessions::SessionIdGenerator::GetInstance()->Init(local_state_.get());

chrome/browser/browser_process_impl.h

@@ -47,7 +47,6 @@ class PluginsResourceService;
 
 namespace base {
 class CommandLine;
-class SequencedTaskRunner;
 }
 
 namespace extensions {
@@ -75,9 +74,7 @@ class TabLifecycleUnitSource;
 class BrowserProcessImpl : public BrowserProcess,
                            public KeepAliveStateObserver {
  public:
-  // |local_state_task_runner| must be a shutdown-blocking task runner.
-  explicit BrowserProcessImpl(
-      base::SequencedTaskRunner* local_state_task_runner);
+  BrowserProcessImpl();
   ~BrowserProcessImpl() override;
 
   // Called to complete initialization.
@@ -323,9 +320,6 @@ class BrowserProcessImpl : public BrowserProcess,
 
   scoped_refptr<DownloadRequestLimiter> download_request_limiter_;
 
-  // Sequenced task runner for local state related I/O tasks.
-  const scoped_refptr<base::SequencedTaskRunner> local_state_task_runner_;
-
   // 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

@@ -31,8 +31,7 @@ class BrowserProcessImplTest : public ::testing::Test {
         loop_(base::MessageLoop::TYPE_UI),
         ui_thread_(content::BrowserThread::UI, &loop_),
         command_line_(base::CommandLine::NO_PROGRAM),
-        browser_process_impl_(
-            new BrowserProcessImpl(base::ThreadTaskRunnerHandle::Get().get())) {
+        browser_process_impl_(new BrowserProcessImpl()) {
     // Create() and StartWithDefaultParams() TaskScheduler in seperate steps to
     // properly simulate the browser process' lifecycle.
     base::TaskScheduler::Create("BrowserProcessImplTest");

chrome/browser/chrome_browser_main.cc

@@ -349,7 +349,7 @@ void AddFirstRunNewTabs(StartupBrowserCreator* browser_creator,
 }
 #endif  // !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
 
-void InitializeLocalState(base::SequencedTaskRunner* local_state_task_runner) {
+void InitializeLocalState() {
   TRACE_EVENT0("startup", "ChromeBrowserMainParts::InitializeLocalState")
 
   // Load local state.  This includes the application locale so we know which
@@ -406,7 +406,6 @@ void InitializeLocalState(base::SequencedTaskRunner* local_state_task_runner) {
                                    std::string());
       const std::unique_ptr<PrefService> parent_local_state =
           chrome_prefs::CreateLocalState(parent_profile,
-                                         local_state_task_runner,
                                          g_browser_process->policy_service(),
                                          std::move(registry), false, nullptr);
       // Right now, we only inherit the locale setting from the parent profile.
@@ -690,20 +689,6 @@ bool IsWebDriverOverridingPolicy(PrefService* local_state) {
                 prefs::kWebDriverOverridesIncompatiblePolicies)));
 }
 
-// The initial read is done synchronously, the TaskPriority is thus only used
-// for flushes to disks and BACKGROUND is therefore appropriate. Priority of
-// remaining BACKGROUND+BLOCK_SHUTDOWN tasks is bumped by the TaskScheduler on
-// shutdown. However, some shutdown use cases happen without
-// TaskScheduler::Shutdown() (e.g. ChromeRestartRequest::Start() and
-// BrowserProcessImpl::EndSession()) and we must thus unfortunately make this
-// USER_VISIBLE until we solve https://crbug.com/747495 to allow bumping
-// priority of a sequence on demand.
-scoped_refptr<base::SequencedTaskRunner> CreateLocalStateTaskRunner() {
-  return base::CreateSequencedTaskRunnerWithTraits(
-      {base::MayBlock(), base::TaskPriority::USER_VISIBLE,
-       base::TaskShutdownBehavior::BLOCK_SHUTDOWN});
-}
-
 // Initializes the shared instance of ResourceBundle and returns the locale. An
 // empty string return value indicates failure.
 std::string InitResourceBundleAndDetermineLocale(
@@ -1005,16 +990,11 @@ int ChromeBrowserMainParts::PreEarlyInitialization() {
   for (size_t i = 0; i < chrome_extra_parts_.size(); ++i)
     chrome_extra_parts_[i]->PreEarlyInitialization();
 
-  // Create BrowserProcess in PreEarlyInitialization() so that we can load
-  // field trials (and all it depends upon).
-  scoped_refptr<base::SequencedTaskRunner> local_state_task_runner =
-      CreateLocalStateTaskRunner();
-  browser_process_ =
-      std::make_unique<BrowserProcessImpl>(local_state_task_runner.get());
+  browser_process_ = std::make_unique<BrowserProcessImpl>();
 
   bool failed_to_load_resource_bundle = false;
-  const int load_local_state_result = LoadLocalState(
-      local_state_task_runner.get(), &failed_to_load_resource_bundle);
+  const int load_local_state_result =
+      LoadLocalState(&failed_to_load_resource_bundle);
   if (load_local_state_result == chrome::RESULT_CODE_MISSING_DATA &&
       failed_to_load_resource_bundle) {
     if (base::CommandLine::ForCurrentProcess()->HasSwitch(
@@ -1089,13 +1069,12 @@ int ChromeBrowserMainParts::PreCreateThreads() {
 }
 
 int ChromeBrowserMainParts::LoadLocalState(
-    base::SequencedTaskRunner* local_state_task_runner,
     bool* failed_to_load_resource_bundle) {
   *failed_to_load_resource_bundle = false;
   if (!base::PathService::Get(chrome::DIR_USER_DATA, &user_data_dir_))
     return chrome::RESULT_CODE_MISSING_DATA;
 
-  InitializeLocalState(local_state_task_runner);
+  InitializeLocalState();
 
   ConvertFlagsToSwitches();
 

chrome/browser/chrome_browser_main.h

@@ -29,10 +29,6 @@ class StartupTimeBomb;
 class ShutdownWatcherHelper;
 class WebUsbDetector;
 
-namespace base {
-class SequencedTaskRunner;
-}
-
 namespace chrome_browser {
 // For use by ShowMissingLocaleMessageBox.
 #if defined(OS_WIN)
@@ -130,8 +126,7 @@ class ChromeBrowserMainParts : public content::BrowserMainParts {
   // If the return value is RESULT_CODE_MISSING_DATA, then
   // |failed_to_load_resource_bundle| indicates if the ResourceBundle couldn't
   // be loaded.
-  int LoadLocalState(base::SequencedTaskRunner* local_state_task_runner,
-                     bool* failed_to_load_resource_bundle);
+  int LoadLocalState(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

chrome/browser/prefs/chrome_pref_service_factory.cc

@@ -453,18 +453,17 @@ const char kSettingsEnforcementGroupEnforceAlwaysWithExtensionsAndDSE[] =
 
 std::unique_ptr<PrefService> CreateLocalState(
     const base::FilePath& pref_filename,
-    base::SequencedTaskRunner* pref_io_task_runner,
     policy::PolicyService* policy_service,
     scoped_refptr<PrefRegistry> pref_registry,
     bool async,
     std::unique_ptr<PrefValueStore::Delegate> delegate) {
   sync_preferences::PrefServiceSyncableFactory factory;
-  PrepareFactory(&factory, pref_filename, policy_service,
-                 nullptr,  // supervised_user_settings
-                 new JsonPrefStore(pref_filename, pref_io_task_runner,
-                                   std::unique_ptr<PrefFilter>()),
-                 nullptr,  // extension_prefs
-                 async);
+  PrepareFactory(
+      &factory, pref_filename, policy_service,
+      nullptr,  // supervised_user_settings
+      new JsonPrefStore(pref_filename, std::unique_ptr<PrefFilter>()),
+      nullptr,  // extension_prefs
+      async);
   return factory.Create(std::move(pref_registry), std::move(delegate));
 }
 

chrome/browser/prefs/chrome_pref_service_factory.h

@@ -66,7 +66,6 @@ extern const char kSettingsEnforcementGroupEnforceAlwaysWithExtensionsAndDSE[];
 
 std::unique_ptr<PrefService> CreateLocalState(
     const base::FilePath& pref_filename,
-    base::SequencedTaskRunner* pref_io_task_runner,
     policy::PolicyService* policy_service,
     scoped_refptr<PrefRegistry> pref_registry,
     bool async,

chrome/browser/prefs/profile_pref_store_manager.cc

@@ -92,7 +92,7 @@ PersistentPrefStore* ProfilePrefStoreManager::CreateProfilePrefStore(
     prefs::mojom::TrackedPreferenceValidationDelegatePtr validation_delegate) {
   if (!kPlatformSupportsPreferenceTracking) {
     return new JsonPrefStore(profile_path_.Append(chrome::kPreferencesFilename),
-                             io_task_runner, nullptr);
+                             nullptr, io_task_runner);
   }
   return CreateTrackedPersistentPrefStore(
       CreateTrackedPrefStoreConfiguration(

chrome/browser/profiles/profile_browsertest.cc

@@ -17,6 +17,7 @@
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
 #include "base/sequenced_task_runner.h"
@@ -675,6 +676,20 @@ IN_PROC_BROWSER_TEST_F(ProfileBrowserTest,
   }
 #endif
 
+  // It is important that the MessageLoop not pump extra messages during
+  // EndSession() as some of those may be tasks queued to attempt to revive
+  // services and processes that were just intentionally killed. This is a
+  // regression blocker for https://crbug.com/318527.
+  // Need to use this WeakPtr workaround as the browser test harness runs all
+  // tasks until idle when tearing down.
+  struct FailsIfCalledWhileOnStack
+      : public base::SupportsWeakPtr<FailsIfCalledWhileOnStack> {
+    void Fail() { ADD_FAILURE(); }
+  } fails_if_called_while_on_stack;
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::BindOnce(&FailsIfCalledWhileOnStack::Fail,
+                                fails_if_called_while_on_stack.AsWeakPtr()));
+
   // This retry loop reduces flakiness due to the fact that this ultimately
   // tests whether or not a code path hits a timed wait.
   bool succeeded = false;

chrome/browser/supervised_user/supervised_user_settings_service.cc

@@ -70,8 +70,8 @@ void SupervisedUserSettingsService::Init(
     bool load_synchronously) {
   base::FilePath path =
       profile_path.Append(chrome::kSupervisedUserSettingsFilename);
-  PersistentPrefStore* store = new JsonPrefStore(path, sequenced_task_runner,
-                                                 std::unique_ptr<PrefFilter>());
+  PersistentPrefStore* store = new JsonPrefStore(
+      path, std::unique_ptr<PrefFilter>(), sequenced_task_runner);
   Init(store);
   if (load_synchronously) {
     store_->ReadPrefs();

chrome/service/service_process_prefs.cc

@@ -13,8 +13,8 @@
 ServiceProcessPrefs::ServiceProcessPrefs(const base::FilePath& pref_filename,
                                          base::SequencedTaskRunner* task_runner)
     : prefs_(new JsonPrefStore(pref_filename,
-                               task_runner,
-                               std::unique_ptr<PrefFilter>())) {}
+                               std::unique_ptr<PrefFilter>(),
+                               task_runner)) {}
 
 ServiceProcessPrefs::~ServiceProcessPrefs() {}
 

components/cronet/cronet_prefs_manager.cc

@@ -232,8 +232,8 @@ CronetPrefsManager::CronetPrefsManager(
   base::FilePath filepath =
       storage_file_path.Append(kPrefsDirectoryName).Append(kPrefsFileName);
 
-  json_pref_store_ = new JsonPrefStore(filepath, file_task_runner,
-                                       std::unique_ptr<PrefFilter>());
+  json_pref_store_ = new JsonPrefStore(filepath, std::unique_ptr<PrefFilter>(),
+                                       file_task_runner);
 
   // Register prefs and set up the PrefService.
   PrefServiceFactory factory;

components/prefs/json_pref_store.cc

@@ -132,8 +132,8 @@ std::unique_ptr<JsonPrefStore::ReadResult> ReadPrefsFromDisk(
 
 JsonPrefStore::JsonPrefStore(
     const base::FilePath& pref_filename,
-    scoped_refptr<base::SequencedTaskRunner> file_task_runner,
-    std::unique_ptr<PrefFilter> pref_filter)
+    std::unique_ptr<PrefFilter> pref_filter,
+    scoped_refptr<base::SequencedTaskRunner> file_task_runner)
     : path_(pref_filename),
       file_task_runner_(std::move(file_task_runner)),
       prefs_(new base::DictionaryValue()),

components/prefs/json_pref_store.h

@@ -62,12 +62,20 @@ class COMPONENTS_PREFS_EXPORT JsonPrefStore
   // have the base::TaskShutdownBehavior::BLOCK_SHUTDOWN and base::MayBlock()
   // traits. Unless external tasks need to run on the same sequence as
   // JsonPrefStore tasks, keep the default value.
+  // The initial read is done synchronously, the TaskPriority is thus only used
+  // for flushes to disks and BACKGROUND is therefore appropriate. Priority of
+  // remaining BACKGROUND+BLOCK_SHUTDOWN tasks is bumped by the TaskScheduler on
+  // shutdown. However, some shutdown use cases happen without
+  // TaskScheduler::Shutdown() (e.g. ChromeRestartRequest::Start() and
+  // BrowserProcessImpl::EndSession()) and we must thus unfortunately make this
+  // USER_VISIBLE until we solve https://crbug.com/747495 to allow bumping
+  // priority of a sequence on demand.
   JsonPrefStore(const base::FilePath& pref_filename,
+                std::unique_ptr<PrefFilter> pref_filter = nullptr,
                 scoped_refptr<base::SequencedTaskRunner> file_task_runner =
                     base::CreateSequencedTaskRunnerWithTraits(
-                        {base::MayBlock(),
-                         base::TaskShutdownBehavior::BLOCK_SHUTDOWN}),
-                std::unique_ptr<PrefFilter> pref_filter = nullptr);
+                        {base::MayBlock(), base::TaskPriority::USER_VISIBLE,
+                         base::TaskShutdownBehavior::BLOCK_SHUTDOWN}));
 
   // PrefStore overrides:
   bool GetValue(const std::string& key,

components/prefs/json_pref_store_unittest.cc

@@ -452,8 +452,7 @@ TEST_P(JsonPrefStoreTest, ReadWithInterceptor) {
   InterceptingPrefFilter* raw_intercepting_pref_filter_ =
       intercepting_pref_filter.get();
   auto pref_store = base::MakeRefCounted<JsonPrefStore>(
-      input_file, base::CreateSequencedTaskRunnerWithTraits({base::MayBlock()}),
-      std::move(intercepting_pref_filter));
+      input_file, std::move(intercepting_pref_filter));
 
   ASSERT_EQ(PersistentPrefStore::PREF_READ_ERROR_ASYNCHRONOUS_TASK_INCOMPLETE,
             pref_store->ReadPrefs());
@@ -495,8 +494,7 @@ TEST_P(JsonPrefStoreTest, ReadAsyncWithInterceptor) {
   InterceptingPrefFilter* raw_intercepting_pref_filter_ =
       intercepting_pref_filter.get();
   auto pref_store = base::MakeRefCounted<JsonPrefStore>(
-      input_file, base::CreateSequencedTaskRunnerWithTraits({base::MayBlock()}),
-      std::move(intercepting_pref_filter));
+      input_file, std::move(intercepting_pref_filter));
 
   MockPrefStoreObserver mock_observer;
   pref_store->AddObserver(&mock_observer);
@@ -1014,8 +1012,7 @@ TEST_F(JsonPrefStoreCallbackTest, TestSerializeDataCallbacks) {
   std::unique_ptr<InterceptingPrefFilter> intercepting_pref_filter(
       new InterceptingPrefFilter(write_callback_observer_.GetCallbackPair()));
   auto pref_store = base::MakeRefCounted<JsonPrefStore>(
-      input_file, base::CreateSequencedTaskRunnerWithTraits({base::MayBlock()}),
-      std::move(intercepting_pref_filter));
+      input_file, std::move(intercepting_pref_filter));
   ImportantFileWriter* file_writer = GetImportantFileWriter(pref_store.get());
 
   EXPECT_EQ(NOT_CALLED,

components/prefs/pref_service_factory.cc

@@ -24,7 +24,7 @@ void PrefServiceFactory::SetUserPrefsFile(
     const base::FilePath& prefs_file,
     base::SequencedTaskRunner* task_runner) {
   user_prefs_ =
-      base::MakeRefCounted<JsonPrefStore>(prefs_file, task_runner, nullptr);
+      base::MakeRefCounted<JsonPrefStore>(prefs_file, nullptr, task_runner);
 }
 
 std::unique_ptr<PrefService> PrefServiceFactory::Create(

ios/chrome/browser/browser_state/chrome_browser_state_impl_io_data.mm

@@ -187,12 +187,11 @@ void ChromeBrowserStateImplIOData::InitializeInternal(
   // Set up a persistent store for use by the network stack on the IO thread.
   base::FilePath network_json_store_filepath(
       profile_path_.Append(kIOSChromeNetworkPersistentStateFilename));
-  network_json_store_ =
-      new JsonPrefStore(network_json_store_filepath,
-                        base::CreateSequencedTaskRunnerWithTraits(
-                            {base::MayBlock(), base::TaskPriority::BEST_EFFORT,
-                             base::TaskShutdownBehavior::BLOCK_SHUTDOWN}),
-                        std::unique_ptr<PrefFilter>());
+  network_json_store_ = new JsonPrefStore(
+      network_json_store_filepath, std::unique_ptr<PrefFilter>(),
+      base::CreateSequencedTaskRunnerWithTraits(
+          {base::MayBlock(), base::TaskPriority::BEST_EFFORT,
+           base::TaskShutdownBehavior::BLOCK_SHUTDOWN}));
   network_json_store_->ReadPrefsAsync(nullptr);
 
   net::URLRequestContext* main_context = main_request_context();

ios/chrome/browser/prefs/ios_chrome_pref_service_factory.cc

@@ -36,7 +36,7 @@ void PrepareFactory(sync_preferences::PrefServiceSyncableFactory* factory,
                     const base::FilePath& pref_filename,
                     base::SequencedTaskRunner* pref_io_task_runner) {
   factory->set_user_prefs(base::MakeRefCounted<JsonPrefStore>(
-      pref_filename, pref_io_task_runner, std::unique_ptr<PrefFilter>()));
+      pref_filename, std::unique_ptr<PrefFilter>(), pref_io_task_runner));
 
   factory->set_read_error_callback(base::Bind(&HandleReadError));
   factory->SetPrefModelAssociatorClient(

services/network/network_context.cc

@@ -1030,7 +1030,7 @@ URLRequestContextOwner NetworkContext::ApplyContextParamsToBuilder(
   std::unique_ptr<PrefService> pref_service;
   if (params_->http_server_properties_path) {
     scoped_refptr<JsonPrefStore> json_pref_store(new JsonPrefStore(
-        *params_->http_server_properties_path,
+        *params_->http_server_properties_path, nullptr,
         base::CreateSequencedTaskRunnerWithTraits(
             {base::MayBlock(), base::TaskShutdownBehavior::BLOCK_SHUTDOWN,
              base::TaskPriority::BEST_EFFORT})));

services/preferences/tracked/tracked_persistent_pref_store_factory.cc

@@ -123,12 +123,12 @@ PersistentPrefStore* CreateTrackedPersistentPrefStore(
   PrefHashFilter* raw_protected_pref_hash_filter =
       protected_pref_hash_filter.get();
 
-  scoped_refptr<JsonPrefStore> unprotected_pref_store(
-      new JsonPrefStore(config->unprotected_pref_filename, io_task_runner.get(),
-                        std::move(unprotected_pref_hash_filter)));
-  scoped_refptr<JsonPrefStore> protected_pref_store(
-      new JsonPrefStore(config->protected_pref_filename, io_task_runner.get(),
-                        std::move(protected_pref_hash_filter)));
+  scoped_refptr<JsonPrefStore> unprotected_pref_store(new JsonPrefStore(
+      config->unprotected_pref_filename,
+      std::move(unprotected_pref_hash_filter), io_task_runner.get()));
+  scoped_refptr<JsonPrefStore> protected_pref_store(new JsonPrefStore(
+      config->protected_pref_filename, std::move(protected_pref_hash_filter),
+      io_task_runner.get()));
 
   SetupTrackedPreferencesMigration(
       unprotected_pref_names, protected_pref_names,