Avoided calls to AddRef and Release by moving scoped_refptrs.

Used RepeatingCallback instead of deprecated Callback.

Change-Id: I36a1079c5a5c2c0341d85e1e7230ac9055347349
Reviewed-on: https://chromium-review.googlesource.com/894962
Commit-Queue: François Degros <fdegros@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534877}

chrome/browser/browser_process_impl.cc

@@ -1082,7 +1082,7 @@ void BrowserProcessImpl::CreateLocalState() {
   delegate->InitPrefRegistry(pref_registry.get());
   local_state_ = chrome_prefs::CreateLocalState(
       local_state_path, local_state_task_runner_.get(), policy_service(),
-      pref_registry, false, std::move(delegate));
+      std::move(pref_registry), false, std::move(delegate));
   DCHECK(local_state_);
 }
 

chrome/browser/chrome_browser_main.cc

@@ -389,12 +389,14 @@ void InitializeLocalState(base::SequencedTaskRunner* local_state_task_runner) {
     if (!local_state_file_exists) {
       base::FilePath parent_profile =
           command_line->GetSwitchValuePath(switches::kParentProfile);
-      scoped_refptr<PrefRegistrySimple> registry = new PrefRegistrySimple();
-      std::unique_ptr<PrefService> parent_local_state(
-          chrome_prefs::CreateLocalState(
-              parent_profile, local_state_task_runner,
-              g_browser_process->policy_service(), registry, false, nullptr));
+      scoped_refptr<PrefRegistrySimple> registry =
+          base::MakeRefCounted<PrefRegistrySimple>();
       registry->RegisterStringPref(prefs::kApplicationLocale, 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.
       local_state->SetString(
           prefs::kApplicationLocale,

chrome/browser/prefs/chrome_pref_service_factory.cc

@@ -369,7 +369,7 @@ void PrepareFactory(sync_preferences::PrefServiceSyncableFactory* factory,
                     policy::PolicyService* policy_service,
                     SupervisedUserSettingsService* supervised_user_settings,
                     scoped_refptr<PersistentPrefStore> user_pref_store,
-                    const scoped_refptr<PrefStore>& extension_prefs,
+                    scoped_refptr<PrefStore> extension_prefs,
                     bool async) {
   policy::BrowserPolicyConnector* policy_connector =
       g_browser_process->browser_policy_connector();
@@ -386,12 +386,12 @@ void PrepareFactory(sync_preferences::PrefServiceSyncableFactory* factory,
 #endif
 
   factory->set_async(async);
-  factory->set_extension_prefs(extension_prefs);
+  factory->set_extension_prefs(std::move(extension_prefs));
   factory->set_command_line_prefs(
       base::MakeRefCounted<ChromeCommandLinePrefStore>(
           base::CommandLine::ForCurrentProcess()));
   factory->set_read_error_callback(base::Bind(&HandleReadError, pref_filename));
-  factory->set_user_prefs(user_pref_store);
+  factory->set_user_prefs(std::move(user_pref_store));
   factory->SetPrefModelAssociatorClient(
       ChromePrefModelAssociatorClient::GetInstance());
 }
@@ -440,17 +440,17 @@ std::unique_ptr<PrefService> CreateLocalState(
     const base::FilePath& pref_filename,
     base::SequencedTaskRunner* pref_io_task_runner,
     policy::PolicyService* policy_service,
-    const scoped_refptr<PrefRegistry>& pref_registry,
+    scoped_refptr<PrefRegistry> pref_registry,
     bool async,
     std::unique_ptr<PrefValueStore::Delegate> delegate) {
   sync_preferences::PrefServiceSyncableFactory factory;
   PrepareFactory(&factory, pref_filename, policy_service,
-                 NULL,  // supervised_user_settings
+                 nullptr,  // supervised_user_settings
                  new JsonPrefStore(pref_filename, pref_io_task_runner,
                                    std::unique_ptr<PrefFilter>()),
-                 NULL,  // extension_prefs
+                 nullptr,  // extension_prefs
                  async);
-  return factory.Create(pref_registry.get(), std::move(delegate));
+  return factory.Create(std::move(pref_registry), std::move(delegate));
 }
 
 std::unique_ptr<sync_preferences::PrefServiceSyncable> CreateProfilePrefs(
@@ -458,8 +458,8 @@ std::unique_ptr<sync_preferences::PrefServiceSyncable> CreateProfilePrefs(
     prefs::mojom::TrackedPreferenceValidationDelegatePtr validation_delegate,
     policy::PolicyService* policy_service,
     SupervisedUserSettingsService* supervised_user_settings,
-    const scoped_refptr<PrefStore>& extension_prefs,
-    const scoped_refptr<user_prefs::PrefRegistrySyncable>& pref_registry,
+    scoped_refptr<PrefStore> extension_prefs,
+    scoped_refptr<user_prefs::PrefRegistrySyncable> pref_registry,
     bool async,
     scoped_refptr<base::SequencedTaskRunner> io_task_runner,
     std::unique_ptr<PrefValueStore::Delegate> delegate) {
@@ -471,19 +471,16 @@ std::unique_ptr<sync_preferences::PrefServiceSyncable> CreateProfilePrefs(
       base::MakeUnique<ResetOnLoadObserverImpl>(profile_path),
       mojo::MakeRequest(&reset_on_load_observer));
   sync_preferences::PrefServiceSyncableFactory factory;
-  scoped_refptr<PersistentPrefStore> user_pref_store(
+  scoped_refptr<PersistentPrefStore> user_pref_store =
       CreateProfilePrefStoreManager(profile_path)
           ->CreateProfilePrefStore(
               GetTrackingConfiguration(), kTrackedPrefsReportingIDsCount,
               std::move(io_task_runner), std::move(reset_on_load_observer),
-              std::move(validation_delegate)));
+              std::move(validation_delegate));
   PrepareFactory(&factory, profile_path, policy_service,
-                 supervised_user_settings, user_pref_store, extension_prefs,
-                 async);
-  std::unique_ptr<sync_preferences::PrefServiceSyncable> pref_service =
-      factory.CreateSyncable(pref_registry.get(), std::move(delegate));
-
-  return pref_service;
+                 supervised_user_settings, std::move(user_pref_store),
+                 std::move(extension_prefs), async);
+  return factory.CreateSyncable(std::move(pref_registry), std::move(delegate));
 }
 
 void DisableDomainCheckForTesting() {

chrome/browser/prefs/chrome_pref_service_factory.h

@@ -68,7 +68,7 @@ std::unique_ptr<PrefService> CreateLocalState(
     const base::FilePath& pref_filename,
     base::SequencedTaskRunner* pref_io_task_runner,
     policy::PolicyService* policy_service,
-    const scoped_refptr<PrefRegistry>& pref_registry,
+    scoped_refptr<PrefRegistry> pref_registry,
     bool async,
     std::unique_ptr<PrefValueStore::Delegate> delegate);
 
@@ -77,8 +77,8 @@ std::unique_ptr<sync_preferences::PrefServiceSyncable> CreateProfilePrefs(
     prefs::mojom::TrackedPreferenceValidationDelegatePtr validation_delegate,
     policy::PolicyService* policy_service,
     SupervisedUserSettingsService* supervised_user_settings,
-    const scoped_refptr<PrefStore>& extension_prefs,
-    const scoped_refptr<user_prefs::PrefRegistrySyncable>& pref_registry,
+    scoped_refptr<PrefStore> extension_prefs,
+    scoped_refptr<user_prefs::PrefRegistrySyncable> pref_registry,
     bool async,
     scoped_refptr<base::SequencedTaskRunner> io_task_runner,
     std::unique_ptr<PrefValueStore::Delegate> delegate);

components/prefs/pref_service.cc

@@ -30,7 +30,7 @@ namespace {
 class ReadErrorHandler : public PersistentPrefStore::ReadErrorDelegate {
  public:
   using ErrorCallback =
-      base::Callback<void(PersistentPrefStore::PrefReadError)>;
+      base::RepeatingCallback<void(PersistentPrefStore::PrefReadError)>;
   explicit ReadErrorHandler(ErrorCallback cb) : callback_(cb) {}
 
   void OnError(PersistentPrefStore::PrefReadError error) override {
@@ -61,16 +61,16 @@ uint32_t GetWriteFlags(const PrefService::Preference* pref) {
 PrefService::PrefService(
     std::unique_ptr<PrefNotifierImpl> pref_notifier,
     std::unique_ptr<PrefValueStore> pref_value_store,
-    PersistentPrefStore* user_prefs,
-    PrefRegistry* pref_registry,
-    base::Callback<void(PersistentPrefStore::PrefReadError)>
+    scoped_refptr<PersistentPrefStore> user_prefs,
+    scoped_refptr<PrefRegistry> pref_registry,
+    base::RepeatingCallback<void(PersistentPrefStore::PrefReadError)>
         read_error_callback,
     bool async)
     : pref_notifier_(std::move(pref_notifier)),
       pref_value_store_(std::move(pref_value_store)),
-      pref_registry_(pref_registry),
-      user_pref_store_(user_prefs),
-      read_error_callback_(read_error_callback) {
+      pref_registry_(std::move(pref_registry)),
+      user_pref_store_(std::move(user_prefs)),
+      read_error_callback_(std::move(read_error_callback)) {
   pref_notifier_->SetPrefService(this);
 
   // TODO(battre): This is a check for crbug.com/435208 to make sure that

components/prefs/pref_service.h

@@ -166,9 +166,9 @@ class COMPONENTS_PREFS_EXPORT PrefService {
   // for simplified construction.
   PrefService(std::unique_ptr<PrefNotifierImpl> pref_notifier,
               std::unique_ptr<PrefValueStore> pref_value_store,
-              PersistentPrefStore* user_prefs,
-              PrefRegistry* pref_registry,
-              base::Callback<void(PersistentPrefStore::PrefReadError)>
+              scoped_refptr<PersistentPrefStore> user_prefs,
+              scoped_refptr<PrefRegistry> pref_registry,
+              base::RepeatingCallback<void(PersistentPrefStore::PrefReadError)>
                   read_error_callback,
               bool async);
   virtual ~PrefService();
@@ -359,7 +359,8 @@ class COMPONENTS_PREFS_EXPORT PrefService {
   scoped_refptr<PersistentPrefStore> user_pref_store_;
 
   // Callback to call when a read error occurs.
-  base::Callback<void(PersistentPrefStore::PrefReadError)> read_error_callback_;
+  const base::RepeatingCallback<void(PersistentPrefStore::PrefReadError)>
+      read_error_callback_;
 
  private:
   // Hash map expected to be fastest here since it minimises expensive

components/prefs/pref_service_factory.cc

@@ -37,7 +37,7 @@ void PrefServiceFactory::SetUserPrefsFile(
 }
 
 std::unique_ptr<PrefService> PrefServiceFactory::Create(
-    PrefRegistry* pref_registry,
+    scoped_refptr<PrefRegistry> pref_registry,
     std::unique_ptr<PrefValueStore::Delegate> delegate) {
   auto pref_notifier = std::make_unique<PrefNotifierImpl>();
   auto pref_value_store = std::make_unique<PrefValueStore>(
@@ -47,5 +47,5 @@ std::unique_ptr<PrefService> PrefServiceFactory::Create(
       pref_notifier.get(), std::move(delegate));
   return std::make_unique<PrefService>(
       std::move(pref_notifier), std::move(pref_value_store), user_prefs_.get(),
-      pref_registry, read_error_callback_, async_);
+      std::move(pref_registry), read_error_callback_, async_);
 }

components/prefs/pref_service_factory.h

@@ -55,9 +55,9 @@ class COMPONENTS_PREFS_EXPORT PrefServiceFactory {
   // Sets up error callback for the PrefService.  A do-nothing default
   // is provided if this is not called.
   void set_read_error_callback(
-      const base::Callback<void(PersistentPrefStore::PrefReadError)>&
+      base::RepeatingCallback<void(PersistentPrefStore::PrefReadError)>
           read_error_callback) {
-    read_error_callback_ = read_error_callback;
+    read_error_callback_ = std::move(read_error_callback);
   }
 
   // Specifies to use an actual file-backed user pref store.
@@ -71,7 +71,7 @@ class COMPONENTS_PREFS_EXPORT PrefServiceFactory {
   // Creates a PrefService object initialized with the parameters from
   // this factory.
   std::unique_ptr<PrefService> Create(
-      PrefRegistry* registry,
+      scoped_refptr<PrefRegistry> pref_registry,
       std::unique_ptr<PrefValueStore::Delegate> delegate = nullptr);
 
  protected:
@@ -82,7 +82,8 @@ class COMPONENTS_PREFS_EXPORT PrefServiceFactory {
   scoped_refptr<PersistentPrefStore> user_prefs_;
   scoped_refptr<PrefStore> recommended_prefs_;
 
-  base::Callback<void(PersistentPrefStore::PrefReadError)> read_error_callback_;
+  base::RepeatingCallback<void(PersistentPrefStore::PrefReadError)>
+      read_error_callback_;
 
   // Defaults to false.
   bool async_;

components/sync_preferences/pref_service_syncable.cc

@@ -30,17 +30,17 @@ namespace sync_preferences {
 PrefServiceSyncable::PrefServiceSyncable(
     std::unique_ptr<PrefNotifierImpl> pref_notifier,
     std::unique_ptr<PrefValueStore> pref_value_store,
-    PersistentPrefStore* user_prefs,
-    user_prefs::PrefRegistrySyncable* pref_registry,
+    scoped_refptr<PersistentPrefStore> user_prefs,
+    scoped_refptr<user_prefs::PrefRegistrySyncable> pref_registry,
     const PrefModelAssociatorClient* pref_model_associator_client,
-    base::Callback<void(PersistentPrefStore::PrefReadError)>
+    base::RepeatingCallback<void(PersistentPrefStore::PrefReadError)>
         read_error_callback,
     bool async)
     : PrefService(std::move(pref_notifier),
                   std::move(pref_value_store),
-                  user_prefs,
-                  pref_registry,
-                  read_error_callback,
+                  std::move(user_prefs),
+                  std::move(pref_registry),
+                  std::move(read_error_callback),
                   async),
       pref_service_forked_(false),
       pref_sync_associator_(pref_model_associator_client, syncer::PREFERENCES),
@@ -54,17 +54,17 @@ PrefServiceSyncable::PrefServiceSyncable(
       &PrefServiceSyncable::ProcessPrefChange, base::Unretained(this)));
 
   // Add already-registered syncable preferences to PrefModelAssociator.
-  for (PrefRegistry::const_iterator it = pref_registry->begin();
-       it != pref_registry->end(); ++it) {
-    const std::string& path = it->first;
+  for (const auto& entry : *pref_registry_) {
+    const std::string& path = entry.first;
     AddRegisteredSyncablePreference(path,
                                     pref_registry_->GetRegistrationFlags(path));
   }
 
   // Watch for syncable preferences registered after this point.
-  pref_registry->SetSyncableRegistrationCallback(
-      base::Bind(&PrefServiceSyncable::AddRegisteredSyncablePreference,
-                 base::Unretained(this)));
+  static_cast<user_prefs::PrefRegistrySyncable*>(pref_registry_.get())
+      ->SetSyncableRegistrationCallback(base::BindRepeating(
+          &PrefServiceSyncable::AddRegisteredSyncablePreference,
+          base::Unretained(this)));
 }
 
 PrefServiceSyncable::~PrefServiceSyncable() {
@@ -110,7 +110,7 @@ PrefServiceSyncable::CreateIncognitoPrefService(
       std::move(delegate));
   return std::make_unique<PrefServiceSyncable>(
       std::move(pref_notifier), std::move(pref_value_store),
-      incognito_pref_store.get(), forked_registry.get(),
+      std::move(incognito_pref_store), std::move(forked_registry),
       pref_sync_associator_.client(), read_error_callback_, false);
 }
 

components/sync_preferences/pref_service_syncable.h

@@ -38,10 +38,10 @@ class PrefServiceSyncable : public PrefService {
   PrefServiceSyncable(
       std::unique_ptr<PrefNotifierImpl> pref_notifier,
       std::unique_ptr<PrefValueStore> pref_value_store,
-      PersistentPrefStore* user_prefs,
-      user_prefs::PrefRegistrySyncable* pref_registry,
+      scoped_refptr<PersistentPrefStore> user_prefs,
+      scoped_refptr<user_prefs::PrefRegistrySyncable> pref_registry,
       const PrefModelAssociatorClient* pref_model_associato_client,
-      base::Callback<void(PersistentPrefStore::PrefReadError)>
+      base::RepeatingCallback<void(PersistentPrefStore::PrefReadError)>
           read_error_callback,
       bool async);
   ~PrefServiceSyncable() override;

components/sync_preferences/pref_service_syncable_factory.cc

@@ -57,7 +57,7 @@ void PrefServiceSyncableFactory::SetPrefModelAssociatorClient(
 }
 
 std::unique_ptr<PrefServiceSyncable> PrefServiceSyncableFactory::CreateSyncable(
-    user_prefs::PrefRegistrySyncable* pref_registry,
+    scoped_refptr<user_prefs::PrefRegistrySyncable> pref_registry,
     std::unique_ptr<PrefValueStore::Delegate> delegate) {
   TRACE_EVENT0("browser", "PrefServiceSyncableFactory::CreateSyncable");
   auto pref_notifier = std::make_unique<PrefNotifierImpl>();
@@ -68,8 +68,8 @@ std::unique_ptr<PrefServiceSyncable> PrefServiceSyncableFactory::CreateSyncable(
       pref_notifier.get(), std::move(delegate));
   return std::make_unique<PrefServiceSyncable>(
       std::move(pref_notifier), std::move(pref_value_store), user_prefs_.get(),
-      pref_registry, pref_model_associator_client_, read_error_callback_,
-      async_);
+      std::move(pref_registry), pref_model_associator_client_,
+      read_error_callback_, async_);
 }
 
 }  // namespace sync_preferences

components/sync_preferences/pref_service_syncable_factory.h

@@ -46,7 +46,7 @@ class PrefServiceSyncableFactory : public PrefServiceFactory {
   // |delegate| might be null during test or if we're not using the Mojo pref
   // service.
   std::unique_ptr<PrefServiceSyncable> CreateSyncable(
-      user_prefs::PrefRegistrySyncable* registry,
+      scoped_refptr<user_prefs::PrefRegistrySyncable> pref_registry,
       std::unique_ptr<PrefValueStore::Delegate> delegate = nullptr);
 
  private: