Clean up pref change notification handling.

chrome/browser/prefs/pref_service_unittest.cc

Index: chrome/browser/prefs/pref_service_unittest.cc
diff --git a/chrome/browser/prefs/pref_service_unittest.cc b/chrome/browser/prefs/pref_service_unittest.cc
index 7fc46082e4aba560d96da5e0b8c5a1eeb523b9ef..6575bdf3907a611ed4c5b9bbc8afd6c3a3d2b81a 100644
--- a/chrome/browser/prefs/pref_service_unittest.cc
+++ b/chrome/browser/prefs/pref_service_unittest.cc
@@ -12,14 +12,12 @@
 #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/dummy_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_value_store.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
-#include "chrome/common/notification_observer_mock.h"
-#include "chrome/common/notification_service.h"
-#include "chrome/common/notification_type.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/testing_pref_service.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -27,49 +25,6 @@
 
 using testing::_;
 using testing::Mock;
-using testing::Pointee;
-using testing::Property;
-
-namespace {
-
-class TestPrefObserver : public NotificationObserver {
- public:
-  TestPrefObserver(const PrefService* prefs,
-                   const std::string& pref_name,
-                   const std::string& new_pref_value)
-      : observer_fired_(false),
-        prefs_(prefs),
-        pref_name_(pref_name),
-        new_pref_value_(new_pref_value) {}
-  virtual ~TestPrefObserver() {}
-
-  virtual void Observe(NotificationType type,
-                       const NotificationSource& source,
-                       const NotificationDetails& details) {
-    EXPECT_EQ(type.value, NotificationType::PREF_CHANGED);
-    const PrefService* prefs_in = Source<PrefService>(source).ptr();
-    EXPECT_EQ(prefs_in, prefs_);
-    const std::string* pref_name_in = Details<std::string>(details).ptr();
-    EXPECT_EQ(*pref_name_in, pref_name_);
-    EXPECT_EQ(new_pref_value_, prefs_in->GetString("homepage"));
-    observer_fired_ = true;
-  }
-
-  bool observer_fired() { return observer_fired_; }
-
-  void Reset(const std::string& new_pref_value) {
-    observer_fired_ = false;
-    new_pref_value_ = new_pref_value;
-  }
-
- private:
-  bool observer_fired_;
-  const PrefService* prefs_;
-  const std::string pref_name_;
-  std::string new_pref_value_;
-};
-
-}  // namespace
 
 // TODO(port): port this test to POSIX.
 #if defined(OS_WIN)
@@ -102,33 +57,34 @@ TEST(PrefServiceTest, NoObserverFire) {
   const char pref_name[] = "homepage";
   prefs.RegisterStringPref(pref_name, std::string());
 
-  const std::string new_pref_value("http://www.google.com/");
-  TestPrefObserver obs(&prefs, pref_name, new_pref_value);
-
+  const char new_pref_value[] = "http://www.google.com/";
+  PrefObserverMock obs;
   PrefChangeRegistrar registrar;
   registrar.Init(&prefs);
   registrar.Add(pref_name, &obs);
-  // This should fire the checks in TestPrefObserver::Observe.
-  prefs.SetString(pref_name, new_pref_value);
 
-  // Make sure the observer was actually fired.
-  EXPECT_TRUE(obs.observer_fired());
+  // This should fire the checks in PrefObserverMock::Observe.
+  const StringValue expected_value(new_pref_value);
+  obs.Expect(&prefs, pref_name, &expected_value);
+  prefs.SetString(pref_name, new_pref_value);
+  Mock::VerifyAndClearExpectations(&obs);
 
   // Setting the pref to the same value should not set the pref value a second
   // time.
-  obs.Reset(new_pref_value);
+  EXPECT_CALL(obs, Observe(_, _, _)).Times(0);
   prefs.SetString(pref_name, new_pref_value);
-  EXPECT_FALSE(obs.observer_fired());
+  Mock::VerifyAndClearExpectations(&obs);
 
   // Clearing the pref should cause the pref to fire.
-  obs.Reset(std::string());
+  const StringValue expected_default_value("");
+  obs.Expect(&prefs, pref_name, &expected_default_value);
   prefs.ClearPref(pref_name);
-  EXPECT_TRUE(obs.observer_fired());
+  Mock::VerifyAndClearExpectations(&obs);
 
   // Clearing the pref again should not cause the pref to fire.
-  obs.Reset(std::string());
+  EXPECT_CALL(obs, Observe(_, _, _)).Times(0);
   prefs.ClearPref(pref_name);
-  EXPECT_FALSE(obs.observer_fired());
+  Mock::VerifyAndClearExpectations(&obs);
 }
 
 TEST(PrefServiceTest, HasPrefPath) {
@@ -156,35 +112,38 @@ TEST(PrefServiceTest, Observers) {
   prefs.SetUserPref(pref_name, Value::CreateStringValue("http://www.cnn.com"));
   prefs.RegisterStringPref(pref_name, std::string());
 
-  const std::string new_pref_value("http://www.google.com/");
-  TestPrefObserver obs(&prefs, pref_name, new_pref_value);
+  const char new_pref_value[] = "http://www.google.com/";
+  const StringValue expected_new_pref_value(new_pref_value);
+  PrefObserverMock obs;
   PrefChangeRegistrar registrar;
   registrar.Init(&prefs);
   registrar.Add(pref_name, &obs);
-  // This should fire the checks in TestPrefObserver::Observe.
-  prefs.SetString(pref_name, new_pref_value);
 
-  // Make sure the tests were actually run.
-  EXPECT_TRUE(obs.observer_fired());
+  // This should fire the checks in PrefObserverMock::Observe.
+  obs.Expect(&prefs, pref_name, &expected_new_pref_value);
+  prefs.SetString(pref_name, new_pref_value);
+  Mock::VerifyAndClearExpectations(&obs);
 
   // Now try adding a second pref observer.
-  const std::string new_pref_value2("http://www.youtube.com/");
-  obs.Reset(new_pref_value2);
-  TestPrefObserver obs2(&prefs, pref_name, new_pref_value2);
+  const char new_pref_value2[] = "http://www.youtube.com/";
+  const StringValue expected_new_pref_value2(new_pref_value2);
+  PrefObserverMock obs2;
+  obs.Expect(&prefs, pref_name, &expected_new_pref_value2);
+  obs2.Expect(&prefs, pref_name, &expected_new_pref_value2);
   registrar.Add(pref_name, &obs2);
   // This should fire the checks in obs and obs2.
   prefs.SetString(pref_name, new_pref_value2);
-  EXPECT_TRUE(obs.observer_fired());
-  EXPECT_TRUE(obs2.observer_fired());
+  Mock::VerifyAndClearExpectations(&obs);
+  Mock::VerifyAndClearExpectations(&obs2);
 
   // Make sure obs2 still works after removing obs.
   registrar.Remove(pref_name, &obs);
-  obs.Reset(std::string());
-  obs2.Reset(new_pref_value);
+  EXPECT_CALL(obs, Observe(_, _, _)).Times(0);
+  obs2.Expect(&prefs, pref_name, &expected_new_pref_value);
   // This should only fire the observer in obs2.
   prefs.SetString(pref_name, new_pref_value);
-  EXPECT_FALSE(obs.observer_fired());
-  EXPECT_TRUE(obs2.observer_fired());
+  Mock::VerifyAndClearExpectations(&obs);
+  Mock::VerifyAndClearExpectations(&obs2);
 }
 
 TEST(PrefServiceTest, ProxyFromCommandLineNotPolicy) {
@@ -340,24 +299,11 @@ class PrefServiceSetValueTest : public testing::Test {
   static const char kValue[];
 
   PrefServiceSetValueTest()
-      : name_string_(kName),
-        null_value_(Value::CreateNullValue()) {}
-
-  void SetExpectNoNotification() {
-    EXPECT_CALL(observer_, Observe(_, _, _)).Times(0);
-  }
-
-  void SetExpectPrefChanged() {
-    EXPECT_CALL(observer_,
-                Observe(NotificationType(NotificationType::PREF_CHANGED), _,
-                        Property(&Details<std::string>::ptr,
-                                 Pointee(name_string_))));
-  }
+      : null_value_(Value::CreateNullValue()) {}
 
   TestingPrefService prefs_;
-  std::string name_string_;
   scoped_ptr<Value> null_value_;
-  NotificationObserverMock observer_;
+  PrefObserverMock observer_;
 };
 
 const char PrefServiceSetValueTest::kName[] = "name";
@@ -365,8 +311,7 @@ const char PrefServiceSetValueTest::kValue[] = "value";
 
 TEST_F(PrefServiceSetValueTest, SetStringValue) {
   const char default_string[] = "default";
-  const scoped_ptr<Value> default_value(
-      Value::CreateStringValue(default_string));
+  const StringValue default_value(default_string);
   prefs_.RegisterStringPref(kName, default_string);
 
   PrefChangeRegistrar registrar;
@@ -374,18 +319,18 @@ TEST_F(PrefServiceSetValueTest, SetStringValue) {
   registrar.Add(kName, &observer_);
 
   // Changing the controlling store from default to user triggers notification.
-  SetExpectPrefChanged();
-  prefs_.Set(kName, *default_value);
+  observer_.Expect(&prefs_, kName, &default_value);
+  prefs_.Set(kName, default_value);
   Mock::VerifyAndClearExpectations(&observer_);
 
-  SetExpectNoNotification();
-  prefs_.Set(kName, *default_value);
+  EXPECT_CALL(observer_, Observe(_, _, _)).Times(0);
+  prefs_.Set(kName, default_value);
   Mock::VerifyAndClearExpectations(&observer_);
 
-  const scoped_ptr<Value> new_value(Value::CreateStringValue(kValue));
-  SetExpectPrefChanged();
-  prefs_.Set(kName, *new_value);
-  EXPECT_EQ(kValue, prefs_.GetString(kName));
+  StringValue new_value(kValue);
+  observer_.Expect(&prefs_, kName, &new_value);
+  prefs_.Set(kName, new_value);
+  Mock::VerifyAndClearExpectations(&observer_);
 }
 
 TEST_F(PrefServiceSetValueTest, SetDictionaryValue) {
@@ -396,30 +341,23 @@ TEST_F(PrefServiceSetValueTest, SetDictionaryValue) {
 
   // Dictionary values are special: setting one to NULL is the same as clearing
   // the user value, allowing the NULL default to take (or keep) control.
-  SetExpectNoNotification();
+  EXPECT_CALL(observer_, Observe(_, _, _)).Times(0);
   prefs_.Set(kName, *null_value_);
   Mock::VerifyAndClearExpectations(&observer_);
 
   DictionaryValue new_value;
   new_value.SetString(kName, kValue);
-  SetExpectPrefChanged();
+  observer_.Expect(&prefs_, kName, &new_value);
   prefs_.Set(kName, new_value);
   Mock::VerifyAndClearExpectations(&observer_);
-  DictionaryValue* dict = prefs_.GetMutableDictionary(kName);
-  EXPECT_EQ(1U, dict->size());
-  std::string out_value;
-  dict->GetString(kName, &out_value);
-  EXPECT_EQ(kValue, out_value);
 
-  SetExpectNoNotification();
+  EXPECT_CALL(observer_, Observe(_, _, _)).Times(0);
   prefs_.Set(kName, new_value);
   Mock::VerifyAndClearExpectations(&observer_);
 
-  SetExpectPrefChanged();
+  observer_.Expect(&prefs_, kName, null_value_.get());
   prefs_.Set(kName, *null_value_);
   Mock::VerifyAndClearExpectations(&observer_);
-  dict = prefs_.GetMutableDictionary(kName);
-  EXPECT_EQ(0U, dict->size());
 }
 
 TEST_F(PrefServiceSetValueTest, SetListValue) {
@@ -430,28 +368,21 @@ TEST_F(PrefServiceSetValueTest, SetListValue) {
 
   // List values are special: setting one to NULL is the same as clearing the
   // user value, allowing the NULL default to take (or keep) control.
-  SetExpectNoNotification();
+  EXPECT_CALL(observer_, Observe(_, _, _)).Times(0);
   prefs_.Set(kName, *null_value_);
   Mock::VerifyAndClearExpectations(&observer_);
 
   ListValue new_value;
   new_value.Append(Value::CreateStringValue(kValue));
-  SetExpectPrefChanged();
+  observer_.Expect(&prefs_, kName, &new_value);
   prefs_.Set(kName, new_value);
   Mock::VerifyAndClearExpectations(&observer_);
-  const ListValue* list = prefs_.GetMutableList(kName);
-  ASSERT_EQ(1U, list->GetSize());
-  std::string out_value;
-  list->GetString(0, &out_value);
-  EXPECT_EQ(kValue, out_value);
 
-  SetExpectNoNotification();
+  EXPECT_CALL(observer_, Observe(_, _, _)).Times(0);
   prefs_.Set(kName, new_value);
   Mock::VerifyAndClearExpectations(&observer_);
 
-  SetExpectPrefChanged();
+  observer_.Expect(&prefs_, kName, null_value_.get());
   prefs_.Set(kName, *null_value_);
   Mock::VerifyAndClearExpectations(&observer_);
-  list = prefs_.GetMutableList(kName);
-  EXPECT_EQ(0U, list->GetSize());
 }