Refactor PrefService, pulling the PREF_CHANGED notification infrastructure out so

it can be accessed by both the PrefService, for user preferences, and the individal
PrefStores, for preferences set "from below" (e.g., by configuration policy changes).

BUG=50722
TEST=covered by unit tests
Review URL: http://codereview.chromium.org/3052045

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

14 files changed, 692 insertions, 277 deletions

diff --git a/chrome/browser/extensions/extension_pref_store.cc b/chrome/browser/extensions/extension_pref_store.cc
index dbac8cc..c8f4b91 100644
--- a/chrome/browser/extensions/extension_pref_store.cc
+++ b/chrome/browser/extensions/extension_pref_store.cc
@@ -115,7 +115,7 @@ void ExtensionPrefStore::UpdateOnePref(const char* path) {
   }
 
   if (pref_service)
-    pref_service->FireObserversIfChanged(path, old_value.get());
+    pref_service->pref_notifier()->OnPreferenceSet(path, old_value.get());
 }
 
 void ExtensionPrefStore::UpdatePrefs(const PrefValueMap* pref_values) {
diff --git a/chrome/browser/extensions/extension_pref_store_unittest.cc b/chrome/browser/extensions/extension_pref_store_unittest.cc
index 05cd322..a05730a 100644
--- a/chrome/browser/extensions/extension_pref_store_unittest.cc
+++ b/chrome/browser/extensions/extension_pref_store_unittest.cc
@@ -2,8 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <gtest/gtest.h>
-
 #include <string>
 #include <vector>
 
@@ -15,6 +13,8 @@
 #include "chrome/browser/pref_value_store.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/test/testing_pref_service.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 
@@ -76,26 +76,34 @@ class TestExtensionPrefStore : public ExtensionPrefStore {
   PrefService* pref_service_;
 };
 
+// Mock PrefNotifier that allows the notifications to be tracked.
+class MockPrefNotifier : public PrefNotifier {
+ public:
+  MockPrefNotifier(PrefService* service, PrefValueStore* value_store)
+    : PrefNotifier(service, value_store) {}
+
+  virtual ~MockPrefNotifier() {}
+
+  MOCK_METHOD1(FireObservers, void(const char* path));
+};
+
+// Mock PrefService that allows the PrefNotifier to be injected.
 class MockPrefService : public PrefService {
  public:
-  explicit MockPrefService(PrefValueStore* value_store)
-      : PrefService(value_store),
-        fired_observers_(false) {}
-
-  // Tracks whether the observers would have been notified.
-  virtual void FireObserversIfChanged(const char* pref_name,
-                                      const Value* old_value) {
-    fired_observers_ = PrefIsChanged(pref_name, old_value);
+  explicit MockPrefService(PrefValueStore* pref_value_store)
+      : PrefService(pref_value_store) {
   }
 
-  bool fired_observers_;
+  void SetPrefNotifier(MockPrefNotifier* notifier) {
+    pref_notifier_.reset(notifier);
+  }
 };
 
-// Use static constants to avoid confusing std::map with hard-coded strings.
-static const char* kPref1 = "path1.subpath";
-static const char* kPref2 = "path2";
-static const char* kPref3 = "path3";
-static const char* kPref4 = "path4";
+// Use constants to avoid confusing std::map with hard-coded strings.
+const char kPref1[] = "path1.subpath";
+const char kPref2[] = "path2";
+const char kPref3[] = "path3";
+const char kPref4[] = "path4";
 
 }  // namespace
 
@@ -316,28 +324,36 @@ TEST(ExtensionPrefStoreTest, UninstallExtensionFromMiddle) {
 }
 
 TEST(ExtensionPrefStoreTest, NotifyWhenNeeded) {
+  using testing::Mock;
+
   TestExtensionPrefStore* eps = new TestExtensionPrefStore;
   ASSERT_TRUE(eps->ext1 != NULL);
 
   // The PrefValueStore takes ownership of the PrefStores; in this case, that's
   // only an ExtensionPrefStore. Likewise, the PrefService takes ownership of
-  // the PrefValueStore.
+  // the PrefValueStore and PrefNotifier.
   PrefValueStore* value_store = new TestingPrefService::TestingPrefValueStore(
       NULL, eps, NULL, NULL, NULL);
   scoped_ptr<MockPrefService> pref_service(new MockPrefService(value_store));
+  MockPrefNotifier* pref_notifier = new MockPrefNotifier(pref_service.get(),
+      value_store);
+  pref_service->SetPrefNotifier(pref_notifier);
+
   eps->SetPrefService(pref_service.get());
   pref_service->RegisterStringPref(kPref1, std::string());
 
+  EXPECT_CALL(*pref_notifier, FireObservers(kPref1));
   eps->InstallExtensionPref(eps->ext1, kPref1,
       Value::CreateStringValue("https://www.chromium.org"));
-  EXPECT_TRUE(pref_service->fired_observers_);
+  Mock::VerifyAndClearExpectations(pref_notifier);
+
+  EXPECT_CALL(*pref_notifier, FireObservers(kPref1)).Times(0);
   eps->InstallExtensionPref(eps->ext1, kPref1,
       Value::CreateStringValue("https://www.chromium.org"));
-  EXPECT_FALSE(pref_service->fired_observers_);
+  Mock::VerifyAndClearExpectations(pref_notifier);
+
+  EXPECT_CALL(*pref_notifier, FireObservers(kPref1)).Times(2);
   eps->InstallExtensionPref(eps->ext1, kPref1,
       Value::CreateStringValue("chrome://newtab"));
-  EXPECT_TRUE(pref_service->fired_observers_);
-
   eps->UninstallExtension(eps->ext1);
-  EXPECT_TRUE(pref_service->fired_observers_);
 }
diff --git a/chrome/browser/pref_notifier.cc b/chrome/browser/pref_notifier.cc
new file mode 100644
index 0000000..8cf5f1d
--- /dev/null
+++ b/chrome/browser/pref_notifier.cc
@@ -0,0 +1,140 @@
+// Copyright (c) 2010 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 "chrome/browser/pref_notifier.h"
+
+#include "base/stl_util-inl.h"
+#include "base/utf_string_conversions.h"
+#include "chrome/browser/policy/configuration_policy_pref_store.h"
+#include "chrome/browser/pref_service.h"
+#include "chrome/browser/pref_value_store.h"
+#include "chrome/common/notification_service.h"
+
+
+PrefNotifier::PrefNotifier(PrefService* service, PrefValueStore* value_store)
+    : pref_service_(service),
+      pref_value_store_(value_store) {
+  registrar_.Add(this,
+                 NotificationType(NotificationType::POLICY_CHANGED),
+                 NotificationService::AllSources());
+}
+
+PrefNotifier::~PrefNotifier() {
+  DCHECK(CalledOnValidThread());
+
+  // Verify that there are no pref observers when we shut down.
+  for (PrefObserverMap::iterator it = pref_observers_.begin();
+       it != pref_observers_.end(); ++it) {
+    NotificationObserverList::Iterator obs_iterator(*(it->second));
+    if (obs_iterator.GetNext()) {
+      LOG(WARNING) << "pref observer found at shutdown " << it->first;
+    }
+  }
+
+  STLDeleteContainerPairSecondPointers(pref_observers_.begin(),
+                                       pref_observers_.end());
+  pref_observers_.clear();
+}
+
+void PrefNotifier::OnPreferenceSet(const char* pref_name,
+                                   const Value* old_value) {
+  if (pref_value_store_->PrefHasChanged(pref_name, old_value))
+    FireObservers(pref_name);
+}
+
+void PrefNotifier::OnUserPreferenceSet(const char* pref_name,
+                                       const Value* old_value) {
+  // TODO(pamg): Adjust to account for source PrefStore.
+  OnPreferenceSet(pref_name, old_value);
+}
+
+void PrefNotifier::FireObservers(const char* path) {
+  DCHECK(CalledOnValidThread());
+
+  // Convert path to a std::string because the Details constructor requires a
+  // class.
+  std::string path_str(path);
+  PrefObserverMap::iterator observer_iterator = pref_observers_.find(path_str);
+  if (observer_iterator == pref_observers_.end())
+    return;
+
+  NotificationObserverList::Iterator it(*(observer_iterator->second));
+  NotificationObserver* observer;
+  while ((observer = it.GetNext()) != NULL) {
+    observer->Observe(NotificationType::PREF_CHANGED,
+                      Source<PrefService>(pref_service_),
+                      Details<std::string>(&path_str));
+  }
+}
+
+void PrefNotifier::AddPrefObserver(const char* path,
+                                   NotificationObserver* obs) {
+  // Get the pref observer list associated with the path.
+  NotificationObserverList* observer_list = NULL;
+  PrefObserverMap::iterator observer_iterator = pref_observers_.find(path);
+  if (observer_iterator == pref_observers_.end()) {
+    observer_list = new NotificationObserverList;
+    pref_observers_[path] = observer_list;
+  } else {
+    observer_list = observer_iterator->second;
+  }
+
+  // Verify that this observer doesn't already exist.
+  NotificationObserverList::Iterator it(*observer_list);
+  NotificationObserver* existing_obs;
+  while ((existing_obs = it.GetNext()) != NULL) {
+    DCHECK(existing_obs != obs) << path << " observer already registered";
+    if (existing_obs == obs)
+      return;
+  }
+
+  // Ok, safe to add the pref observer.
+  observer_list->AddObserver(obs);
+}
+
+void PrefNotifier::RemovePrefObserver(const char* path,
+                                     NotificationObserver* obs) {
+  DCHECK(CalledOnValidThread());
+
+  PrefObserverMap::iterator observer_iterator = pref_observers_.find(path);
+  if (observer_iterator == pref_observers_.end()) {
+    return;
+  }
+
+  NotificationObserverList* observer_list = observer_iterator->second;
+  observer_list->RemoveObserver(obs);
+}
+
+void PrefNotifier::FireObserversForRefreshedManagedPrefs(
+    std::vector<std::string> changed_prefs_paths) {
+  DCHECK(CalledOnValidThread());
+  std::vector<std::string>::const_iterator current;
+  for (current = changed_prefs_paths.begin();
+       current != changed_prefs_paths.end();
+       ++current) {
+    FireObservers(current->c_str());
+  }
+}
+
+void PrefNotifier::Observe(NotificationType type,
+                           const NotificationSource& source,
+                           const NotificationDetails& details) {
+  if (type == NotificationType::POLICY_CHANGED) {
+      PrefValueStore::AfterRefreshCallback* callback =
+          NewCallback(this,
+                      &PrefNotifier::FireObserversForRefreshedManagedPrefs);
+      // The notification of the policy refresh can come from any thread,
+      // but the manipulation of the PrefValueStore must happen on the UI
+      // thread, thus the policy refresh must be explicitly started on it.
+      ChromeThread::PostTask(
+          ChromeThread::UI, FROM_HERE,
+          NewRunnableMethod(
+              pref_value_store_,
+              &PrefValueStore::RefreshPolicyPrefs,
+              ConfigurationPolicyPrefStore::CreateManagedPolicyPrefStore(),
+              ConfigurationPolicyPrefStore::CreateRecommendedPolicyPrefStore(),
+
+              callback));
+  }
+}
diff --git a/chrome/browser/pref_notifier.h b/chrome/browser/pref_notifier.h
new file mode 100644
index 0000000..3779818
--- /dev/null
+++ b/chrome/browser/pref_notifier.h
@@ -0,0 +1,108 @@
+// Copyright (c) 2010 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 CHROME_BROWSER_PREF_NOTIFIER_H_
+#define CHROME_BROWSER_PREF_NOTIFIER_H_
+#pragma once
+
+#include <string>
+#include <vector>
+
+#include "base/hash_tables.h"
+#include "base/non_thread_safe.h"
+#include "base/observer_list.h"
+#include "chrome/common/notification_registrar.h"
+
+class NotificationObserver;
+class PrefService;
+class PrefValueStore;
+class Value;
+
+// Registers observers for particular preferences and sends notifications when
+// preference values or sources (i.e., which preference layer controls the
+// preference) change.
+class PrefNotifier : public NonThreadSafe,
+                     public NotificationObserver {
+ public:
+  // PrefStores must be listed here in order from highest to lowest priority.
+  //   MANAGED contains all managed (policy) preference values.
+  //   EXTENSION contains preference values set by extensions.
+  //   COMMAND_LINE contains preference values set by command-line switches.
+  //   USER contains all user-set preference values.
+  //   RECOMMENDED contains all recommended (policy) preference values.
+  // This enum is kept in pref_notifier.h rather than pref_value_store.h in
+  // order to minimize additional headers needed by the *PrefStore files.
+  enum PrefStoreType {
+    // INVALID_STORE is not associated with an actual PrefStore but used as
+    // an invalid marker, e.g. as a return value.
+    INVALID_STORE = -1,
+    MANAGED_STORE = 0,
+    EXTENSION_STORE,
+    COMMAND_LINE_STORE,
+    USER_STORE,
+    RECOMMENDED_STORE,
+    PREF_STORE_TYPE_MAX = RECOMMENDED_STORE
+  };
+
+  // The |service| with which this notifier is associated will be sent as the
+  // source of any notifications.
+  PrefNotifier(PrefService* service, PrefValueStore* value_store);
+
+  virtual ~PrefNotifier();
+
+  // For the given pref_name, fire any observer of the pref only if |old_value|
+  // is different from the current value.
+  // TODO(pamg): Also send notifications if the controlling store has changed.
+  void OnPreferenceSet(const char* pref_name, const Value* old_value);
+
+  // For the given pref_name, fire any observer of the pref only if |old_value|
+  // is different from the current value. Convenience method to be called when
+  // a preference is set in the USER_STORE.
+  void OnUserPreferenceSet(const char* pref_name, const Value* old_value);
+
+  // For the given pref_name, fire any observer of the pref. Virtual so it can
+  // be mocked for unit testing.
+  virtual void FireObservers(const char* path);
+
+  // If the pref at the given path changes, we call the observer's Observe
+  // method with NOTIFY_PREF_CHANGED.
+  void AddPrefObserver(const char* path, NotificationObserver* obs);
+  void RemovePrefObserver(const char* path, NotificationObserver* obs);
+
+ protected:
+  // A map from pref names to a list of observers.  Observers get fired in the
+  // order they are added. These should only be accessed externally for unit
+  // testing.
+  typedef ObserverList<NotificationObserver> NotificationObserverList;
+  typedef base::hash_map<std::string, NotificationObserverList*>
+      PrefObserverMap;
+  const PrefObserverMap* pref_observers() { return &pref_observers_; }
+
+ private:
+  // Weak references.
+  PrefService* pref_service_;
+  PrefValueStore* pref_value_store_;
+
+  NotificationRegistrar registrar_;
+
+  PrefObserverMap pref_observers_;
+
+  // Called after a policy refresh to notify relevant preference observers.
+  // |changed_prefs_paths| is the vector of preference paths changed by the
+  // policy update. It is passed by value and not reference because
+  // this method is called asynchronously as a callback from another thread.
+  // Copying the vector guarantees that the vector's lifecycle spans the
+  // method's invocation.
+  void FireObserversForRefreshedManagedPrefs(
+      std::vector<std::string> changed_prefs_paths);
+
+  // NotificationObserver methods:
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details);
+
+  DISALLOW_COPY_AND_ASSIGN(PrefNotifier);
+};
+
+#endif  // CHROME_BROWSER_PREF_NOTIFIER_H_
diff --git a/chrome/browser/pref_notifier_unittest.cc b/chrome/browser/pref_notifier_unittest.cc
new file mode 100644
index 0000000..a6c2da9
--- /dev/null
+++ b/chrome/browser/pref_notifier_unittest.cc
@@ -0,0 +1,282 @@
+// Copyright (c) 2010 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 "chrome/browser/pref_notifier.h"
+#include "chrome/browser/pref_service.h"
+#include "chrome/browser/pref_value_store.h"
+#include "chrome/common/notification_observer.h"
+#include "chrome/common/notification_type.h"
+#include "chrome/common/notification_service.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+
+namespace {
+
+const char kChangedPref[] = "changed_pref";
+const char kUnchangedPref[] = "unchanged_pref";
+
+bool DetailsAreChangedPref(const Details<std::string>& details) {
+  std::string* string_in = Details<std::string>(details).ptr();
+  return strcmp(string_in->c_str(), kChangedPref) == 0;
+}
+
+// Test PrefNotifier that allows tracking of observers and notifications.
+class MockPrefNotifier : public PrefNotifier {
+ public:
+  MockPrefNotifier(PrefService* prefs, PrefValueStore* value_store)
+      : PrefNotifier(prefs, value_store) {}
+
+  virtual ~MockPrefNotifier() {}
+
+  MOCK_METHOD1(FireObservers, void(const char* path));
+
+  void RealFireObservers(const char* path) {
+    PrefNotifier::FireObservers(path);
+  }
+
+  size_t CountObserver(const char* path, NotificationObserver* obs) {
+    PrefObserverMap::const_iterator observer_iterator =
+        pref_observers()->find(path);
+    if (observer_iterator == pref_observers()->end())
+      return false;
+
+    NotificationObserverList* observer_list = observer_iterator->second;
+    NotificationObserverList::Iterator it(*observer_list);
+    NotificationObserver* existing_obs;
+    size_t count = 0;
+    while ((existing_obs = it.GetNext()) != NULL) {
+      if (existing_obs == obs)
+        count++;
+    }
+
+    return count;
+  }
+};
+
+// Mock PrefValueStore that has no PrefStores and injects a simpler
+// PrefHasChanged().
+class MockPrefValueStore : public PrefValueStore {
+ public:
+  MockPrefValueStore() : PrefValueStore(NULL, NULL, NULL, NULL, NULL) {}
+
+  virtual ~MockPrefValueStore() {}
+
+  // This mock version returns true if the pref path starts with "changed".
+  virtual bool PrefHasChanged(const char* path, const Value* old_value) {
+    std::string path_str(path);
+    if (path_str.compare(0, 7, "changed") == 0)
+      return true;
+    return false;
+  }
+};
+
+// Mock PrefService that allows the PrefNotifier to be injected.
+class MockPrefService : public PrefService {
+ public:
+  explicit MockPrefService(PrefValueStore* pref_value_store)
+      : PrefService(pref_value_store) {}
+
+  void SetPrefNotifier(PrefNotifier* notifier) {
+    pref_notifier_.reset(notifier);
+  }
+};
+
+// Mock PrefObserver that verifies notifications.
+class MockPrefObserver : public NotificationObserver {
+ public:
+  virtual ~MockPrefObserver() {}
+
+  MOCK_METHOD3(Observe, void(NotificationType type,
+                             const NotificationSource& source,
+                             const NotificationDetails& details));
+};
+
+// Test fixture class.
+class PrefNotifierTest : public testing::Test {
+ protected:
+  virtual void SetUp() {
+    value_store_ = new MockPrefValueStore;
+    pref_service_.reset(new MockPrefService(value_store_));
+    notifier_ = new MockPrefNotifier(pref_service_.get(), value_store_);
+    pref_service_->SetPrefNotifier(notifier_);
+  }
+
+  // The PrefService takes ownership of the PrefValueStore and PrefNotifier.
+  PrefValueStore* value_store_;
+  MockPrefNotifier* notifier_;
+  scoped_ptr<MockPrefService> pref_service_;
+
+  MockPrefObserver obs1_;
+  MockPrefObserver obs2_;
+};
+
+
+TEST_F(PrefNotifierTest, OnPreferenceSet) {
+  EXPECT_CALL(*notifier_, FireObservers(kChangedPref));
+  EXPECT_CALL(*notifier_, FireObservers(kUnchangedPref)).Times(0);
+  notifier_->OnPreferenceSet(kChangedPref, NULL);
+  notifier_->OnPreferenceSet(kUnchangedPref, NULL);
+}
+
+TEST_F(PrefNotifierTest, OnUserPreferenceSet) {
+  EXPECT_CALL(*notifier_, FireObservers(kChangedPref));
+  EXPECT_CALL(*notifier_, FireObservers(kUnchangedPref)).Times(0);
+  notifier_->OnUserPreferenceSet(kChangedPref, NULL);
+  notifier_->OnUserPreferenceSet(kUnchangedPref, NULL);
+}
+
+TEST_F(PrefNotifierTest, AddAndRemovePrefObservers) {
+  const char pref_name[] = "homepage";
+  const char pref_name2[] = "proxy";
+
+  notifier_->AddPrefObserver(pref_name, &obs1_);
+  ASSERT_EQ(1u, notifier_->CountObserver(pref_name, &obs1_));
+  ASSERT_EQ(0u, notifier_->CountObserver(pref_name2, &obs1_));
+  ASSERT_EQ(0u, notifier_->CountObserver(pref_name, &obs2_));
+  ASSERT_EQ(0u, notifier_->CountObserver(pref_name2, &obs2_));
+
+  // Re-adding the same observer for the same pref doesn't change anything.
+  // Skip this in debug mode, since it hits a DCHECK and death tests aren't
+  // thread-safe.
+#if defined(NDEBUG)
+  notifier_->AddPrefObserver(pref_name, &obs1_);
+  ASSERT_EQ(1u, notifier_->CountObserver(pref_name, &obs1_));
+  ASSERT_EQ(0u, notifier_->CountObserver(pref_name2, &obs1_));
+  ASSERT_EQ(0u, notifier_->CountObserver(pref_name, &obs2_));
+  ASSERT_EQ(0u, notifier_->CountObserver(pref_name2, &obs2_));
+#endif  // NDEBUG
+
+  // Ensure that we can add the same observer to a different pref.
+  notifier_->AddPrefObserver(pref_name2, &obs1_);
+  ASSERT_EQ(1u, notifier_->CountObserver(pref_name, &obs1_));
+  ASSERT_EQ(1u, notifier_->CountObserver(pref_name2, &obs1_));
+  ASSERT_EQ(0u, notifier_->CountObserver(pref_name, &obs2_));
+  ASSERT_EQ(0u, notifier_->CountObserver(pref_name2, &obs2_));
+
+  // Ensure that we can add another observer to the same pref.
+  notifier_->AddPrefObserver(pref_name, &obs2_);
+  ASSERT_EQ(1u, notifier_->CountObserver(pref_name, &obs1_));
+  ASSERT_EQ(1u, notifier_->CountObserver(pref_name2, &obs1_));
+  ASSERT_EQ(1u, notifier_->CountObserver(pref_name, &obs2_));
+  ASSERT_EQ(0u, notifier_->CountObserver(pref_name2, &obs2_));
+
+  // Ensure that we can remove all observers, and that removing a non-existent
+  // observer is harmless.
+  notifier_->RemovePrefObserver(pref_name, &obs1_);
+  ASSERT_EQ(0u, notifier_->CountObserver(pref_name, &obs1_));
+  ASSERT_EQ(1u, notifier_->CountObserver(pref_name2, &obs1_));
+  ASSERT_EQ(1u, notifier_->CountObserver(pref_name, &obs2_));
+  ASSERT_EQ(0u, notifier_->CountObserver(pref_name2, &obs2_));
+
+  notifier_->RemovePrefObserver(pref_name, &obs2_);
+  ASSERT_EQ(0u, notifier_->CountObserver(pref_name, &obs1_));
+  ASSERT_EQ(1u, notifier_->CountObserver(pref_name2, &obs1_));
+  ASSERT_EQ(0u, notifier_->CountObserver(pref_name, &obs2_));
+  ASSERT_EQ(0u, notifier_->CountObserver(pref_name2, &obs2_));
+
+  notifier_->RemovePrefObserver(pref_name, &obs1_);
+  ASSERT_EQ(0u, notifier_->CountObserver(pref_name, &obs1_));
+  ASSERT_EQ(1u, notifier_->CountObserver(pref_name2, &obs1_));
+  ASSERT_EQ(0u, notifier_->CountObserver(pref_name, &obs2_));
+  ASSERT_EQ(0u, notifier_->CountObserver(pref_name2, &obs2_));
+
+  notifier_->RemovePrefObserver(pref_name2, &obs1_);
+  ASSERT_EQ(0u, notifier_->CountObserver(pref_name, &obs1_));
+  ASSERT_EQ(0u, notifier_->CountObserver(pref_name2, &obs1_));
+  ASSERT_EQ(0u, notifier_->CountObserver(pref_name, &obs2_));
+  ASSERT_EQ(0u, notifier_->CountObserver(pref_name2, &obs2_));
+}
+
+TEST_F(PrefNotifierTest, FireObservers) {
+  using testing::_;
+  using testing::Field;
+  using testing::Invoke;
+  using testing::Mock;
+  using testing::Truly;
+
+  // Tell googlemock to pass calls to the mock's "back door" too.
+  ON_CALL(*notifier_, FireObservers(_))
+      .WillByDefault(Invoke(notifier_, &MockPrefNotifier::RealFireObservers));
+  EXPECT_CALL(*notifier_, FireObservers(kChangedPref)).Times(4);
+  EXPECT_CALL(*notifier_, FireObservers(kUnchangedPref)).Times(0);
+
+  notifier_->AddPrefObserver(kChangedPref, &obs1_);
+  notifier_->AddPrefObserver(kUnchangedPref, &obs1_);
+
+  EXPECT_CALL(obs1_, Observe(
+      Field(&NotificationType::value, NotificationType::PREF_CHANGED),
+      Source<PrefService>(pref_service_.get()),
+      Truly(DetailsAreChangedPref)));
+  EXPECT_CALL(obs2_, Observe(_, _, _)).Times(0);
+  notifier_->OnPreferenceSet(kChangedPref, NULL);
+  Mock::VerifyAndClearExpectations(&obs1_);
+  Mock::VerifyAndClearExpectations(&obs2_);
+
+  EXPECT_CALL(obs1_, Observe(_, _, _)).Times(0);
+  EXPECT_CALL(obs2_, Observe(_, _, _)).Times(0);
+  notifier_->OnPreferenceSet(kUnchangedPref, NULL);
+  Mock::VerifyAndClearExpectations(&obs1_);
+  Mock::VerifyAndClearExpectations(&obs2_);
+
+  notifier_->AddPrefObserver(kChangedPref, &obs2_);
+  notifier_->AddPrefObserver(kUnchangedPref, &obs2_);
+
+  EXPECT_CALL(obs1_, Observe(
+      Field(&NotificationType::value, NotificationType::PREF_CHANGED),
+      Source<PrefService>(pref_service_.get()),
+      Truly(DetailsAreChangedPref)));
+  EXPECT_CALL(obs2_, Observe(
+      Field(&NotificationType::value, NotificationType::PREF_CHANGED),
+      Source<PrefService>(pref_service_.get()),
+      Truly(DetailsAreChangedPref)));
+  notifier_->OnPreferenceSet(kChangedPref, NULL);
+  Mock::VerifyAndClearExpectations(&obs1_);
+  Mock::VerifyAndClearExpectations(&obs2_);
+
+  EXPECT_CALL(obs1_, Observe(_, _, _)).Times(0);
+  EXPECT_CALL(obs2_, Observe(_, _, _)).Times(0);
+  notifier_->OnPreferenceSet(kUnchangedPref, NULL);
+  Mock::VerifyAndClearExpectations(&obs1_);
+  Mock::VerifyAndClearExpectations(&obs2_);
+
+  // Make sure removing an observer from one pref doesn't affect anything else.
+  notifier_->RemovePrefObserver(kChangedPref, &obs1_);
+
+  EXPECT_CALL(obs1_, Observe(_, _, _)).Times(0);
+  EXPECT_CALL(obs2_, Observe(
+      Field(&NotificationType::value, NotificationType::PREF_CHANGED),
+      Source<PrefService>(pref_service_.get()),
+      Truly(DetailsAreChangedPref)));
+  notifier_->OnPreferenceSet(kChangedPref, NULL);
+  Mock::VerifyAndClearExpectations(&obs1_);
+  Mock::VerifyAndClearExpectations(&obs2_);
+
+  EXPECT_CALL(obs1_, Observe(_, _, _)).Times(0);
+  EXPECT_CALL(obs2_, Observe(_, _, _)).Times(0);
+  notifier_->OnPreferenceSet(kUnchangedPref, NULL);
+  Mock::VerifyAndClearExpectations(&obs1_);
+  Mock::VerifyAndClearExpectations(&obs2_);
+
+  // Make sure removing an observer entirely doesn't affect anything else.
+  notifier_->RemovePrefObserver(kUnchangedPref, &obs1_);
+
+  EXPECT_CALL(obs1_, Observe(_, _, _)).Times(0);
+  EXPECT_CALL(obs2_, Observe(
+      Field(&NotificationType::value, NotificationType::PREF_CHANGED),
+      Source<PrefService>(pref_service_.get()),
+      Truly(DetailsAreChangedPref)));
+  notifier_->OnPreferenceSet(kChangedPref, NULL);
+  Mock::VerifyAndClearExpectations(&obs1_);
+  Mock::VerifyAndClearExpectations(&obs2_);
+
+  EXPECT_CALL(obs1_, Observe(_, _, _)).Times(0);
+  EXPECT_CALL(obs2_, Observe(_, _, _)).Times(0);
+  notifier_->OnPreferenceSet(kUnchangedPref, NULL);
+
+  notifier_->RemovePrefObserver(kChangedPref, &obs2_);
+  notifier_->RemovePrefObserver(kUnchangedPref, &obs2_);
+}
+
+}  // namespace
diff --git a/chrome/browser/pref_service.cc b/chrome/browser/pref_service.cc
index 9e3ad7e..aff5f2e 100644
--- a/chrome/browser/pref_service.cc
+++ b/chrome/browser/pref_service.cc
@@ -20,7 +20,6 @@
 #include "base/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "chrome/browser/chrome_thread.h"
-#include "chrome/browser/policy/configuration_policy_pref_store.h"
 #include "chrome/browser/profile.h"
 #include "chrome/common/notification_service.h"
 #include "grit/chromium_strings.h"
@@ -93,29 +92,14 @@ PrefService* PrefService::CreateUserPrefService(const FilePath& pref_filename) {
 
 PrefService::PrefService(PrefValueStore* pref_value_store)
     : pref_value_store_(pref_value_store) {
+  pref_notifier_.reset(new PrefNotifier(this, pref_value_store));
   InitFromStorage();
-  registrar_.Add(this,
-                 NotificationType(NotificationType::POLICY_CHANGED),
-                 NotificationService::AllSources());
 }
 
 PrefService::~PrefService() {
   DCHECK(CalledOnValidThread());
-
-  // Verify that there are no pref observers when we shut down.
-  for (PrefObserverMap::iterator it = pref_observers_.begin();
-       it != pref_observers_.end(); ++it) {
-    NotificationObserverList::Iterator obs_iterator(*(it->second));
-    if (obs_iterator.GetNext()) {
-      LOG(WARNING) << "pref observer found at shutdown " << it->first;
-    }
-  }
-
   STLDeleteContainerPointers(prefs_.begin(), prefs_.end());
   prefs_.clear();
-  STLDeleteContainerPairSecondPointers(pref_observers_.begin(),
-                                       pref_observers_.end());
-  pref_observers_.clear();
 }
 
 void PrefService::InitFromStorage() {
@@ -341,39 +325,6 @@ bool PrefService::IsManagedPreference(const char* pref_name) const {
   return false;
 }
 
-void PrefService::FireObserversIfChanged(const char* path,
-                                         const Value* old_value) {
-  if (PrefIsChanged(path, old_value))
-    FireObservers(path);
-}
-
-void PrefService::FireObservers(const char* path) {
-  DCHECK(CalledOnValidThread());
-
-  // Convert path to a std::string because the Details constructor requires a
-  // class.
-  std::string path_str(path);
-  PrefObserverMap::iterator observer_iterator = pref_observers_.find(path_str);
-  if (observer_iterator == pref_observers_.end())
-    return;
-
-  NotificationObserverList::Iterator it(*(observer_iterator->second));
-  NotificationObserver* observer;
-  while ((observer = it.GetNext()) != NULL) {
-    observer->Observe(NotificationType::PREF_CHANGED,
-                      Source<PrefService>(this),
-                      Details<std::string>(&path_str));
-  }
-}
-
-bool PrefService::PrefIsChanged(const char* path,
-                                const Value* old_value) {
-  Value* new_value = NULL;
-  pref_value_store_->GetValue(path, &new_value);
-  // Some unit tests have no values for certain prefs.
-  return (!new_value || !old_value->Equals(new_value));
-}
-
 const DictionaryValue* PrefService::GetDictionary(const char* path) const {
   DCHECK(CalledOnValidThread());
 
@@ -404,49 +355,12 @@ const ListValue* PrefService::GetList(const char* path) const {
 
 void PrefService::AddPrefObserver(const char* path,
                                   NotificationObserver* obs) {
-  DCHECK(CalledOnValidThread());
-
-  const Preference* pref = FindPreference(path);
-  if (!pref) {
-    NOTREACHED() << "Trying to add an observer for an unregistered pref: "
-        << path;
-    return;
-  }
-
-  // Get the pref observer list associated with the path.
-  NotificationObserverList* observer_list = NULL;
-  PrefObserverMap::iterator observer_iterator = pref_observers_.find(path);
-  if (observer_iterator == pref_observers_.end()) {
-    observer_list = new NotificationObserverList;
-    pref_observers_[path] = observer_list;
-  } else {
-    observer_list = observer_iterator->second;
-  }
-
-  // Verify that this observer doesn't already exist.
-  NotificationObserverList::Iterator it(*observer_list);
-  NotificationObserver* existing_obs;
-  while ((existing_obs = it.GetNext()) != NULL) {
-    DCHECK(existing_obs != obs) << path << " observer already registered";
-    if (existing_obs == obs)
-      return;
-  }
-
-  // Ok, safe to add the pref observer.
-  observer_list->AddObserver(obs);
+  pref_notifier_->AddPrefObserver(path, obs);
 }
 
 void PrefService::RemovePrefObserver(const char* path,
                                      NotificationObserver* obs) {
-  DCHECK(CalledOnValidThread());
-
-  PrefObserverMap::iterator observer_iterator = pref_observers_.find(path);
-  if (observer_iterator == pref_observers_.end()) {
-    return;
-  }
-
-  NotificationObserverList* observer_list = observer_iterator->second;
-  observer_list->RemoveObserver(obs);
+  pref_notifier_->RemovePrefObserver(path, obs);
 }
 
 void PrefService::RegisterPreference(Preference* pref) {
@@ -472,8 +386,10 @@ void PrefService::ClearPref(const char* path) {
   bool has_old_value = pref_value_store_->GetValue(path, &value);
   pref_value_store_->RemoveUserPrefValue(path);
 
+  // TODO(pamg): Removing the user value when there's a higher-priority setting
+  // doesn't change the value and shouldn't fire observers.
   if (has_old_value)
-    FireObservers(path);
+    pref_notifier_->FireObservers(path);
 }
 
 void PrefService::Set(const char* path, const Value& value) {
@@ -492,7 +408,7 @@ void PrefService::Set(const char* path, const Value& value) {
     scoped_ptr<Value> old_value(GetPrefCopy(path));
     if (!old_value->Equals(&value)) {
       pref_value_store_->RemoveUserPrefValue(path);
-      FireObservers(path);
+      pref_notifier_->FireObservers(path);
     }
     return;
   }
@@ -504,7 +420,7 @@ void PrefService::Set(const char* path, const Value& value) {
   scoped_ptr<Value> old_value(GetPrefCopy(path));
   pref_value_store_->SetUserPrefValue(path, value.DeepCopy());
 
-  FireObserversIfChanged(path, old_value.get());
+  pref_notifier_->OnUserPreferenceSet(path, old_value.get());
 }
 
 void PrefService::SetBoolean(const char* path, bool value) {
@@ -528,7 +444,7 @@ void PrefService::SetBoolean(const char* path, bool value) {
   Value* new_value = Value::CreateBooleanValue(value);
   pref_value_store_->SetUserPrefValue(path, new_value);
 
-  FireObserversIfChanged(path, old_value.get());
+  pref_notifier_->OnUserPreferenceSet(path, old_value.get());
 }
 
 void PrefService::SetInteger(const char* path, int value) {
@@ -552,7 +468,7 @@ void PrefService::SetInteger(const char* path, int value) {
   Value* new_value = Value::CreateIntegerValue(value);
   pref_value_store_->SetUserPrefValue(path, new_value);
 
-  FireObserversIfChanged(path, old_value.get());
+  pref_notifier_->OnUserPreferenceSet(path, old_value.get());
 }
 
 void PrefService::SetReal(const char* path, double value) {
@@ -576,7 +492,7 @@ void PrefService::SetReal(const char* path, double value) {
   Value* new_value = Value::CreateRealValue(value);
   pref_value_store_->SetUserPrefValue(path, new_value);
 
-  FireObserversIfChanged(path, old_value.get());
+  pref_notifier_->OnUserPreferenceSet(path, old_value.get());
 }
 
 void PrefService::SetString(const char* path, const std::string& value) {
@@ -600,7 +516,7 @@ void PrefService::SetString(const char* path, const std::string& value) {
   Value* new_value = Value::CreateStringValue(value);
   pref_value_store_->SetUserPrefValue(path, new_value);
 
-  FireObserversIfChanged(path, old_value.get());
+  pref_notifier_->OnUserPreferenceSet(path, old_value.get());
 }
 
 void PrefService::SetFilePath(const char* path, const FilePath& value) {
@@ -632,7 +548,7 @@ void PrefService::SetFilePath(const char* path, const FilePath& value) {
   pref_value_store_->SetUserPrefValue(path, new_value);
 #endif
 
-  FireObserversIfChanged(path, old_value.get());
+  pref_notifier_->OnUserPreferenceSet(path, old_value.get());
 }
 
 void PrefService::SetInt64(const char* path, int64 value) {
@@ -656,7 +572,7 @@ void PrefService::SetInt64(const char* path, int64 value) {
   Value* new_value = Value::CreateStringValue(base::Int64ToString(value));
   pref_value_store_->SetUserPrefValue(path, new_value);
 
-  FireObserversIfChanged(path, old_value.get());
+  pref_notifier_->OnUserPreferenceSet(path, old_value.get());
 }
 
 int64 PrefService::GetInt64(const char* path) const {
@@ -802,38 +718,6 @@ bool PrefService::Preference::IsUserControlled() const {
   return pref_value_store_->PrefValueFromUserStore(name_.c_str());
 }
 
-void PrefService::FireObserversForRefreshedManagedPrefs(
-    std::vector<std::string> changed_prefs_paths) {
-  DCHECK(CalledOnValidThread());
-  std::vector<std::string>::const_iterator current;
-  for (current = changed_prefs_paths.begin();
-       current != changed_prefs_paths.end();
-       ++current) {
-    FireObservers(current->c_str());
-  }
-}
-
 bool PrefService::Preference::IsUserModifiable() const {
   return pref_value_store_->PrefValueUserModifiable(name_.c_str());
 }
-
-void PrefService::Observe(NotificationType type,
-                          const NotificationSource& source,
-                          const NotificationDetails& details) {
-  if (type == NotificationType::POLICY_CHANGED) {
-      PrefValueStore::AfterRefreshCallback* callback =
-          NewCallback(this,
-                      &PrefService::FireObserversForRefreshedManagedPrefs);
-      // The notification of the policy refresh can come from any thread,
-      // but the manipulation of the PrefValueStore must happen on the UI
-      // thread, thus the policy refresh must be explicitly started on it.
-      ChromeThread::PostTask(
-          ChromeThread::UI, FROM_HERE,
-          NewRunnableMethod(
-              pref_value_store_.get(),
-              &PrefValueStore::RefreshPolicyPrefs,
-              ConfigurationPolicyPrefStore::CreateManagedPolicyPrefStore(),
-              ConfigurationPolicyPrefStore::CreateRecommendedPolicyPrefStore(),
-              callback));
-  }
-}
diff --git a/chrome/browser/pref_service.h b/chrome/browser/pref_service.h
index 53a8523..6489b32 100644
--- a/chrome/browser/pref_service.h
+++ b/chrome/browser/pref_service.h
@@ -13,26 +13,17 @@
 #include <vector>
 
 #include "base/file_path.h"
-#include "base/hash_tables.h"
 #include "base/non_thread_safe.h"
-#include "base/observer_list.h"
-#include "base/ref_counted.h"
 #include "base/scoped_ptr.h"
 #include "base/values.h"
 #include "chrome/browser/pref_value_store.h"
-#include "chrome/common/notification_observer.h"
-#include "chrome/common/notification_registrar.h"
 #include "chrome/common/pref_store.h"
 
-class NotificationDetails;
-class NotificationSource;
-class NotificationType;
-class Preference;
+class NotificationObserver;
+class PrefNotifier;
 class Profile;
-class ScopedPrefUpdate;
 
-class PrefService : public NonThreadSafe,
-                    public NotificationObserver {
+class PrefService : public NonThreadSafe {
  public:
   // A helper class to store all the information associated with a preference.
   class Preference {
@@ -114,8 +105,8 @@ class PrefService : public NonThreadSafe,
   // preferences).
   static PrefService* CreateUserPrefService(const FilePath& pref_filename);
 
-  // This constructor is primarily used by tests. The |PrefValueStore| provides
-  // preference values.
+  // This constructor is primarily used by tests. The |pref_value_store|
+  // provides preference values.
   explicit PrefService(PrefValueStore* pref_value_store);
 
   virtual ~PrefService();
@@ -225,20 +216,15 @@ class PrefService : public NonThreadSafe,
   // preference is not registered.
   const Preference* FindPreference(const char* pref_name) const;
 
-  // For the given pref_name, fire any observer of the pref only if |old_value|
-  // is different from the current value.  Virtual so it can be mocked for a
-  // unit test.
-  virtual void FireObserversIfChanged(const char* pref_name,
-                                      const Value* old_value);
-
   bool read_only() const { return pref_value_store_->ReadOnly(); }
 
- protected:
-  // For the given pref_name, fire any observer of the pref.
-  void FireObservers(const char* pref_name);
+  PrefNotifier* pref_notifier() const { return pref_notifier_.get(); }
 
-  // This should only be accessed by subclasses for unit-testing.
-  bool PrefIsChanged(const char* path, const Value* old_value);
+ protected:
+  // The PrefNotifier handles registering and notifying preference observers.
+  // It is created and owned by this PrefService. Subclasses may access it for
+  // unit testing.
+  scoped_ptr<PrefNotifier> pref_notifier_;
 
  private:
   // Add a preference to the PreferenceMap.  If the pref already exists, return
@@ -256,42 +242,13 @@ class PrefService : public NonThreadSafe,
   // This should only be called from the constructor.
   void InitFromStorage();
 
-  // NotificationObserver methods:
-  virtual void Observe(NotificationType type,
-                       const NotificationSource& source,
-                       const NotificationDetails& details);
-
-  // Called after a policy refresh to notify relevant preference observers.
-  // |changed_prefs_paths| is the vector of preference paths changed by the
-  // policy update. It is passed by value and not reference because
-  // this method is called asynchronously as a callback from another thread.
-  // Copying the vector guarantees that the vector's lifecycle spans the
-  // method's invocation.
-  void FireObserversForRefreshedManagedPrefs(
-      std::vector<std::string> changed_prefs_paths);
-
-  // The value of a Preference can be:
-  // managed, user defined, recommended or default.
-  // The PrefValueStore manages enforced, user defined and recommended values
-  // for Preferences. It returns the value of a Preference with the
-  // highest priority, and allows to set user defined values for preferences
-  // that are not managed.
+  // The PrefValueStore provides prioritized preference values. It is created
+  // and owned by this PrefService. Subclasses may access it for unit testing.
   scoped_refptr<PrefValueStore> pref_value_store_;
 
-  NotificationRegistrar registrar_;
-
   // A set of all the registered Preference objects.
   PreferenceSet prefs_;
 
-  // A map from pref names to a list of observers.  Observers get fired in the
-  // order they are added.
-  typedef ObserverList<NotificationObserver> NotificationObserverList;
-  typedef base::hash_map<std::string, NotificationObserverList*>
-      PrefObserverMap;
-  PrefObserverMap pref_observers_;
-
-  friend class ScopedPrefUpdate;
-
   DISALLOW_COPY_AND_ASSIGN(PrefService);
 };
 
diff --git a/chrome/browser/pref_value_store.cc b/chrome/browser/pref_value_store.cc
index 47b01e0..d5c7ab4 100644
--- a/chrome/browser/pref_value_store.cc
+++ b/chrome/browser/pref_value_store.cc
@@ -46,7 +46,7 @@ bool PrefValueStore::GetValue(const std::string& name,
                               Value** out_value) const {
   // Check the |PrefStore|s in order of their priority from highest to lowest
   // to find the value of the preference described by the given preference name.
-  for (size_t i = 0; i <= PREF_STORE_TYPE_MAX; ++i) {
+  for (size_t i = 0; i <= PrefNotifier::PREF_STORE_TYPE_MAX; ++i) {
     if (pref_stores_[i].get() &&
         pref_stores_[i]->prefs()->Get(name.c_str(), out_value)) {
       return true;
@@ -59,7 +59,7 @@ bool PrefValueStore::GetValue(const std::string& name,
 
 bool PrefValueStore::WritePrefs() {
   bool success = true;
-  for (size_t i = 0; i <= PREF_STORE_TYPE_MAX; ++i) {
+  for (size_t i = 0; i <= PrefNotifier::PREF_STORE_TYPE_MAX; ++i) {
     if (pref_stores_[i].get())
       success = pref_stores_[i]->WritePrefs() && success;
   }
@@ -67,7 +67,7 @@ bool PrefValueStore::WritePrefs() {
 }
 
 void PrefValueStore::ScheduleWritePrefs() {
-  for (size_t i = 0; i <= PREF_STORE_TYPE_MAX; ++i) {
+  for (size_t i = 0; i <= PrefNotifier::PREF_STORE_TYPE_MAX; ++i) {
     if (pref_stores_[i].get())
       pref_stores_[i]->ScheduleWritePrefs();
   }
@@ -75,7 +75,7 @@ void PrefValueStore::ScheduleWritePrefs() {
 
 PrefStore::PrefReadError PrefValueStore::ReadPrefs() {
   PrefStore::PrefReadError result = PrefStore::PREF_READ_ERROR_NONE;
-  for (size_t i = 0; i <= PREF_STORE_TYPE_MAX; ++i) {
+  for (size_t i = 0; i <= PrefNotifier::PREF_STORE_TYPE_MAX; ++i) {
     if (pref_stores_[i].get()) {
       PrefStore::PrefReadError this_error = pref_stores_[i]->ReadPrefs();
       if (result == PrefStore::PREF_READ_ERROR_NONE)
@@ -94,49 +94,60 @@ bool PrefValueStore::HasPrefPath(const char* path) const {
   return rv;
 }
 
+bool PrefValueStore::PrefHasChanged(const char* path,
+                                    const Value* old_value) {
+  Value* new_value = NULL;
+  GetValue(path, &new_value);
+  // Some unit tests have no values for certain prefs.
+  return (!new_value || !old_value->Equals(new_value));
+}
+
 // Note the |DictionaryValue| referenced by the |PrefStore| user_prefs_
 // (returned by the method prefs()) takes the ownership of the Value referenced
 // by in_value.
 void PrefValueStore::SetUserPrefValue(const char* name, Value* in_value) {
-  pref_stores_[USER]->prefs()->Set(name, in_value);
+  pref_stores_[PrefNotifier::USER_STORE]->prefs()->Set(name, in_value);
 }
 
 bool PrefValueStore::ReadOnly() {
-  return pref_stores_[USER]->ReadOnly();
+  return pref_stores_[PrefNotifier::USER_STORE]->ReadOnly();
 }
 
 void PrefValueStore::RemoveUserPrefValue(const char* name) {
-  if (pref_stores_[USER].get()) {
-    pref_stores_[USER]->prefs()->Remove(name, NULL);
+  if (pref_stores_[PrefNotifier::USER_STORE].get()) {
+    pref_stores_[PrefNotifier::USER_STORE]->prefs()->Remove(name, NULL);
   }
 }
 
 bool PrefValueStore::PrefValueInManagedStore(const char* name) {
-  return PrefValueInStore(name, MANAGED);
+  return PrefValueInStore(name, PrefNotifier::MANAGED_STORE);
 }
 
 bool PrefValueStore::PrefValueInExtensionStore(const char* name) {
-  return PrefValueInStore(name, EXTENSION);
+  return PrefValueInStore(name, PrefNotifier::EXTENSION_STORE);
 }
 
 bool PrefValueStore::PrefValueInUserStore(const char* name) {
-  return PrefValueInStore(name, USER);
+  return PrefValueInStore(name, PrefNotifier::USER_STORE);
 }
 
 bool PrefValueStore::PrefValueFromExtensionStore(const char* name) {
-  return ControllingPrefStoreForPref(name) == EXTENSION;
+  return ControllingPrefStoreForPref(name) == PrefNotifier::EXTENSION_STORE;
 }
 
 bool PrefValueStore::PrefValueFromUserStore(const char* name) {
-  return ControllingPrefStoreForPref(name) == USER;
+  return ControllingPrefStoreForPref(name) == PrefNotifier::USER_STORE;
 }
 
 bool PrefValueStore::PrefValueUserModifiable(const char* name) {
-  PrefStoreType effective_store = ControllingPrefStoreForPref(name);
-  return effective_store >= USER || effective_store == INVALID;
+  PrefNotifier::PrefStoreType effective_store =
+      ControllingPrefStoreForPref(name);
+  return effective_store >= PrefNotifier::USER_STORE ||
+         effective_store == PrefNotifier::INVALID_STORE;
 }
 
-bool PrefValueStore::PrefValueInStore(const char* name, PrefStoreType type) {
+bool PrefValueStore::PrefValueInStore(const char* name,
+                                      PrefNotifier::PrefStoreType type) {
   if (!pref_stores_[type].get()) {
     // No store of that type set, so this pref can't be in it.
     return false;
@@ -145,13 +156,13 @@ bool PrefValueStore::PrefValueInStore(const char* name, PrefStoreType type) {
   return pref_stores_[type]->prefs()->Get(name, &tmp_value);
 }
 
-PrefValueStore::PrefStoreType PrefValueStore::ControllingPrefStoreForPref(
+PrefNotifier::PrefStoreType PrefValueStore::ControllingPrefStoreForPref(
     const char* name) {
-  for (int i = 0; i <= PREF_STORE_TYPE_MAX; ++i) {
-    if (PrefValueInStore(name, static_cast<PrefStoreType>(i)))
-      return static_cast<PrefStoreType>(i);
+  for (int i = 0; i <= PrefNotifier::PREF_STORE_TYPE_MAX; ++i) {
+    if (PrefValueInStore(name, static_cast<PrefNotifier::PrefStoreType>(i)))
+      return static_cast<PrefNotifier::PrefStoreType>(i);
   }
-  return INVALID;
+  return PrefNotifier::INVALID_STORE;
 }
 
 void PrefValueStore::RefreshPolicyPrefsCompletion(
@@ -159,9 +170,11 @@ void PrefValueStore::RefreshPolicyPrefsCompletion(
     PrefStore* new_recommended_pref_store,
     AfterRefreshCallback* callback_pointer) {
   scoped_ptr<AfterRefreshCallback> callback(callback_pointer);
-  DictionaryValue* managed_prefs_before(pref_stores_[MANAGED]->prefs());
+  DictionaryValue* managed_prefs_before(
+      pref_stores_[PrefNotifier::MANAGED_STORE]->prefs());
   DictionaryValue* managed_prefs_after(new_managed_pref_store->prefs());
-  DictionaryValue* recommended_prefs_before(pref_stores_[RECOMMENDED]->prefs());
+  DictionaryValue* recommended_prefs_before(
+      pref_stores_[PrefNotifier::RECOMMENDED_STORE]->prefs());
   DictionaryValue* recommended_prefs_after(new_recommended_pref_store->prefs());
 
   std::vector<std::string> changed_managed_paths;
@@ -182,8 +195,9 @@ void PrefValueStore::RefreshPolicyPrefsCompletion(
                  changed_paths.begin());
   changed_paths.resize(last_insert - changed_paths.begin());
 
-  pref_stores_[MANAGED].reset(new_managed_pref_store);
-  pref_stores_[RECOMMENDED].reset(new_recommended_pref_store);
+  pref_stores_[PrefNotifier::MANAGED_STORE].reset(new_managed_pref_store);
+  pref_stores_[PrefNotifier::RECOMMENDED_STORE].reset(
+      new_recommended_pref_store);
   callback->Run(changed_paths);
 }
 
@@ -241,9 +255,9 @@ PrefValueStore::PrefValueStore(PrefStore* managed_prefs,
                                PrefStore* command_line_prefs,
                                PrefStore* user_prefs,
                                PrefStore* recommended_prefs) {
-  pref_stores_[MANAGED].reset(managed_prefs);
-  pref_stores_[EXTENSION].reset(extension_prefs);
-  pref_stores_[COMMAND_LINE].reset(command_line_prefs);
-  pref_stores_[USER].reset(user_prefs);
-  pref_stores_[RECOMMENDED].reset(recommended_prefs);
+  pref_stores_[PrefNotifier::MANAGED_STORE].reset(managed_prefs);
+  pref_stores_[PrefNotifier::EXTENSION_STORE].reset(extension_prefs);
+  pref_stores_[PrefNotifier::COMMAND_LINE_STORE].reset(command_line_prefs);
+  pref_stores_[PrefNotifier::USER_STORE].reset(user_prefs);
+  pref_stores_[PrefNotifier::RECOMMENDED_STORE].reset(recommended_prefs);
 }
diff --git a/chrome/browser/pref_value_store.h b/chrome/browser/pref_value_store.h
index 9dfa56e..c1901f6 100644
--- a/chrome/browser/pref_value_store.h
+++ b/chrome/browser/pref_value_store.h
@@ -18,23 +18,19 @@
 #include "base/scoped_ptr.h"
 #include "base/values.h"
 #include "chrome/browser/chrome_thread.h"
+#include "chrome/browser/pref_notifier.h"
 #include "chrome/common/pref_store.h"
 
 class PrefStore;
 class Profile;
 
-// The class PrefValueStore provides values for preferences. Each Preference
-// has a unique name. This name is used to retrieve the value of a Preference.
-// The value of a preference can be either managed, user-defined or recommended.
-// Managed preference values are set (managed) by a third person (like an
-// admin for example). They have the highest priority and can not be
-// altered by the user.
-// User-defined values are chosen by the user. If there is already
-// a managed value for a preference the user-defined value is ignored and
-// the managed value is used (returned).
-// Otherwise user-defined values have a higher precedence than recommended
-// values. Recommended preference values are set by a third person
-// (like an admin).
+// The PrefValueStore manages various sources of values for Preferences
+// (e.g., configuration policies, extensions, and user settings). It returns
+// the value of a Preference from the source with the highest priority, and
+// allows setting user-defined values for preferences that are not managed.
+// See PrefNotifier for a list of the available preference sources (PrefStores)
+// and their descriptions.
+//
 // Unless otherwise explicitly noted, all of the methods of this class must
 // be called on the UI thread.
 class PrefValueStore : public base::RefCountedThreadSafe<PrefValueStore> {
@@ -74,6 +70,11 @@ class PrefValueStore : public base::RefCountedThreadSafe<PrefValueStore> {
   // Returns true if the PrefValueStore contains the given preference.
   bool HasPrefPath(const char* name) const;
 
+  // Returns true if the effective value of the preference has changed from its
+  // |old_value|. Virtual so it can be mocked for a unit test.
+  // TODO(pamg): Also return true when the controlling PrefStore changes.
+  virtual bool PrefHasChanged(const char* path, const Value* old_value);
+
   // Returns true if the PrefValueStore is read-only.
   // Because the managed and recommended PrefStores are always read-only, the
   // PrefValueStore as a whole is read-only if the PrefStore containing the user
@@ -151,27 +152,14 @@ class PrefValueStore : public base::RefCountedThreadSafe<PrefValueStore> {
   FRIEND_TEST_ALL_PREFIXES(PrefValueStoreTest,
                            TestRefreshPolicyPrefsCompletion);
 
-  // PrefStores must be listed here in order from highest to lowest priority.
-  enum PrefStoreType {
-    // Not associated with an actual PrefStore but used as invalid marker, e.g.
-    // as return value.
-    INVALID = -1,
-    MANAGED = 0,
-    EXTENSION,
-    COMMAND_LINE,
-    USER,
-    RECOMMENDED,
-    PREF_STORE_TYPE_MAX = RECOMMENDED
-  };
-
-  scoped_ptr<PrefStore> pref_stores_[PREF_STORE_TYPE_MAX + 1];
+  scoped_ptr<PrefStore> pref_stores_[PrefNotifier::PREF_STORE_TYPE_MAX + 1];
 
-  bool PrefValueInStore(const char* name, PrefStoreType type);
+  bool PrefValueInStore(const char* name, PrefNotifier::PrefStoreType type);
 
   // Returns the pref store type identifying the source that controls the
   // Preference identified by |name|. If none of the sources has a value,
   // INVALID is returned.
-  PrefStoreType ControllingPrefStoreForPref(const char* name);
+  PrefNotifier::PrefStoreType ControllingPrefStoreForPref(const char* name);
 
   // Called during policy refresh after ReadPrefs completes on the thread
   // that initiated the policy refresh. RefreshPolicyPrefsCompletion takes
diff --git a/chrome/browser/pref_value_store_unittest.cc b/chrome/browser/pref_value_store_unittest.cc
index 0effd04..c159945 100644
--- a/chrome/browser/pref_value_store_unittest.cc
+++ b/chrome/browser/pref_value_store_unittest.cc
@@ -15,6 +15,16 @@
 using testing::_;
 using testing::Mock;
 
+namespace {
+
+class MockPolicyRefreshCallback {
+ public:
+  MockPolicyRefreshCallback() {}
+  MOCK_METHOD1(DoCallback, void(const std::vector<std::string>));
+};
+
+}  // namespace
+
 // Names of the preferences used in this test program.
 namespace prefs {
   const char kCurrentThemeID[] = "extensions.theme.id";
@@ -31,7 +41,7 @@ namespace prefs {
   const char kApplicationLocale[] = "intl.app_locale";
 }
 
-// Potentailly expected values of all preferences used in this test program.
+// Potentially expected values of all preferences used in this test program.
 // The "user" namespace is defined globally in an ARM system header, so we need
 // something different here.
 namespace user_pref {
@@ -281,6 +291,25 @@ TEST_F(PrefValueStoreTest, HasPrefPath) {
   EXPECT_FALSE(pref_value_store_->HasPrefPath(prefs::kMissingPref));
 }
 
+TEST_F(PrefValueStoreTest, PrefHasChanged) {
+  ASSERT_TRUE(pref_value_store_->PrefValueInManagedStore(prefs::kHomepage));
+
+  // Pretend we used to have a different enforced value set.
+  scoped_ptr<Value> value(Value::CreateStringValue("http://www.youtube.com"));
+  EXPECT_TRUE(pref_value_store_->PrefHasChanged(prefs::kHomepage, value.get()));
+
+  // Pretend we used to have the same enforced value set.
+  value.reset(Value::CreateStringValue(enforced_pref::kHomepageValue));
+  EXPECT_FALSE(pref_value_store_->PrefHasChanged(prefs::kHomepage,
+      value.get()));
+
+  // Really set a new value in a lower-priority store.
+  Value* new_value = Value::CreateStringValue("http://www.chromium.org");
+  pref_value_store_->SetUserPrefValue(prefs::kHomepage, new_value);
+  EXPECT_FALSE(pref_value_store_->PrefHasChanged(prefs::kHomepage,
+      value.get()));
+}
+
 TEST_F(PrefValueStoreTest, ReadPrefs) {
   pref_value_store_->ReadPrefs();
   // The ReadPrefs method of the |DummyPrefStore| deletes the |pref_store|s
@@ -456,12 +485,6 @@ TEST_F(PrefValueStoreTest, PrefValueInUserStore) {
   EXPECT_FALSE(pref_value_store_->PrefValueFromUserStore(prefs::kMissingPref));
 }
 
-class MockPolicyRefreshCallback {
- public:
-  MockPolicyRefreshCallback() {}
-  MOCK_METHOD1(DoCallback, void(const std::vector<std::string>));
-};
-
 TEST_F(PrefValueStoreTest, TestPolicyRefresh) {
   // pref_value_store_ is initialized by PrefValueStoreTest to have values
   // in both it's managed and recommended store. By replacing them with
diff --git a/chrome/browser/scoped_pref_update.cc b/chrome/browser/scoped_pref_update.cc
index 5e74ee3..bc8c382 100644
--- a/chrome/browser/scoped_pref_update.cc
+++ b/chrome/browser/scoped_pref_update.cc
@@ -16,5 +16,5 @@ ScopedPrefUpdate::ScopedPrefUpdate(PrefService* service, const wchar_t* path)
       path_(WideToUTF8(path)) {}
 
 ScopedPrefUpdate::~ScopedPrefUpdate() {
-  service_->FireObservers(path_.c_str());
+  service_->pref_notifier()->FireObservers(path_.c_str());
 }
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 8846dd5..772005a 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -2143,6 +2143,8 @@
         'browser/power_save_blocker_win.cc',
         'browser/pref_member.cc',
         'browser/pref_member.h',
+        'browser/pref_notifier.cc',
+        'browser/pref_notifier.h',
         'browser/pref_service.cc',
         'browser/pref_service.h',
         'browser/pref_set_observer.cc',
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 860c652..93c1c7d 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1050,6 +1050,7 @@
         'browser/policy/mock_configuration_policy_provider.h',
         'browser/policy/mock_configuration_policy_store.h',
         'browser/pref_member_unittest.cc',
+        'browser/pref_notifier_unittest.cc',
         'browser/pref_service_unittest.cc',
         'browser/pref_set_observer_unittest.cc',
         'browser/pref_value_store_unittest.cc',
diff --git a/chrome/test/testing_pref_service.cc b/chrome/test/testing_pref_service.cc
index 81d04e8..a5a3b98 100644
--- a/chrome/test/testing_pref_service.cc
+++ b/chrome/test/testing_pref_service.cc
@@ -62,11 +62,11 @@ void TestingPrefService::SetPref(PrefStore* pref_store,
                                  const char* path,
                                  Value* value) {
   pref_store->prefs()->Set(path, value);
-  FireObservers(path);
+  pref_notifier()->FireObservers(path);
 }
 
 void TestingPrefService::RemovePref(PrefStore* pref_store,
                                     const char* path) {
   pref_store->prefs()->Remove(path, NULL);
-  FireObservers(path);
+  pref_notifier()->FireObservers(path);
 }