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

1 files changed, 74 insertions, 69 deletions

diff --git a/chrome/browser/prefs/pref_service_unittest.cc b/chrome/browser/prefs/pref_service_unittest.cc
index 6575bdf..8ada07a 100644
--- a/chrome/browser/prefs/pref_service_unittest.cc
+++ b/chrome/browser/prefs/pref_service_unittest.cc
@@ -12,10 +12,11 @@
 #include "chrome/browser/policy/mock_configuration_policy_provider.h"
 #include "chrome/browser/prefs/browser_prefs.h"
 #include "chrome/browser/prefs/command_line_pref_store.h"
-#include "chrome/browser/prefs/testing_pref_store.h"
 #include "chrome/browser/prefs/pref_change_registrar.h"
 #include "chrome/browser/prefs/pref_observer_mock.h"
+#include "chrome/browser/prefs/pref_service_mock_builder.h"
 #include "chrome/browser/prefs/pref_value_store.h"
+#include "chrome/browser/prefs/testing_pref_store.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
@@ -146,18 +147,6 @@ TEST(PrefServiceTest, Observers) {
   Mock::VerifyAndClearExpectations(&obs2);
 }
 
-TEST(PrefServiceTest, ProxyFromCommandLineNotPolicy) {
-  CommandLine command_line(CommandLine::NO_PROGRAM);
-  command_line.AppendSwitch(switches::kProxyAutoDetect);
-  TestingPrefService prefs(NULL, NULL, &command_line);
-  browser::RegisterUserPrefs(&prefs);
-  EXPECT_TRUE(prefs.GetBoolean(prefs::kProxyAutoDetect));
-  const PrefService::Preference* pref =
-      prefs.FindPreference(prefs::kProxyAutoDetect);
-  ASSERT_TRUE(pref);
-  EXPECT_FALSE(pref->IsManaged());
-}
-
 TEST(PrefServiceTest, ProxyPolicyOverridesCommandLineOptions) {
   CommandLine command_line(CommandLine::NO_PROGRAM);
   command_line.AppendSwitchASCII(switches::kProxyBypassList, "123");
@@ -177,24 +166,28 @@ TEST(PrefServiceTest, ProxyPolicyOverridesCommandLineOptions) {
 
   // First verify that command-line options are set correctly when
   // there is no policy in effect.
-  TestingPrefService prefs(NULL, NULL, &command_line);
-  browser::RegisterUserPrefs(&prefs);
-  EXPECT_FALSE(prefs.GetBoolean(prefs::kProxyAutoDetect));
-  EXPECT_FALSE(prefs.GetBoolean(prefs::kNoProxyServer));
-  EXPECT_EQ("789", prefs.GetString(prefs::kProxyServer));
-  EXPECT_EQ("456", prefs.GetString(prefs::kProxyPacUrl));
-  EXPECT_EQ("123", prefs.GetString(prefs::kProxyBypassList));
+  PrefServiceMockBuilder builder;
+  builder.WithCommandLine(&command_line);
+  scoped_ptr<PrefService> prefs(builder.Create());
+  browser::RegisterUserPrefs(prefs.get());
+  EXPECT_FALSE(prefs->GetBoolean(prefs::kProxyAutoDetect));
+  EXPECT_FALSE(prefs->GetBoolean(prefs::kNoProxyServer));
+  EXPECT_EQ("789", prefs->GetString(prefs::kProxyServer));
+  EXPECT_EQ("456", prefs->GetString(prefs::kProxyPacUrl));
+  EXPECT_EQ("123", prefs->GetString(prefs::kProxyBypassList));
 
   // Try a second time time with the managed PrefStore in place, the
   // manual proxy policy should have removed all traces of the command
   // line and replaced them with the policy versions.
-  TestingPrefService prefs2(provider.get(), NULL, &command_line);
-  browser::RegisterUserPrefs(&prefs2);
-  EXPECT_FALSE(prefs2.GetBoolean(prefs::kProxyAutoDetect));
-  EXPECT_FALSE(prefs2.GetBoolean(prefs::kNoProxyServer));
-  EXPECT_EQ("ghi", prefs2.GetString(prefs::kProxyServer));
-  EXPECT_EQ("def", prefs2.GetString(prefs::kProxyPacUrl));
-  EXPECT_EQ("abc", prefs2.GetString(prefs::kProxyBypassList));
+  builder.WithCommandLine(&command_line);
+  builder.WithManagedPlatformProvider(provider.get());
+  scoped_ptr<PrefService> prefs2(builder.Create());
+  browser::RegisterUserPrefs(prefs2.get());
+  EXPECT_FALSE(prefs2->GetBoolean(prefs::kProxyAutoDetect));
+  EXPECT_FALSE(prefs2->GetBoolean(prefs::kNoProxyServer));
+  EXPECT_EQ("ghi", prefs2->GetString(prefs::kProxyServer));
+  EXPECT_EQ("def", prefs2->GetString(prefs::kProxyPacUrl));
+  EXPECT_EQ("abc", prefs2->GetString(prefs::kProxyBypassList));
 }
 
 TEST(PrefServiceTest, ProxyPolicyOverridesUnrelatedCommandLineOptions) {
@@ -210,25 +203,29 @@ TEST(PrefServiceTest, ProxyPolicyOverridesUnrelatedCommandLineOptions) {
 
   // First verify that command-line options are set correctly when
   // there is no policy in effect.
-  TestingPrefService prefs(NULL, NULL, &command_line);
-  browser::RegisterUserPrefs(&prefs);
-  EXPECT_FALSE(prefs.GetBoolean(prefs::kProxyAutoDetect));
-  EXPECT_FALSE(prefs.GetBoolean(prefs::kNoProxyServer));
-  EXPECT_EQ("789", prefs.GetString(prefs::kProxyServer));
-  EXPECT_EQ("456", prefs.GetString(prefs::kProxyPacUrl));
-  EXPECT_EQ("123", prefs.GetString(prefs::kProxyBypassList));
+  PrefServiceMockBuilder builder;
+  builder.WithCommandLine(&command_line);
+  scoped_ptr<PrefService> prefs(builder.Create());
+  browser::RegisterUserPrefs(prefs.get());
+  EXPECT_FALSE(prefs->GetBoolean(prefs::kProxyAutoDetect));
+  EXPECT_FALSE(prefs->GetBoolean(prefs::kNoProxyServer));
+  EXPECT_EQ("789", prefs->GetString(prefs::kProxyServer));
+  EXPECT_EQ("456", prefs->GetString(prefs::kProxyPacUrl));
+  EXPECT_EQ("123", prefs->GetString(prefs::kProxyBypassList));
 
   // Try a second time time with the managed PrefStore in place, the
   // no proxy policy should have removed all traces of the command
   // line proxy settings, even though they were not the specific one
   // set in policy.
-  TestingPrefService prefs2(provider.get(), NULL, &command_line);
-  browser::RegisterUserPrefs(&prefs2);
-  EXPECT_FALSE(prefs2.GetBoolean(prefs::kProxyAutoDetect));
-  EXPECT_FALSE(prefs2.GetBoolean(prefs::kNoProxyServer));
-  EXPECT_EQ(std::string(), prefs2.GetString(prefs::kProxyServer));
-  EXPECT_EQ(std::string(), prefs2.GetString(prefs::kProxyPacUrl));
-  EXPECT_EQ(std::string(), prefs2.GetString(prefs::kProxyBypassList));
+  builder.WithCommandLine(&command_line);
+  builder.WithManagedPlatformProvider(provider.get());
+  scoped_ptr<PrefService> prefs2(builder.Create());
+  browser::RegisterUserPrefs(prefs2.get());
+  EXPECT_FALSE(prefs2->GetBoolean(prefs::kProxyAutoDetect));
+  EXPECT_FALSE(prefs2->GetBoolean(prefs::kNoProxyServer));
+  EXPECT_EQ(std::string(), prefs2->GetString(prefs::kProxyServer));
+  EXPECT_EQ(std::string(), prefs2->GetString(prefs::kProxyPacUrl));
+  EXPECT_EQ(std::string(), prefs2->GetString(prefs::kProxyBypassList));
 }
 
 TEST(PrefServiceTest, ProxyPolicyOverridesCommandLineNoProxy) {
@@ -242,24 +239,28 @@ TEST(PrefServiceTest, ProxyPolicyOverridesCommandLineNoProxy) {
 
   // First verify that command-line options are set correctly when
   // there is no policy in effect.
-  TestingPrefService prefs(NULL, NULL, &command_line);
-  browser::RegisterUserPrefs(&prefs);
-  EXPECT_FALSE(prefs.GetBoolean(prefs::kProxyAutoDetect));
-  EXPECT_TRUE(prefs.GetBoolean(prefs::kNoProxyServer));
-  EXPECT_EQ(std::string(), prefs.GetString(prefs::kProxyServer));
-  EXPECT_EQ(std::string(), prefs.GetString(prefs::kProxyPacUrl));
-  EXPECT_EQ(std::string(), prefs.GetString(prefs::kProxyBypassList));
+  PrefServiceMockBuilder builder;
+  builder.WithCommandLine(&command_line);
+  scoped_ptr<PrefService> prefs(builder.Create());
+  browser::RegisterUserPrefs(prefs.get());
+  EXPECT_FALSE(prefs->GetBoolean(prefs::kProxyAutoDetect));
+  EXPECT_TRUE(prefs->GetBoolean(prefs::kNoProxyServer));
+  EXPECT_EQ(std::string(), prefs->GetString(prefs::kProxyServer));
+  EXPECT_EQ(std::string(), prefs->GetString(prefs::kProxyPacUrl));
+  EXPECT_EQ(std::string(), prefs->GetString(prefs::kProxyBypassList));
 
   // Try a second time time with the managed PrefStore in place, the
   // auto-detect should be overridden. The default pref store must be
   // in place with the appropriate default value for this to work.
-  TestingPrefService prefs2(provider.get(), NULL, &command_line);
-  browser::RegisterUserPrefs(&prefs2);
-  EXPECT_TRUE(prefs2.GetBoolean(prefs::kProxyAutoDetect));
-  EXPECT_FALSE(prefs2.GetBoolean(prefs::kNoProxyServer));
-  EXPECT_EQ(std::string(), prefs2.GetString(prefs::kProxyServer));
-  EXPECT_EQ(std::string(), prefs2.GetString(prefs::kProxyPacUrl));
-  EXPECT_EQ(std::string(), prefs2.GetString(prefs::kProxyBypassList));
+  builder.WithCommandLine(&command_line);
+  builder.WithManagedPlatformProvider(provider.get());
+  scoped_ptr<PrefService> prefs2(builder.Create());
+  browser::RegisterUserPrefs(prefs2.get());
+  EXPECT_TRUE(prefs2->GetBoolean(prefs::kProxyAutoDetect));
+  EXPECT_FALSE(prefs2->GetBoolean(prefs::kNoProxyServer));
+  EXPECT_EQ(std::string(), prefs2->GetString(prefs::kProxyServer));
+  EXPECT_EQ(std::string(), prefs2->GetString(prefs::kProxyPacUrl));
+  EXPECT_EQ(std::string(), prefs2->GetString(prefs::kProxyBypassList));
 }
 
 TEST(PrefServiceTest, ProxyPolicyOverridesCommandLineAutoDetect) {
@@ -273,24 +274,28 @@ TEST(PrefServiceTest, ProxyPolicyOverridesCommandLineAutoDetect) {
 
   // First verify that the auto-detect is set if there is no managed
   // PrefStore.
-  TestingPrefService prefs(NULL, NULL, &command_line);
-  browser::RegisterUserPrefs(&prefs);
-  EXPECT_TRUE(prefs.GetBoolean(prefs::kProxyAutoDetect));
-  EXPECT_FALSE(prefs.GetBoolean(prefs::kNoProxyServer));
-  EXPECT_EQ(std::string(), prefs.GetString(prefs::kProxyServer));
-  EXPECT_EQ(std::string(), prefs.GetString(prefs::kProxyPacUrl));
-  EXPECT_EQ(std::string(), prefs.GetString(prefs::kProxyBypassList));
+  PrefServiceMockBuilder builder;
+  builder.WithCommandLine(&command_line);
+  scoped_ptr<PrefService> prefs(builder.Create());
+  browser::RegisterUserPrefs(prefs.get());
+  EXPECT_TRUE(prefs->GetBoolean(prefs::kProxyAutoDetect));
+  EXPECT_FALSE(prefs->GetBoolean(prefs::kNoProxyServer));
+  EXPECT_EQ(std::string(), prefs->GetString(prefs::kProxyServer));
+  EXPECT_EQ(std::string(), prefs->GetString(prefs::kProxyPacUrl));
+  EXPECT_EQ(std::string(), prefs->GetString(prefs::kProxyBypassList));
 
   // Try a second time time with the managed PrefStore in place, the
   // auto-detect should be overridden. The default pref store must be
   // in place with the appropriate default value for this to work.
-  TestingPrefService prefs2(provider.get(), NULL, &command_line);
-  browser::RegisterUserPrefs(&prefs2);
-  EXPECT_FALSE(prefs2.GetBoolean(prefs::kProxyAutoDetect));
-  EXPECT_TRUE(prefs2.GetBoolean(prefs::kNoProxyServer));
-  EXPECT_EQ(std::string(), prefs2.GetString(prefs::kProxyServer));
-  EXPECT_EQ(std::string(), prefs2.GetString(prefs::kProxyPacUrl));
-  EXPECT_EQ(std::string(), prefs2.GetString(prefs::kProxyBypassList));
+  builder.WithCommandLine(&command_line);
+  builder.WithManagedPlatformProvider(provider.get());
+  scoped_ptr<PrefService> prefs2(builder.Create());
+  browser::RegisterUserPrefs(prefs2.get());
+  EXPECT_FALSE(prefs2->GetBoolean(prefs::kProxyAutoDetect));
+  EXPECT_TRUE(prefs2->GetBoolean(prefs::kNoProxyServer));
+  EXPECT_EQ(std::string(), prefs2->GetString(prefs::kProxyServer));
+  EXPECT_EQ(std::string(), prefs2->GetString(prefs::kProxyPacUrl));
+  EXPECT_EQ(std::string(), prefs2->GetString(prefs::kProxyBypassList));
 }
 
 class PrefServiceSetValueTest : public testing::Test {