Clean up PrefServiceBuilder

* Use simple setters rather than With* methods.
* Explicitly transfer memory ownership.
* Rename "Builder" to "Factory"
* Eliminate the side-effect of resetting the builder from the Create* methods.

Along the way, fix a memory leak in the Android WebView.

BUG=315499
TEST=none (code should continue to compile and tests should continue to pass)
R=joi@chromium.org

Review URL: https://codereview.chromium.org/64193003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@235913 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 156 insertions, 186 deletions

diff --git a/base/prefs/pref_service.h b/base/prefs/pref_service.h
index e7b8914..599a2aa 100644
--- a/base/prefs/pref_service.h
+++ b/base/prefs/pref_service.h
@@ -138,7 +138,7 @@ class BASE_PREFS_EXPORT PrefService : public base::NonThreadSafe {
     const PrefService* pref_service_;
   };
 
-  // You may wish to use PrefServiceBuilder or one of its subclasses
+  // You may wish to use PrefServiceFactory or one of its subclasses
   // for simplified construction.
   PrefService(
       PrefNotifierImpl* pref_notifier,
diff --git a/base/prefs/pref_service_builder.cc b/base/prefs/pref_service_builder.cc
deleted file mode 100644
index 16b4565..0000000
--- a/base/prefs/pref_service_builder.cc
+++ /dev/null
@@ -1,110 +0,0 @@
-// Copyright (c) 2012 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 "base/prefs/pref_service_builder.h"
-
-#include "base/bind.h"
-#include "base/prefs/default_pref_store.h"
-#include "base/prefs/json_pref_store.h"
-#include "base/prefs/pref_notifier_impl.h"
-#include "base/prefs/pref_service.h"
-
-#include "base/prefs/pref_value_store.h"
-
-namespace {
-
-// Do-nothing default implementation.
-void DoNothingHandleReadError(PersistentPrefStore::PrefReadError error) {
-}
-
-}  // namespace
-
-PrefServiceBuilder::PrefServiceBuilder() {
-  ResetDefaultState();
-}
-
-PrefServiceBuilder::~PrefServiceBuilder() {
-}
-
-PrefServiceBuilder& PrefServiceBuilder::WithManagedPrefs(PrefStore* store) {
-  managed_prefs_ = store;
-  return *this;
-}
-
-PrefServiceBuilder& PrefServiceBuilder::WithSupervisedUserPrefs(
-    PrefStore* store) {
-  supervised_user_prefs_ = store;
-  return *this;
-}
-
-PrefServiceBuilder& PrefServiceBuilder::WithExtensionPrefs(PrefStore* store) {
-  extension_prefs_ = store;
-  return *this;
-}
-
-PrefServiceBuilder& PrefServiceBuilder::WithCommandLinePrefs(PrefStore* store) {
-  command_line_prefs_ = store;
-  return *this;
-}
-
-PrefServiceBuilder& PrefServiceBuilder::WithUserPrefs(
-    PersistentPrefStore* store) {
-  user_prefs_ = store;
-  return *this;
-}
-
-PrefServiceBuilder& PrefServiceBuilder::WithRecommendedPrefs(PrefStore* store) {
-  recommended_prefs_ = store;
-  return *this;
-}
-
-PrefServiceBuilder& PrefServiceBuilder::WithReadErrorCallback(
-    const base::Callback<void(PersistentPrefStore::PrefReadError)>&
-    read_error_callback) {
-  read_error_callback_ = read_error_callback;
-  return *this;
-}
-
-PrefServiceBuilder& PrefServiceBuilder::WithUserFilePrefs(
-    const base::FilePath& prefs_file,
-    base::SequencedTaskRunner* task_runner) {
-  user_prefs_ = new JsonPrefStore(prefs_file, task_runner);
-  return *this;
-}
-
-PrefServiceBuilder& PrefServiceBuilder::WithAsync(bool async) {
-  async_ = async;
-  return *this;
-}
-
-PrefService* PrefServiceBuilder::Create(PrefRegistry* pref_registry) {
-  PrefNotifierImpl* pref_notifier = new PrefNotifierImpl();
-  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),
-                      user_prefs_.get(),
-                      pref_registry,
-                      read_error_callback_,
-                      async_);
-  ResetDefaultState();
-  return pref_service;
-}
-
-void PrefServiceBuilder::ResetDefaultState() {
-  managed_prefs_ = NULL;
-  supervised_user_prefs_ = NULL;
-  extension_prefs_ = NULL;
-  command_line_prefs_ = NULL;
-  user_prefs_ = NULL;
-  recommended_prefs_ = NULL;
-  read_error_callback_ = base::Bind(&DoNothingHandleReadError);
-  async_ = false;
-}
diff --git a/base/prefs/pref_service_builder.h b/base/prefs/pref_service_builder.h
deleted file mode 100644
index 7af4392..0000000
--- a/base/prefs/pref_service_builder.h
+++ /dev/null
@@ -1,74 +0,0 @@
-// Copyright (c) 2012 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 BASE_PREFS_PREF_SERVICE_BUILDER_H_
-#define BASE_PREFS_PREF_SERVICE_BUILDER_H_
-
-#include "base/basictypes.h"
-#include "base/callback.h"
-#include "base/memory/ref_counted.h"
-#include "base/prefs/base_prefs_export.h"
-#include "base/prefs/persistent_pref_store.h"
-#include "base/prefs/pref_registry.h"
-#include "base/prefs/pref_store.h"
-
-class PrefService;
-
-namespace base {
-class FilePath;
-class SequencedTaskRunner;
-}
-
-// A class that allows convenient building of PrefService.
-class BASE_PREFS_EXPORT PrefServiceBuilder {
- public:
-  PrefServiceBuilder();
-  virtual ~PrefServiceBuilder();
-
-  // Functions for setting the various parameters of the PrefService to build.
-  // These take ownership of the |store| parameter.
-  PrefServiceBuilder& WithManagedPrefs(PrefStore* store);
-  PrefServiceBuilder& WithSupervisedUserPrefs(PrefStore* store);
-  PrefServiceBuilder& WithExtensionPrefs(PrefStore* store);
-  PrefServiceBuilder& WithCommandLinePrefs(PrefStore* store);
-  PrefServiceBuilder& WithUserPrefs(PersistentPrefStore* store);
-  PrefServiceBuilder& WithRecommendedPrefs(PrefStore* store);
-
-  // Sets up error callback for the PrefService.  A do-nothing default
-  // is provided if this is not called.
-  PrefServiceBuilder& WithReadErrorCallback(
-      const base::Callback<void(PersistentPrefStore::PrefReadError)>&
-          read_error_callback);
-
-  // Specifies to use an actual file-backed user pref store.
-  PrefServiceBuilder& WithUserFilePrefs(
-      const base::FilePath& prefs_file,
-      base::SequencedTaskRunner* task_runner);
-
-  PrefServiceBuilder& WithAsync(bool async);
-
-  // Creates a PrefService object initialized with the parameters from
-  // this builder.
-  virtual PrefService* Create(PrefRegistry* registry);
-
- protected:
-  virtual void ResetDefaultState();
-
-  scoped_refptr<PrefStore> managed_prefs_;
-  scoped_refptr<PrefStore> supervised_user_prefs_;
-  scoped_refptr<PrefStore> extension_prefs_;
-  scoped_refptr<PrefStore> command_line_prefs_;
-  scoped_refptr<PersistentPrefStore> user_prefs_;
-  scoped_refptr<PrefStore> recommended_prefs_;
-
-  base::Callback<void(PersistentPrefStore::PrefReadError)> read_error_callback_;
-
-  // Defaults to false.
-  bool async_;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(PrefServiceBuilder);
-};
-
-#endif  // BASE_PREFS_PREF_SERVICE_BUILDER_H_
diff --git a/base/prefs/pref_service_factory.cc b/base/prefs/pref_service_factory.cc
new file mode 100644
index 0000000..9c59853
--- /dev/null
+++ b/base/prefs/pref_service_factory.cc
@@ -0,0 +1,63 @@
+// Copyright 2013 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 "base/prefs/pref_service_factory.h"
+
+#include "base/bind.h"
+#include "base/prefs/default_pref_store.h"
+#include "base/prefs/json_pref_store.h"
+#include "base/prefs/pref_notifier_impl.h"
+#include "base/prefs/pref_service.h"
+
+#include "base/prefs/pref_value_store.h"
+
+namespace base {
+
+namespace {
+
+// Do-nothing default implementation.
+void DoNothingHandleReadError(PersistentPrefStore::PrefReadError error) {
+}
+
+}  // namespace
+
+PrefServiceFactory::PrefServiceFactory()
+    : managed_prefs_(NULL),
+      supervised_user_prefs_(NULL),
+      extension_prefs_(NULL),
+      command_line_prefs_(NULL),
+      user_prefs_(NULL),
+      recommended_prefs_(NULL),
+      read_error_callback_(base::Bind(&DoNothingHandleReadError)),
+      async_(false) {}
+
+PrefServiceFactory::~PrefServiceFactory() {}
+
+void PrefServiceFactory::SetUserPrefsFile(
+    const base::FilePath& prefs_file,
+    base::SequencedTaskRunner* task_runner) {
+  user_prefs_ = new JsonPrefStore(prefs_file, task_runner);
+}
+
+scoped_ptr<PrefService> PrefServiceFactory::Create(
+    PrefRegistry* pref_registry) {
+  PrefNotifierImpl* pref_notifier = new PrefNotifierImpl();
+  scoped_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),
+                      user_prefs_.get(),
+                      pref_registry,
+                      read_error_callback_,
+                      async_));
+  return pref_service.Pass();
+}
+
+}  // namespace base
diff --git a/base/prefs/pref_service_factory.h b/base/prefs/pref_service_factory.h
new file mode 100644
index 0000000..ca608c2
--- /dev/null
+++ b/base/prefs/pref_service_factory.h
@@ -0,0 +1,91 @@
+// Copyright 2013 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 BASE_PREFS_PREF_SERVICE_FACTORY_H_
+#define BASE_PREFS_PREF_SERVICE_FACTORY_H_
+
+#include "base/basictypes.h"
+#include "base/callback.h"
+#include "base/memory/ref_counted.h"
+#include "base/prefs/base_prefs_export.h"
+#include "base/prefs/persistent_pref_store.h"
+#include "base/prefs/pref_registry.h"
+#include "base/prefs/pref_store.h"
+
+class PrefService;
+
+namespace base {
+
+class FilePath;
+class SequencedTaskRunner;
+
+// A class that allows convenient building of PrefService.
+class BASE_PREFS_EXPORT PrefServiceFactory {
+ public:
+  PrefServiceFactory();
+  virtual ~PrefServiceFactory();
+
+  // Functions for setting the various parameters of the PrefService to build.
+  void set_managed_prefs(const scoped_refptr<PrefStore>& managed_prefs) {
+    managed_prefs_ = managed_prefs;
+  }
+  void set_supervised_user_prefs(
+      const scoped_refptr<PrefStore>& supervised_user_prefs) {
+    supervised_user_prefs_ = supervised_user_prefs;
+  }
+  void set_extension_prefs(const scoped_refptr<PrefStore>& extension_prefs) {
+    extension_prefs_ = extension_prefs;
+  }
+  void set_command_line_prefs(
+      const scoped_refptr<PrefStore>& command_line_prefs) {
+    command_line_prefs_ = command_line_prefs;
+  }
+  void set_user_prefs(const scoped_refptr<PersistentPrefStore>& user_prefs) {
+    user_prefs_ = user_prefs;
+  }
+  void set_recommended_prefs(
+      const scoped_refptr<PrefStore>& recommended_prefs) {
+    recommended_prefs_ = recommended_prefs;
+  }
+
+  // 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)>&
+          read_error_callback) {
+    read_error_callback_ = read_error_callback;
+  }
+
+  // Specifies to use an actual file-backed user pref store.
+  void SetUserPrefsFile(const base::FilePath& prefs_file,
+                        base::SequencedTaskRunner* task_runner);
+
+  void set_async(bool async) {
+    async_ = async;
+  }
+
+  // Creates a PrefService object initialized with the parameters from
+  // this factory.
+  scoped_ptr<PrefService> Create(PrefRegistry* registry);
+
+ protected:
+  scoped_refptr<PrefStore> managed_prefs_;
+  scoped_refptr<PrefStore> supervised_user_prefs_;
+  scoped_refptr<PrefStore> extension_prefs_;
+  scoped_refptr<PrefStore> command_line_prefs_;
+  scoped_refptr<PersistentPrefStore> user_prefs_;
+  scoped_refptr<PrefStore> recommended_prefs_;
+
+  base::Callback<void(PersistentPrefStore::PrefReadError)> read_error_callback_;
+
+  // Defaults to false.
+  bool async_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(PrefServiceFactory);
+};
+
+}  // namespace base
+
+#endif  // BASE_PREFS_PREF_SERVICE_FACTORY_H_
diff --git a/base/prefs/testing_pref_service.h b/base/prefs/testing_pref_service.h
index 1af4ba6..a9ab937 100644
--- a/base/prefs/testing_pref_service.h
+++ b/base/prefs/testing_pref_service.h
@@ -90,7 +90,7 @@ class TestingPrefServiceSimple
   // an existing TestingPrefServiceSimple instance. On a production
   // PrefService you would do all registrations before constructing
   // it, passing it a PrefRegistry via its constructor (or via
-  // e.g. PrefServiceBuilder).
+  // e.g. PrefServiceFactory).
   PrefRegistrySimple* registry();
 
  private: