Use std::unique_ptr expressing ownership in PrefService and PrefServiceSyncable.

Change-Id: I2fe10a0aa80796b4af0a36c7db355cfa8b8144e7
Bug: 
Reviewed-on: https://chromium-review.googlesource.com/790112Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Commit-Queue: François Degros <fdegros@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521238}

components/prefs/pref_service.cc

@@ -59,15 +59,15 @@ uint32_t GetWriteFlags(const PrefService::Preference* pref) {
 }  // namespace
 
 PrefService::PrefService(
-    PrefNotifierImpl* pref_notifier,
-    PrefValueStore* pref_value_store,
+    std::unique_ptr<PrefNotifierImpl> pref_notifier,
+    std::unique_ptr<PrefValueStore> pref_value_store,
     PersistentPrefStore* user_prefs,
     PrefRegistry* pref_registry,
     base::Callback<void(PersistentPrefStore::PrefReadError)>
         read_error_callback,
     bool async)
-    : pref_notifier_(pref_notifier),
-      pref_value_store_(pref_value_store),
+    : 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) {

components/prefs/pref_service.h

@@ -163,14 +163,13 @@ class COMPONENTS_PREFS_EXPORT PrefService {
 
   // You may wish to use PrefServiceFactory or one of its subclasses
   // for simplified construction.
-  PrefService(
-      PrefNotifierImpl* pref_notifier,
-      PrefValueStore* pref_value_store,
-      PersistentPrefStore* user_prefs,
-      PrefRegistry* pref_registry,
-      base::Callback<void(PersistentPrefStore::PrefReadError)>
-          read_error_callback,
-      bool async);
+  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)>
+                  read_error_callback,
+              bool async);
   virtual ~PrefService();
 
   // Lands pending writes to disk. This should only be used if we need to save

components/prefs/pref_service_factory.cc

@@ -4,6 +4,8 @@
 
 #include "components/prefs/pref_service_factory.h"
 
+#include <memory>
+
 #include "base/bind.h"
 #include "base/sequenced_task_runner.h"
 #include "components/prefs/default_pref_store.h"
@@ -37,14 +39,13 @@ void PrefServiceFactory::SetUserPrefsFile(
 std::unique_ptr<PrefService> PrefServiceFactory::Create(
     PrefRegistry* pref_registry,
     std::unique_ptr<PrefValueStore::Delegate> delegate) {
-  PrefNotifierImpl* pref_notifier = new PrefNotifierImpl();
-  std::unique_ptr<PrefService> pref_service(new PrefService(
-      pref_notifier,
-      new PrefValueStore(managed_prefs_.get(), supervised_user_prefs_.get(),
-                         extension_prefs_.get(), command_line_prefs_.get(),
-                         user_prefs_.get(), recommended_prefs_.get(),
-                         pref_registry->defaults().get(), pref_notifier,
-                         std::move(delegate)),
-      user_prefs_.get(), pref_registry, read_error_callback_, async_));
-  return pref_service;
+  auto pref_notifier = std::make_unique<PrefNotifierImpl>();
+  auto pref_value_store = std::make_unique<PrefValueStore>(
+      managed_prefs_.get(), supervised_user_prefs_.get(),
+      extension_prefs_.get(), command_line_prefs_.get(), user_prefs_.get(),
+      recommended_prefs_.get(), pref_registry->defaults().get(),
+      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_);
 }

components/prefs/pref_value_store.cc

@@ -76,7 +76,7 @@ PrefValueStore::PrefValueStore(PrefStore* managed_prefs,
 
 PrefValueStore::~PrefValueStore() {}
 
-PrefValueStore* PrefValueStore::CloneAndSpecialize(
+std::unique_ptr<PrefValueStore> PrefValueStore::CloneAndSpecialize(
     PrefStore* managed_prefs,
     PrefStore* supervised_user_prefs,
     PrefStore* extension_prefs,
@@ -102,10 +102,10 @@ PrefValueStore* PrefValueStore::CloneAndSpecialize(
   if (!default_prefs)
     default_prefs = GetPrefStore(DEFAULT_STORE);
 
-  return new PrefValueStore(managed_prefs, supervised_user_prefs,
-                            extension_prefs, command_line_prefs, user_prefs,
-                            recommended_prefs, default_prefs, pref_notifier,
-                            std::move(delegate));
+  return std::make_unique<PrefValueStore>(
+      managed_prefs, supervised_user_prefs, extension_prefs, command_line_prefs,
+      user_prefs, recommended_prefs, default_prefs, pref_notifier,
+      std::move(delegate));
 }
 
 void PrefValueStore::set_callback(const PrefChangedCallback& callback) {

components/prefs/pref_value_store.h

@@ -6,6 +6,7 @@
 #define COMPONENTS_PREFS_PREF_VALUE_STORE_H_
 
 #include <map>
+#include <memory>
 #include <string>
 #include <type_traits>
 #include <vector>
@@ -115,7 +116,7 @@ class COMPONENTS_PREFS_EXPORT PrefValueStore {
   // by the parameters passed, if unequal NULL.
   //
   // The new PrefValueStore is passed the |delegate| in its constructor.
-  PrefValueStore* CloneAndSpecialize(
+  std::unique_ptr<PrefValueStore> CloneAndSpecialize(
       PrefStore* managed_prefs,
       PrefStore* supervised_user_prefs,
       PrefStore* extension_prefs,

components/prefs/testing_pref_service.cc

@@ -4,6 +4,8 @@
 
 #include "components/prefs/testing_pref_service.h"
 
+#include <memory>
+
 #include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "components/prefs/default_pref_store.h"
@@ -21,15 +23,15 @@ TestingPrefServiceBase<PrefService, PrefRegistry>::TestingPrefServiceBase(
     PrefRegistry* pref_registry,
     PrefNotifierImpl* pref_notifier)
     : PrefService(
-          pref_notifier,
-          new PrefValueStore(managed_prefs,
-                             nullptr,
-                             extension_prefs,
-                             nullptr,
-                             user_prefs,
-                             recommended_prefs,
-                             pref_registry->defaults().get(),
-                             pref_notifier),
+          std::unique_ptr<PrefNotifierImpl>(pref_notifier),
+          std::make_unique<PrefValueStore>(managed_prefs,
+                                           nullptr,
+                                           extension_prefs,
+                                           nullptr,
+                                           user_prefs,
+                                           recommended_prefs,
+                                           pref_registry->defaults().get(),
+                                           pref_notifier),
           user_prefs,
           pref_registry,
           base::Bind(&TestingPrefServiceBase<PrefService,

components/prefs/testing_pref_service.h

@@ -29,16 +29,16 @@ class TestingPrefServiceBase : public SuperPrefService {
  public:
   virtual ~TestingPrefServiceBase();
 
-  // Read the value of a preference from the managed layer. Returns NULL if the
+  // Reads the value of a preference from the managed layer. Returns NULL if the
   // preference is not defined at the managed layer.
   const base::Value* GetManagedPref(const std::string& path) const;
 
-  // Set a preference on the managed layer and fire observers if the preference
-  // changed.
+  // Sets a preference on the managed layer and fires observers if the
+  // preference changed.
   void SetManagedPref(const std::string& path,
                       std::unique_ptr<base::Value> value);
 
-  // Clear the preference on the managed layer and fire observers if the
+  // Clears the preference on the managed layer and fire observers if the
   // preference has been defined previously.
   void RemoveManagedPref(const std::string& path);
 

components/sync_preferences/pref_service_syncable.cc

@@ -4,6 +4,8 @@
 
 #include "components/sync_preferences/pref_service_syncable.h"
 
+#include <utility>
+
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/files/file_path.h"
@@ -26,16 +28,16 @@
 namespace sync_preferences {
 
 PrefServiceSyncable::PrefServiceSyncable(
-    PrefNotifierImpl* pref_notifier,
-    PrefValueStore* pref_value_store,
+    std::unique_ptr<PrefNotifierImpl> pref_notifier,
+    std::unique_ptr<PrefValueStore> pref_value_store,
     PersistentPrefStore* user_prefs,
     user_prefs::PrefRegistrySyncable* pref_registry,
     const PrefModelAssociatorClient* pref_model_associator_client,
     base::Callback<void(PersistentPrefStore::PrefReadError)>
         read_error_callback,
     bool async)
-    : PrefService(pref_notifier,
-                  pref_value_store,
+    : PrefService(std::move(pref_notifier),
+                  std::move(pref_value_store),
                   user_prefs,
                   pref_registry,
                   read_error_callback,
@@ -48,7 +50,7 @@ PrefServiceSyncable::PrefServiceSyncable(
   priority_pref_sync_associator_.SetPrefService(this);
 
   // Let PrefModelAssociators know about changes to preference values.
-  pref_value_store->set_callback(base::Bind(
+  pref_value_store_->set_callback(base::Bind(
       &PrefServiceSyncable::ProcessPrefChange, base::Unretained(this)));
 
   // Add already-registered syncable preferences to PrefModelAssociator.
@@ -78,7 +80,7 @@ PrefServiceSyncable* PrefServiceSyncable::CreateIncognitoPrefService(
     const std::vector<const char*>& overlay_pref_names,
     std::unique_ptr<PrefValueStore::Delegate> delegate) {
   pref_service_forked_ = true;
-  PrefNotifierImpl* pref_notifier = new PrefNotifierImpl();
+  auto pref_notifier = std::make_unique<PrefNotifierImpl>();
 
   scoped_refptr<user_prefs::PrefRegistrySyncable> forked_registry =
       static_cast<user_prefs::PrefRegistrySyncable*>(pref_registry_.get())
@@ -96,16 +98,17 @@ PrefServiceSyncable* PrefServiceSyncable::CreateIncognitoPrefService(
   for (const char* overlay_pref_name : overlay_pref_names)
     incognito_pref_store->RegisterOverlayPref(overlay_pref_name);
 
+  auto pref_value_store = pref_value_store_->CloneAndSpecialize(
+      nullptr,  // managed
+      nullptr,  // supervised_user
+      incognito_extension_pref_store,
+      nullptr,  // command_line_prefs
+      incognito_pref_store.get(),
+      nullptr,  // recommended
+      forked_registry->defaults().get(), pref_notifier.get(),
+      std::move(delegate));
   PrefServiceSyncable* incognito_service = new PrefServiceSyncable(
-      pref_notifier,
-      pref_value_store_->CloneAndSpecialize(nullptr,  // managed
-                                            nullptr,  // supervised_user
-                                            incognito_extension_pref_store,
-                                            nullptr,  // command_line_prefs
-                                            incognito_pref_store.get(),
-                                            nullptr,  // recommended
-                                            forked_registry->defaults().get(),
-                                            pref_notifier, std::move(delegate)),
+      std::move(pref_notifier), std::move(pref_value_store),
       incognito_pref_store.get(), forked_registry.get(),
       pref_sync_associator_.client(), read_error_callback_, false);
   return incognito_service;

components/sync_preferences/pref_service_syncable.h

@@ -7,6 +7,7 @@
 
 #include <stdint.h>
 
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -35,8 +36,8 @@ class PrefServiceSyncable : public PrefService {
   // You may wish to use PrefServiceFactory or one of its subclasses
   // for simplified construction.
   PrefServiceSyncable(
-      PrefNotifierImpl* pref_notifier,
-      PrefValueStore* pref_value_store,
+      std::unique_ptr<PrefNotifierImpl> pref_notifier,
+      std::unique_ptr<PrefValueStore> pref_value_store,
       PersistentPrefStore* user_prefs,
       user_prefs::PrefRegistrySyncable* pref_registry,
       const PrefModelAssociatorClient* pref_model_associato_client,

components/sync_preferences/pref_service_syncable_factory.cc

@@ -4,6 +4,8 @@
 
 #include "components/sync_preferences/pref_service_syncable_factory.h"
 
+#include <memory>
+
 #include "base/trace_event/trace_event.h"
 #include "build/build_config.h"
 #include "components/pref_registry/pref_registry_syncable.h"
@@ -56,18 +58,16 @@ std::unique_ptr<PrefServiceSyncable> PrefServiceSyncableFactory::CreateSyncable(
     user_prefs::PrefRegistrySyncable* pref_registry,
     std::unique_ptr<PrefValueStore::Delegate> delegate) {
   TRACE_EVENT0("browser", "PrefServiceSyncableFactory::CreateSyncable");
-  PrefNotifierImpl* pref_notifier = new PrefNotifierImpl();
-
-  std::unique_ptr<PrefServiceSyncable> pref_service(new PrefServiceSyncable(
-      pref_notifier,
-      new PrefValueStore(managed_prefs_.get(), supervised_user_prefs_.get(),
-                         extension_prefs_.get(), command_line_prefs_.get(),
-                         user_prefs_.get(), recommended_prefs_.get(),
-                         pref_registry->defaults().get(), pref_notifier,
-                         std::move(delegate)),
-      user_prefs_.get(), pref_registry, pref_model_associator_client_,
-      read_error_callback_, async_));
-  return pref_service;
+  auto pref_notifier = std::make_unique<PrefNotifierImpl>();
+  auto pref_value_store = std::make_unique<PrefValueStore>(
+      managed_prefs_.get(), supervised_user_prefs_.get(),
+      extension_prefs_.get(), command_line_prefs_.get(), user_prefs_.get(),
+      recommended_prefs_.get(), pref_registry->defaults().get(),
+      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_);
 }
 
 }  // namespace sync_preferences

components/sync_preferences/testing_pref_service_syncable.cc

@@ -20,15 +20,15 @@ TestingPrefServiceBase<sync_preferences::PrefServiceSyncable,
                            user_prefs::PrefRegistrySyncable* pref_registry,
                            PrefNotifierImpl* pref_notifier)
     : sync_preferences::PrefServiceSyncable(
-          pref_notifier,
-          new PrefValueStore(managed_prefs,
-                             nullptr,          // supervised_user_prefs
-                             extension_prefs,  // extension_prefs
-                             nullptr,          // command_line_prefs
-                             user_prefs,
-                             recommended_prefs,
-                             pref_registry->defaults().get(),
-                             pref_notifier),
+          std::unique_ptr<PrefNotifierImpl>(pref_notifier),
+          std::make_unique<PrefValueStore>(managed_prefs,
+                                           nullptr,  // supervised_user_prefs
+                                           extension_prefs,  // extension_prefs
+                                           nullptr,  // command_line_prefs
+                                           user_prefs,
+                                           recommended_prefs,
+                                           pref_registry->defaults().get(),
+                                           pref_notifier),
           user_prefs,
           pref_registry,
           nullptr,  // pref_model_associator_client

services/preferences/pref_store_consistency_unittest.cc

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <memory>
 #include <utility>
 
 #include "base/containers/circular_deque.h"
@@ -68,17 +69,18 @@ class PrefServiceConnection : public mojom::PrefStoreObserver,
 
     pref_store_client_ =
         base::MakeRefCounted<PersistentPrefStoreClient>(std::move(connection));
-    PrefNotifierImpl* pref_notifier = new PrefNotifierImpl();
+    auto pref_notifier = std::make_unique<PrefNotifierImpl>();
     auto pref_registry = base::MakeRefCounted<PrefRegistrySimple>();
     pref_registry->RegisterIntegerPref(kKey, kInitialValue);
     pref_registry->RegisterIntegerPref(kOtherKey, kInitialValue);
     pref_registry->RegisterDictionaryPref(kDictionaryKey);
-    auto* pref_value_store = new PrefValueStore(
+    auto pref_value_store = std::make_unique<PrefValueStore>(
         nullptr, nullptr, nullptr, nullptr, pref_store_client_.get(), nullptr,
-        pref_registry->defaults().get(), pref_notifier);
-    pref_service_ = std::make_unique<::PrefService>(
-        pref_notifier, pref_value_store, pref_store_client_.get(),
-        pref_registry.get(), base::Bind(&DoNothingHandleReadError), true);
+        pref_registry->defaults().get(), pref_notifier.get());
+    pref_service_ = std::make_unique<PrefService>(
+        std::move(pref_notifier), std::move(pref_value_store),
+        pref_store_client_.get(), pref_registry.get(),
+        base::Bind(&DoNothingHandleReadError), true);
   }
 
   ~PrefServiceConnection() override {

services/preferences/public/cpp/in_process_service_factory.cc

@@ -39,7 +39,7 @@ class InProcessPrefServiceFactory::RegisteringDelegate
             PrefStore* user_prefs,
             PrefStore* recommended_prefs,
             PrefStore* default_prefs,
-            PrefNotifier* pref_notifier) override {
+            PrefNotifier* /*pref_notifier*/) override {
     if (!factory_)
       return;
 

services/preferences/public/cpp/pref_service_factory.cc

@@ -4,6 +4,9 @@
 
 #include "services/preferences/public/cpp/pref_service_factory.h"
 
+#include <memory>
+#include <utility>
+
 #include "base/callback_helpers.h"
 #include "components/prefs/overlay_user_pref_store.h"
 #include "components/prefs/persistent_pref_store.h"
@@ -108,14 +111,16 @@ void OnConnect(
     }
     persistent_pref_store = overlay_pref_store;
   }
-  PrefNotifierImpl* pref_notifier = new PrefNotifierImpl();
-  auto* pref_value_store = new PrefValueStore(
+  auto pref_notifier = std::make_unique<PrefNotifierImpl>();
+  auto pref_value_store = std::make_unique<PrefValueStore>(
       managed_prefs.get(), supervised_user_prefs.get(), extension_prefs.get(),
       command_line_prefs.get(), persistent_pref_store.get(),
-      recommended_prefs.get(), pref_registry->defaults().get(), pref_notifier);
+      recommended_prefs.get(), pref_registry->defaults().get(),
+      pref_notifier.get());
   auto pref_service = std::make_unique<PrefService>(
-      pref_notifier, pref_value_store, persistent_pref_store.get(),
-      pref_registry.get(), base::Bind(&DoNothingHandleReadError), true);
+      std::move(pref_notifier), std::move(pref_value_store),
+      persistent_pref_store.get(), pref_registry.get(),
+      base::Bind(&DoNothingHandleReadError), true);
   switch (pref_service->GetAllPrefStoresInitializationStatus()) {
     case PrefService::INITIALIZATION_STATUS_WAITING:
       pref_service->AddPrefInitObserver(base::Bind(&OnPrefServiceInit,

services/preferences/public/cpp/tests/persistent_pref_store_client_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "services/preferences/public/cpp/persistent_pref_store_client.h"
 
+#include <memory>
 #include <utility>
 
 #include "base/macros.h"
@@ -49,11 +50,12 @@ class PersistentPrefStoreClientTest : public testing::Test,
     auto pref_registry = base::MakeRefCounted<PrefRegistrySimple>();
     pref_registry->RegisterDictionaryPref(kDictionaryKey);
     pref_registry->RegisterDictionaryPref(kUninitializedDictionaryKey);
-    PrefNotifierImpl* pref_notifier = new PrefNotifierImpl;
+    auto pref_notifier = std::make_unique<PrefNotifierImpl>();
+    auto pref_value_store = std::make_unique<PrefValueStore>(
+        nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr,
+        pref_notifier.get());
     pref_service_ = std::make_unique<PrefService>(
-        pref_notifier,
-        new PrefValueStore(nullptr, nullptr, nullptr, nullptr, nullptr, nullptr,
-                           nullptr, pref_notifier),
+        std::move(pref_notifier), std::move(pref_value_store),
         persistent_pref_store_client.get(), pref_registry.get(),
         base::Bind(&DoNothingWithReadError), false);
   }