Sanitize PrefStore interface.

This reworks the PrefStore interface, specifically:

- Split up the interface into PrefStore, which only provides reading functionality, and the derived PersistentPrefStore for the actual user pref store
- Remove the hurt-me-plenty prefs() function from PrefStore, instead provide Get/Set/Remove operations
- Remove special handling of default and user pref store from PrefValueStore and put it into PrefService
- Pref change notification handling now almost exclusively handled through PrefValueStore
- Adjust all consumers of these interfaces (but keep ConfigurationPolicyPrefStore untouched, that's up next on the list)

BUG=64232
TEST=existing unit tests

Review URL: http://codereview.chromium.org/5646003

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

5 files changed, 35 insertions, 110 deletions

diff --git a/chrome/test/reliability/page_load_test.cc b/chrome/test/reliability/page_load_test.cc
index 571c16a..fa06d56 100644
--- a/chrome/test/reliability/page_load_test.cc
+++ b/chrome/test/reliability/page_load_test.cc
@@ -50,6 +50,7 @@
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/net/url_fixer_upper.h"
 #include "chrome/browser/prefs/pref_service.h"
+#include "chrome/browser/prefs/pref_service_mock_builder.h"
 #include "chrome/common/automation_messages.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths.h"
@@ -63,8 +64,8 @@
 #include "chrome/test/automation/browser_proxy.h"
 #include "chrome/test/automation/tab_proxy.h"
 #include "chrome/test/automation/window_proxy.h"
-#include "chrome/test/ui/ui_test.h"
 #include "chrome/test/reliability/page_load_test.h"
+#include "chrome/test/ui/ui_test.h"
 #include "net/base/net_util.h"
 
 namespace {
@@ -532,12 +533,8 @@ class PageLoadTest : public UITest {
   // that was saved by the app as it closed.  The caller takes ownership of the
   // returned PrefService object.
   PrefService* GetLocalState() {
-    FilePath local_state_path = user_data_dir()
-        .Append(chrome::kLocalStateFilename);
-
-    PrefService* local_state = PrefService::CreateUserPrefService(
-        local_state_path);
-    return local_state;
+    FilePath path = user_data_dir().Append(chrome::kLocalStateFilename);
+    return PrefServiceMockBuilder().WithUserFilePrefs(path).Create();
   }
 
   void GetStabilityMetrics(NavigationMetrics* metrics) {
diff --git a/chrome/test/testing_pref_service.cc b/chrome/test/testing_pref_service.cc
index 9cc5321..66342bc 100644
--- a/chrome/test/testing_pref_service.cc
+++ b/chrome/test/testing_pref_service.cc
@@ -4,11 +4,11 @@
 
 #include "chrome/test/testing_pref_service.h"
 
+#include "chrome/browser/policy/configuration_policy_pref_store.h"
 #include "chrome/browser/prefs/command_line_pref_store.h"
-#include "chrome/browser/prefs/testing_pref_store.h"
 #include "chrome/browser/prefs/pref_notifier.h"
 #include "chrome/browser/prefs/pref_value_store.h"
-#include "chrome/browser/policy/configuration_policy_pref_store.h"
+#include "chrome/browser/prefs/testing_pref_store.h"
 
 // TODO(pamg): Instantiate no PrefStores by default. Allow callers to specify
 // which they want, and expand usage of this class to more unit tests.
@@ -23,37 +23,7 @@ TestingPrefService::TestingPrefService()
         NULL) {
 }
 
-TestingPrefService::TestingPrefService(
-    policy::ConfigurationPolicyProvider* managed_platform_provider,
-    policy::ConfigurationPolicyProvider* device_management_provider,
-    CommandLine* command_line)
-    : PrefService(
-        managed_platform_prefs_ = CreatePolicyPrefStoreFromProvider(
-            managed_platform_provider),
-        device_management_prefs_ =
-            CreatePolicyPrefStoreFromProvider(device_management_provider),
-        NULL,
-        CreateCommandLinePrefStore(command_line),
-        user_prefs_ = new TestingPrefStore(),
-        NULL,
-        NULL) {
-}
-
-PrefStore* TestingPrefService::CreatePolicyPrefStoreFromProvider(
-    policy::ConfigurationPolicyProvider* provider) {
-  if (provider)
-    return new policy::ConfigurationPolicyPrefStore(provider);
-  return new TestingPrefStore();
-}
-
-PrefStore* TestingPrefService::CreateCommandLinePrefStore(
-    CommandLine* command_line) {
-  if (command_line)
-    return new CommandLinePrefStore(command_line);
-  return new TestingPrefStore();
-}
-
-const Value* TestingPrefService::GetManagedPref(const char* path) {
+const Value* TestingPrefService::GetManagedPref(const char* path) const {
   return GetPref(managed_platform_prefs_, path);
 }
 
@@ -65,17 +35,7 @@ void TestingPrefService::RemoveManagedPref(const char* path) {
   RemovePref(managed_platform_prefs_, path);
 }
 
-void TestingPrefService::SetManagedPrefWithoutNotification(const char* path,
-                                                           Value* value) {
-  managed_platform_prefs_->prefs()->Set(path, value);
-}
-
-void TestingPrefService::RemoveManagedPrefWithoutNotification(
-    const char* path) {
-  managed_platform_prefs_->prefs()->Remove(path, NULL);
-}
-
-const Value* TestingPrefService::GetUserPref(const char* path) {
+const Value* TestingPrefService::GetUserPref(const char* path) const {
   return GetPref(user_prefs_, path);
 }
 
@@ -87,21 +47,19 @@ void TestingPrefService::RemoveUserPref(const char* path) {
   RemovePref(user_prefs_, path);
 }
 
-const Value* TestingPrefService::GetPref(PrefStore* pref_store,
-                                         const char* path) {
-  Value* result;
-  return pref_store->prefs()->Get(path, &result) ? result : NULL;
+const Value* TestingPrefService::GetPref(TestingPrefStore* pref_store,
+                                         const char* path) const {
+  Value* res;
+  return pref_store->GetValue(path, &res) == PrefStore::READ_OK ? res : NULL;
 }
 
-void TestingPrefService::SetPref(PrefStore* pref_store,
+void TestingPrefService::SetPref(TestingPrefStore* pref_store,
                                  const char* path,
                                  Value* value) {
-  pref_store->prefs()->Set(path, value);
-  pref_notifier()->OnPreferenceChanged(path);
+  pref_store->SetValue(path, value);
 }
 
-void TestingPrefService::RemovePref(PrefStore* pref_store,
+void TestingPrefService::RemovePref(TestingPrefStore* pref_store,
                                     const char* path) {
-  pref_store->prefs()->Remove(path, NULL);
-  pref_notifier()->OnPreferenceChanged(path);
+  pref_store->RemoveValue(path);
 }
diff --git a/chrome/test/testing_pref_service.h b/chrome/test/testing_pref_service.h
index c5adc021..25349b6 100644
--- a/chrome/test/testing_pref_service.h
+++ b/chrome/test/testing_pref_service.h
@@ -8,11 +8,7 @@
 
 #include "chrome/browser/prefs/pref_service.h"
 
-class CommandLine;
-namespace policy {
-class ConfigurationPolicyProvider;
-}
-class PrefStore;
+class TestingPrefStore;
 
 // A PrefService subclass for testing. It operates totally in memory and
 // provides additional API for manipulating preferences at the different levels
@@ -21,23 +17,11 @@ class TestingPrefService : public PrefService {
  public:
   // Create an empty instance.
   TestingPrefService();
-
-  // Create an instance that has a managed PrefStore and a command- line
-  // PrefStore. |managed_platform_provider| contains the provider with which to
-  // initialize the managed platform PrefStore. If it is NULL, then a
-  // TestingPrefStore will be created. |device_management_provider| contains the
-  // provider with which to initialize the device management
-  // PrefStore. |command_line| contains the provider with which to initialize
-  // the command line PrefStore. If it is NULL then a TestingPrefStore will be
-  // created as the command line PrefStore.
-  TestingPrefService(
-      policy::ConfigurationPolicyProvider* managed_platform_provider,
-      policy::ConfigurationPolicyProvider* device_management_provider,
-      CommandLine* command_line);
+  virtual ~TestingPrefService() {}
 
   // Read the value of a preference from the managed layer. Returns NULL if the
   // preference is not defined at the managed layer.
-  const Value* GetManagedPref(const char* path);
+  const Value* GetManagedPref(const char* path) const;
 
   // Set a preference on the managed layer and fire observers if the preference
   // changed. Assumes ownership of |value|.
@@ -47,48 +31,26 @@ class TestingPrefService : public PrefService {
   // preference has been defined previously.
   void RemoveManagedPref(const char* path);
 
-  // Set a preference on the managed layer.  Assumes ownership of |value|.
-  // We don't fire observers for each change because in the real code, the
-  // notification is sent after all the prefs have been loaded.  See
-  // ConfigurationPolicyPrefStore::ReadPrefs().
-  void SetManagedPrefWithoutNotification(const char* path, Value* value);
-
-  // Clear the preference on the managed layer.
-  // We don't fire observers for each change because in the real code, the
-  // notification is sent after all the prefs have been loaded.  See
-  // ConfigurationPolicyPrefStore::ReadPrefs().
-  void RemoveManagedPrefWithoutNotification(const char* path);
-
   // Similar to the above, but for user preferences.
-  const Value* GetUserPref(const char* path);
+  const Value* GetUserPref(const char* path) const;
   void SetUserPref(const char* path, Value* value);
   void RemoveUserPref(const char* path);
 
  private:
-  // Creates a ConfigurationPolicyPrefStore based on the provided
-  // |provider| or a TestingPrefStore if |provider| is NULL.
-  PrefStore* CreatePolicyPrefStoreFromProvider(
-      policy::ConfigurationPolicyProvider* provider);
-
-  // Creates a CommandLinePrefStore based on the supplied
-  // |command_line| or a TestingPrefStore if |command_line| is NULL.
-  PrefStore* CreateCommandLinePrefStore(CommandLine* command_line);
-
   // Reads the value of the preference indicated by |path| from |pref_store|.
   // Returns NULL if the preference was not found.
-  const Value* GetPref(PrefStore* pref_store, const char* path);
+  const Value* GetPref(TestingPrefStore* pref_store, const char* path) const;
 
   // Sets the value for |path| in |pref_store|.
-  void SetPref(PrefStore* pref_store, const char* path, Value* value);
+  void SetPref(TestingPrefStore* pref_store, const char* path, Value* value);
 
   // Removes the preference identified by |path| from |pref_store|.
-  void RemovePref(PrefStore* pref_store, const char* path);
+  void RemovePref(TestingPrefStore* pref_store, const char* path);
 
   // Pointers to the pref stores our value store uses.
-  PrefStore* managed_platform_prefs_;  // weak
-  PrefStore* device_management_prefs_;  // weak
-  PrefStore* user_prefs_;  // weak
-  PrefStore* default_prefs_;  // weak
+  TestingPrefStore* managed_platform_prefs_;  // weak
+  TestingPrefStore* device_management_prefs_;  // weak
+  TestingPrefStore* user_prefs_;  // weak
 
   DISALLOW_COPY_AND_ASSIGN(TestingPrefService);
 };
diff --git a/chrome/test/testing_profile.cc b/chrome/test/testing_profile.cc
index d94fa06..5266771 100644
--- a/chrome/test/testing_profile.cc
+++ b/chrome/test/testing_profile.cc
@@ -17,6 +17,7 @@
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/content_settings/host_content_settings_map.h"
 #include "chrome/browser/dom_ui/ntp_resource_cache.h"
+#include "chrome/browser/extensions/extension_pref_store.h"
 #include "chrome/browser/extensions/extensions_service.h"
 #include "chrome/browser/favicon_service.h"
 #include "chrome/browser/geolocation/geolocation_content_settings_map.h"
@@ -328,7 +329,10 @@ void TestingProfile::UseThemeProvider(BrowserThemeProvider* theme_provider) {
 scoped_refptr<ExtensionsService> TestingProfile::CreateExtensionsService(
     const CommandLine* command_line,
     const FilePath& install_directory) {
-  extension_prefs_.reset(new ExtensionPrefs(GetPrefs(),install_directory));
+  extension_pref_store_.reset(new ExtensionPrefStore);
+  extension_prefs_.reset(new ExtensionPrefs(GetPrefs(),
+                                            install_directory,
+                                            extension_pref_store_.get()));
   extensions_service_ = new ExtensionsService(this,
                                               command_line,
                                               install_directory,
diff --git a/chrome/test/testing_profile.h b/chrome/test/testing_profile.h
index 70395bb..4f53925 100644
--- a/chrome/test/testing_profile.h
+++ b/chrome/test/testing_profile.h
@@ -25,6 +25,7 @@ class BookmarkModel;
 class BrowserThemeProvider;
 class CommandLine;
 class DesktopNotificationService;
+class ExtensionPrefStore;
 class ExtensionPrefs;
 class FaviconService;
 class FindBarState;
@@ -406,6 +407,9 @@ class TestingProfile : public Profile {
   FilePath last_selected_directory_;
   scoped_refptr<history::TopSites> top_sites_;  // For history and thumbnails.
 
+  // Extension pref store, created for use by |extension_prefs_|.
+  scoped_ptr<ExtensionPrefStore> extension_pref_store_;
+
   // The Extension Preferences. Only created if CreateExtensionsService is
   // invoked.
   scoped_ptr<ExtensionPrefs> extension_prefs_;