Observer interface for PolicyProviders

BUG=67707
TEST=existing unit tests still work, in particular: *Policy*

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

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

17 files changed, 196 insertions, 137 deletions

diff --git a/chrome/browser/policy/asynchronous_policy_loader.cc b/chrome/browser/policy/asynchronous_policy_loader.cc
index cf67066..6ccae7a 100644
--- a/chrome/browser/policy/asynchronous_policy_loader.cc
+++ b/chrome/browser/policy/asynchronous_policy_loader.cc
@@ -14,7 +14,6 @@ AsynchronousPolicyLoader::AsynchronousPolicyLoader(
     AsynchronousPolicyProvider::Delegate* delegate,
     int reload_interval_minutes)
     : delegate_(delegate),
-      provider_(NULL),
       reload_task_(NULL),
       reload_interval_(base::TimeDelta::FromMinutes(reload_interval_minutes)),
       origin_loop_(MessageLoop::current()),
@@ -45,15 +44,10 @@ void AsynchronousPolicyLoader::Stop() {
   }
 }
 
-void AsynchronousPolicyLoader::SetProvider(
-    AsynchronousPolicyProvider* provider) {
-  provider_ = provider;
-}
-
 AsynchronousPolicyLoader::~AsynchronousPolicyLoader() {
 }
 
-// Manages the life cycle of a new policy map during until it's life cycle is
+// Manages the life cycle of a new policy map during until its life cycle is
 // taken over by the policy loader.
 class UpdatePolicyTask : public Task {
  public:
@@ -79,6 +73,16 @@ void AsynchronousPolicyLoader::Reload() {
   }
 }
 
+void AsynchronousPolicyLoader::AddObserver(
+    ConfigurationPolicyProvider::Observer* observer) {
+  observer_list_.AddObserver(observer);
+}
+
+void AsynchronousPolicyLoader::RemoveObserver(
+    ConfigurationPolicyProvider::Observer* observer) {
+  observer_list_.RemoveObserver(observer);
+}
+
 void AsynchronousPolicyLoader::CancelReloadTask() {
   if (reload_task_) {
     // Only check the thread if there's still a reload task. During
@@ -137,11 +141,9 @@ void AsynchronousPolicyLoader::UpdatePolicy(DictionaryValue* new_policy_raw) {
   DCHECK(policy_.get());
   if (!policy_->Equals(new_policy.get())) {
     policy_.reset(new_policy.release());
-    // TODO(danno): Change the notification between the provider and the
-    // PrefStore into a notification mechanism, removing the need for the
-    // WeakPtr for the provider.
-    if (provider_)
-      provider_->NotifyStoreOfPolicyChange();
+    FOR_EACH_OBSERVER(ConfigurationPolicyProvider::Observer,
+                      observer_list_,
+                      OnUpdatePolicy());
   }
 }
 
diff --git a/chrome/browser/policy/asynchronous_policy_loader.h b/chrome/browser/policy/asynchronous_policy_loader.h
index 8a5838f..677ce6f 100644
--- a/chrome/browser/policy/asynchronous_policy_loader.h
+++ b/chrome/browser/policy/asynchronous_policy_loader.h
@@ -7,14 +7,14 @@
 #pragma once
 
 #include "base/message_loop.h"
+#include "base/observer_list.h"
 #include "base/ref_counted.h"
 #include "base/values.h"
 #include "chrome/browser/policy/asynchronous_policy_provider.h"
+#include "chrome/browser/policy/configuration_policy_provider.h"
 
 namespace policy {
 
-class ConfigurationPolicyProvider;
-
 // Used by the implementation of asynchronous policy provider to manage the
 // tasks on the file thread that do the heavy lifting of loading policies.
 class AsynchronousPolicyLoader
@@ -34,9 +34,8 @@ class AsynchronousPolicyLoader
   // Stops any pending reload tasks.
   virtual void Stop();
 
-  // Associates a provider with the loader to allow the loader to notify the
-  // provider when policy changes.
-  void SetProvider(AsynchronousPolicyProvider* provider);
+  void AddObserver(ConfigurationPolicyProvider::Observer* observer);
+  void RemoveObserver(ConfigurationPolicyProvider::Observer* observer);
 
   const DictionaryValue* policy() const { return policy_.get(); }
 
@@ -83,9 +82,9 @@ class AsynchronousPolicyLoader
   void InitAfterFileThreadAvailable();
 
   // Replaces the existing policy to value map with a new one, sending
-  // notification to the provider if there is a policy change. Must be called on
-  // |origin_loop_| so that it's safe to call back into the provider, which is
-  // not thread-safe. Takes ownership of |new_policy|.
+  // notification to the observers if there is a policy change. Must be called
+  // on |origin_loop_| so that it's safe to call back into the provider, which
+  // is not thread-safe. Takes ownership of |new_policy|.
   void UpdatePolicy(DictionaryValue* new_policy);
 
   // Provides the low-level mechanics for loading policy.
@@ -94,10 +93,6 @@ class AsynchronousPolicyLoader
   // Current policy.
   scoped_ptr<DictionaryValue> policy_;
 
-  // The provider this loader is associated with. Access only on the thread that
-  // called the constructor. See |origin_loop_| below.
-  AsynchronousPolicyProvider* provider_;
-
   // The reload task. Access only on the file thread. Holds a reference to the
   // currently posted task, so we can cancel and repost it if necessary.
   CancelableTask* reload_task_;
@@ -113,6 +108,8 @@ class AsynchronousPolicyLoader
   // True if Stop has been called.
   bool stopped_;
 
+  ObserverList<ConfigurationPolicyProvider::Observer, true> observer_list_;
+
   DISALLOW_COPY_AND_ASSIGN(AsynchronousPolicyLoader);
 };
 
diff --git a/chrome/browser/policy/asynchronous_policy_loader_unittest.cc b/chrome/browser/policy/asynchronous_policy_loader_unittest.cc
index 1fc1ee5..3cc416c 100644
--- a/chrome/browser/policy/asynchronous_policy_loader_unittest.cc
+++ b/chrome/browser/policy/asynchronous_policy_loader_unittest.cc
@@ -6,10 +6,7 @@
 #include "chrome/browser/policy/asynchronous_policy_provider.h"
 #include "chrome/browser/policy/asynchronous_policy_test_base.h"
 #include "chrome/browser/policy/mock_configuration_policy_provider.h"
-#include "chrome/common/notification_observer_mock.h"
-#include "chrome/common/notification_registrar.h"
-#include "chrome/common/notification_service.h"
-#include "chrome/common/notification_type.h"
+#include "testing/gmock/include/gmock/gmock.h"
 
 using ::testing::_;
 using ::testing::InSequence;
@@ -17,6 +14,12 @@ using ::testing::Return;
 
 namespace policy {
 
+class MockConfigurationPolicyObserver
+    : public ConfigurationPolicyProvider::Observer {
+ public:
+  MOCK_METHOD0(OnUpdatePolicy, void());
+};
+
 class AsynchronousPolicyLoaderTest : public AsynchronousPolicyTestBase {
  public:
   AsynchronousPolicyLoaderTest() {}
@@ -99,27 +102,26 @@ TEST_F(AsynchronousPolicyLoaderTest, Stop) {
 // if the policy changed.
 TEST_F(AsynchronousPolicyLoaderTest, ProviderNotificationOnPolicyChange) {
   InSequence s;
-  NotificationObserverMock observer;
-  //  NotificationService service;
-  NotificationRegistrar registrar;
-  registrar.Add(&observer,
-                NotificationType::POLICY_CHANGED,
-                NotificationService::AllSources());
+  MockConfigurationPolicyObserver observer;
   int dictionary_number_1 = 0;
   int dictionary_number_2 = 0;
   EXPECT_CALL(*delegate_, Load()).WillOnce(
       CreateSequencedTestDictionary(&dictionary_number_1));
   EXPECT_CALL(*delegate_, Load()).WillOnce(
       CreateSequencedTestDictionary(&dictionary_number_2));
-  EXPECT_CALL(observer, Observe(_, _, _)).Times(0);
+  EXPECT_CALL(observer, OnUpdatePolicy()).Times(0);
   EXPECT_CALL(*delegate_, Load()).WillOnce(
       CreateSequencedTestDictionary(&dictionary_number_2));
-  EXPECT_CALL(observer, Observe(_, _, _)).Times(1);
+  EXPECT_CALL(observer, OnUpdatePolicy()).Times(1);
   EXPECT_CALL(*delegate_, Load()).WillOnce(
       CreateSequencedTestDictionary(&dictionary_number_1));
   scoped_refptr<AsynchronousPolicyLoader> loader =
       new AsynchronousPolicyLoader(delegate_.release(), 10);
   AsynchronousPolicyProvider provider(NULL, loader);
+  // |registrar| must be declared last so that it is destroyed first.
+  ConfigurationPolicyObserverRegistrar registrar;
+  registrar.Init(&provider);
+  registrar.AddObserver(&observer);
   loop_.RunAllPending();
   loader->Reload();
   loop_.RunAllPending();
diff --git a/chrome/browser/policy/asynchronous_policy_provider.cc b/chrome/browser/policy/asynchronous_policy_provider.cc
index 87b94a6..e697b05 100644
--- a/chrome/browser/policy/asynchronous_policy_provider.cc
+++ b/chrome/browser/policy/asynchronous_policy_provider.cc
@@ -13,17 +13,12 @@ AsynchronousPolicyProvider::AsynchronousPolicyProvider(
     scoped_refptr<AsynchronousPolicyLoader> loader)
     : ConfigurationPolicyProvider(policy_list),
       loader_(loader) {
-  // TODO(danno): This explicit registration of the provider shouldn't be
-  // necessary. Instead, the PrefStore should explicitly observe preference
-  // changes that are reported during the policy change.
-  loader_->SetProvider(this);
   loader_->Init();
 }
 
 AsynchronousPolicyProvider::~AsynchronousPolicyProvider() {
   DCHECK(CalledOnValidThread());
   loader_->Stop();
-  loader_->SetProvider(NULL);
 }
 
 bool AsynchronousPolicyProvider::Provide(
@@ -34,6 +29,16 @@ bool AsynchronousPolicyProvider::Provide(
   return true;
 }
 
+void AsynchronousPolicyProvider::AddObserver(
+    ConfigurationPolicyProvider::Observer* observer) {
+  loader_->AddObserver(observer);
+}
+
+void AsynchronousPolicyProvider::RemoveObserver(
+    ConfigurationPolicyProvider::Observer* observer) {
+  loader_->RemoveObserver(observer);
+}
+
 scoped_refptr<AsynchronousPolicyLoader> AsynchronousPolicyProvider::loader() {
   return loader_;
 }
diff --git a/chrome/browser/policy/asynchronous_policy_provider.h b/chrome/browser/policy/asynchronous_policy_provider.h
index 0f6b649..561fc37 100644
--- a/chrome/browser/policy/asynchronous_policy_provider.h
+++ b/chrome/browser/policy/asynchronous_policy_provider.h
@@ -21,7 +21,6 @@ class AsynchronousPolicyLoader;
 // policy is handled by a delegate passed at construction time.
 class AsynchronousPolicyProvider
     : public ConfigurationPolicyProvider,
-      public base::SupportsWeakPtr<AsynchronousPolicyProvider>,
       public NonThreadSafe {
  public:
   // Must be implemented by subclasses of the asynchronous policy provider to
@@ -50,6 +49,10 @@ class AsynchronousPolicyProvider
   scoped_refptr<AsynchronousPolicyLoader> loader_;
 
  private:
+  // ConfigurationPolicyProvider overrides:
+  virtual void AddObserver(ConfigurationPolicyProvider::Observer* observer);
+  virtual void RemoveObserver(ConfigurationPolicyProvider::Observer* observer);
+
   DISALLOW_COPY_AND_ASSIGN(AsynchronousPolicyProvider);
 };
 
diff --git a/chrome/browser/policy/configuration_policy_pref_store.cc b/chrome/browser/policy/configuration_policy_pref_store.cc
index 7717ac5..f4876f0 100644
--- a/chrome/browser/policy/configuration_policy_pref_store.cc
+++ b/chrome/browser/policy/configuration_policy_pref_store.cc
@@ -440,7 +440,7 @@ class SearchTermsDataForValidation : public SearchTermsData {
   DISALLOW_COPY_AND_ASSIGN(SearchTermsDataForValidation);
 };
 
-}  // namepsace
+}  // namespace
 
 void ConfigurationPolicyPrefKeeper::FinalizeDefaultSearchPolicySettings() {
   bool enabled = true;
@@ -708,7 +708,7 @@ ConfigurationPolicyProviderKeeper::CreateRecommendedProvider() {
 #endif
 }
 
-}
+}  // namespace
 
 ConfigurationPolicyPrefStore::ConfigurationPolicyPrefStore(
     ConfigurationPolicyProvider* provider)
@@ -717,10 +717,8 @@ ConfigurationPolicyPrefStore::ConfigurationPolicyPrefStore(
   // Read initial policy.
   policy_keeper_.reset(new ConfigurationPolicyPrefKeeper(provider));
 
-  // TODO(mnissler): Remove after provider has proper observer interface.
-  registrar_.Add(this,
-                 NotificationType(NotificationType::POLICY_CHANGED),
-                 NotificationService::AllSources());
+  registrar_.Init(provider_);
+  registrar_.AddObserver(this);
 }
 
 ConfigurationPolicyPrefStore::~ConfigurationPolicyPrefStore() {
@@ -745,6 +743,10 @@ ConfigurationPolicyPrefStore::GetValue(const std::string& key,
   return policy_keeper_->GetValue(key, value);
 }
 
+void ConfigurationPolicyPrefStore::OnUpdatePolicy() {
+  Refresh();
+}
+
 // static
 ConfigurationPolicyPrefStore*
 ConfigurationPolicyPrefStore::CreateManagedPlatformPolicyPrefStore() {
@@ -877,13 +879,6 @@ ConfigurationPolicyPrefStore::GetChromePolicyDefinitionList() {
   return &policy_list;
 }
 
-void ConfigurationPolicyPrefStore::Observe(NotificationType type,
-                                           const NotificationSource& source,
-                                           const NotificationDetails& details) {
-  if (type == NotificationType::POLICY_CHANGED)
-    Refresh();
-}
-
 void ConfigurationPolicyPrefStore::Refresh() {
   // Construct a new keeper, determine what changed and swap the keeper in.
   scoped_ptr<ConfigurationPolicyPrefKeeper> new_keeper(
diff --git a/chrome/browser/policy/configuration_policy_pref_store.h b/chrome/browser/policy/configuration_policy_pref_store.h
index 26ba190..526d884 100644
--- a/chrome/browser/policy/configuration_policy_pref_store.h
+++ b/chrome/browser/policy/configuration_policy_pref_store.h
@@ -15,8 +15,6 @@
 #include "base/values.h"
 #include "chrome/browser/policy/configuration_policy_provider.h"
 #include "chrome/browser/policy/configuration_policy_store_interface.h"
-#include "chrome/common/notification_observer.h"
-#include "chrome/common/notification_registrar.h"
 #include "chrome/common/pref_store.h"
 
 class Profile;
@@ -27,8 +25,9 @@ class ConfigurationPolicyPrefKeeper;
 
 // An implementation of PrefStore that bridges policy settings as read from a
 // ConfigurationPolicyProvider to preferences.
-class ConfigurationPolicyPrefStore : public PrefStore,
-                                     public NotificationObserver {
+class ConfigurationPolicyPrefStore
+    : public PrefStore,
+      public ConfigurationPolicyProvider::Observer {
  public:
   // The ConfigurationPolicyPrefStore does not take ownership of the
   // passed-in |provider|.
@@ -36,11 +35,14 @@ class ConfigurationPolicyPrefStore : public PrefStore,
   virtual ~ConfigurationPolicyPrefStore();
 
   // PrefStore methods:
-  virtual void AddObserver(Observer* observer);
-  virtual void RemoveObserver(Observer* observer);
+  virtual void AddObserver(PrefStore::Observer* observer);
+  virtual void RemoveObserver(PrefStore::Observer* observer);
   virtual bool IsInitializationComplete() const;
   virtual ReadResult GetValue(const std::string& key, Value** result) const;
 
+  // ConfigurationPolicyProvider::Observer methods:
+  virtual void OnUpdatePolicy();
+
   // Creates a ConfigurationPolicyPrefStore that reads managed platform policy.
   static ConfigurationPolicyPrefStore* CreateManagedPlatformPolicyPrefStore();
 
@@ -57,12 +59,6 @@ class ConfigurationPolicyPrefStore : public PrefStore,
       GetChromePolicyDefinitionList();
 
  private:
-  // TODO(mnissler): Remove after provider has proper observer interface.
-  // NotificationObserver overrides:
-  void Observe(NotificationType type,
-               const NotificationSource& source,
-               const NotificationDetails& details);
-
   // Refreshes policy information, rereading policy from the provider and
   // sending out change notifications as appropriate.
   void Refresh();
@@ -80,10 +76,9 @@ class ConfigurationPolicyPrefStore : public PrefStore,
   // Current policy preferences.
   scoped_ptr<ConfigurationPolicyPrefKeeper> policy_keeper_;
 
-  // TODO(mnissler): Remove after provider has proper observer interface.
-  NotificationRegistrar registrar_;
+  ObserverList<PrefStore::Observer, true> observers_;
 
-  ObserverList<Observer, true> observers_;
+  ConfigurationPolicyObserverRegistrar registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(ConfigurationPolicyPrefStore);
 };
diff --git a/chrome/browser/policy/configuration_policy_pref_store_unittest.cc b/chrome/browser/policy/configuration_policy_pref_store_unittest.cc
index 895363c..1896aaa 100644
--- a/chrome/browser/policy/configuration_policy_pref_store_unittest.cc
+++ b/chrome/browser/policy/configuration_policy_pref_store_unittest.cc
@@ -40,13 +40,6 @@ class ConfigurationPolicyPrefStoreTestBase : public TESTBASE {
       : provider_(),
         store_(&provider_) {}
 
-  void RefreshPolicy() {
-    NotificationService::current()->Notify(
-        NotificationType::POLICY_CHANGED,
-        Source<ConfigurationPolicyProvider>(&provider_),
-        NotificationService::NoDetails());
-  }
-
   MockConfigurationPolicyProvider provider_;
   ConfigurationPolicyPrefStore store_;
 };
@@ -67,7 +60,7 @@ TEST_P(ConfigurationPolicyPrefStoreListTest, SetValue) {
   in_value->Append(Value::CreateStringValue("test1"));
   in_value->Append(Value::CreateStringValue("test2,"));
   provider_.AddPolicy(GetParam().type(), in_value);
-  RefreshPolicy();
+  store_.OnUpdatePolicy();
   Value* value;
   EXPECT_EQ(PrefStore::READ_OK,
             store_.GetValue(GetParam().pref_name(), &value));
@@ -101,7 +94,7 @@ TEST_P(ConfigurationPolicyPrefStoreStringTest, GetDefault) {
 TEST_P(ConfigurationPolicyPrefStoreStringTest, SetValue) {
   provider_.AddPolicy(GetParam().type(),
                       Value::CreateStringValue("http://chromium.org"));
-  RefreshPolicy();
+  store_.OnUpdatePolicy();
   Value* value;
   EXPECT_EQ(PrefStore::READ_OK,
             store_.GetValue(GetParam().pref_name(), &value));
@@ -140,7 +133,7 @@ TEST_P(ConfigurationPolicyPrefStoreBooleanTest, GetDefault) {
 
 TEST_P(ConfigurationPolicyPrefStoreBooleanTest, SetValue) {
   provider_.AddPolicy(GetParam().type(), Value::CreateBooleanValue(false));
-  RefreshPolicy();
+  store_.OnUpdatePolicy();
   Value* value;
   bool result = true;
   EXPECT_EQ(PrefStore::READ_OK,
@@ -148,7 +141,7 @@ TEST_P(ConfigurationPolicyPrefStoreBooleanTest, SetValue) {
   EXPECT_TRUE(FundamentalValue(false).Equals(value));
 
   provider_.AddPolicy(GetParam().type(), Value::CreateBooleanValue(true));
-  RefreshPolicy();
+  store_.OnUpdatePolicy();
   result = false;
   EXPECT_EQ(PrefStore::READ_OK,
             store_.GetValue(GetParam().pref_name(), &value));
@@ -216,7 +209,7 @@ TEST_P(ConfigurationPolicyPrefStoreIntegerTest, GetDefault) {
 
 TEST_P(ConfigurationPolicyPrefStoreIntegerTest, SetValue) {
   provider_.AddPolicy(GetParam().type(), Value::CreateIntegerValue(2));
-  RefreshPolicy();
+  store_.OnUpdatePolicy();
   Value* value = NULL;
   EXPECT_EQ(PrefStore::READ_OK,
             store_.GetValue(GetParam().pref_name(), &value));
@@ -546,7 +539,7 @@ TEST_F(ConfigurationPolicyPrefStoreSyncTest, Default) {
 
 TEST_F(ConfigurationPolicyPrefStoreSyncTest, Enabled) {
   provider_.AddPolicy(kPolicySyncDisabled, Value::CreateBooleanValue(false));
-  RefreshPolicy();
+  store_.OnUpdatePolicy();
   // Enabling Sync should not set the pref.
   EXPECT_EQ(PrefStore::READ_NO_VALUE,
             store_.GetValue(prefs::kSyncManaged, NULL));
@@ -554,10 +547,11 @@ TEST_F(ConfigurationPolicyPrefStoreSyncTest, Enabled) {
 
 TEST_F(ConfigurationPolicyPrefStoreSyncTest, Disabled) {
   provider_.AddPolicy(kPolicySyncDisabled, Value::CreateBooleanValue(true));
-  RefreshPolicy();
+  store_.OnUpdatePolicy();
   // Sync should be flagged as managed.
   Value* value = NULL;
   EXPECT_EQ(PrefStore::READ_OK, store_.GetValue(prefs::kSyncManaged, &value));
+  ASSERT_TRUE(value != NULL);
   EXPECT_TRUE(FundamentalValue(true).Equals(value));
 }
 
@@ -573,7 +567,7 @@ TEST_F(ConfigurationPolicyPrefStoreAutoFillTest, Default) {
 
 TEST_F(ConfigurationPolicyPrefStoreAutoFillTest, Enabled) {
   provider_.AddPolicy(kPolicyAutoFillEnabled, Value::CreateBooleanValue(true));
-  RefreshPolicy();
+  store_.OnUpdatePolicy();
   // Enabling AutoFill should not set the pref.
   EXPECT_EQ(PrefStore::READ_NO_VALUE,
             store_.GetValue(prefs::kSyncManaged, NULL));
@@ -581,7 +575,7 @@ TEST_F(ConfigurationPolicyPrefStoreAutoFillTest, Enabled) {
 
 TEST_F(ConfigurationPolicyPrefStoreAutoFillTest, Disabled) {
   provider_.AddPolicy(kPolicyAutoFillEnabled, Value::CreateBooleanValue(false));
-  RefreshPolicy();
+  store_.OnUpdatePolicy();
   // Disabling AutoFill should switch the pref to managed.
   Value* value = NULL;
   EXPECT_EQ(PrefStore::READ_OK,
@@ -612,19 +606,19 @@ TEST_F(ConfigurationPolicyPrefStoreRefreshTest, Refresh) {
   EXPECT_CALL(observer_, OnPrefValueChanged(prefs::kHomePage)).Times(1);
   provider_.AddPolicy(kPolicyHomePage,
                       Value::CreateStringValue("http://www.chromium.org"));
-  RefreshPolicy();
+  store_.OnUpdatePolicy();
   Mock::VerifyAndClearExpectations(&observer_);
   EXPECT_EQ(PrefStore::READ_OK,
             store_.GetValue(prefs::kHomePage, &value));
   EXPECT_TRUE(StringValue("http://www.chromium.org").Equals(value));
 
   EXPECT_CALL(observer_, OnPrefValueChanged(_)).Times(0);
-  RefreshPolicy();
+  store_.OnUpdatePolicy();
   Mock::VerifyAndClearExpectations(&observer_);
 
   EXPECT_CALL(observer_, OnPrefValueChanged(prefs::kHomePage)).Times(1);
   provider_.RemovePolicy(kPolicyHomePage);
-  RefreshPolicy();
+  store_.OnUpdatePolicy();
   Mock::VerifyAndClearExpectations(&observer_);
   EXPECT_EQ(PrefStore::READ_NO_VALUE,
             store_.GetValue(prefs::kHomePage, NULL));
@@ -638,7 +632,7 @@ TEST_F(ConfigurationPolicyPrefStoreRefreshTest, Initialization) {
   provider_.SetInitializationComplete(true);
   EXPECT_FALSE(store_.IsInitializationComplete());
 
-  RefreshPolicy();
+  store_.OnUpdatePolicy();
   Mock::VerifyAndClearExpectations(&observer_);
   EXPECT_TRUE(store_.IsInitializationComplete());
 }
diff --git a/chrome/browser/policy/configuration_policy_provider.cc b/chrome/browser/policy/configuration_policy_provider.cc
index a8ca10f..2aee046 100644
--- a/chrome/browser/policy/configuration_policy_provider.cc
+++ b/chrome/browser/policy/configuration_policy_provider.cc
@@ -4,11 +4,45 @@
 
 #include "chrome/browser/policy/configuration_policy_provider.h"
 
+#include <algorithm>
+
 #include "base/values.h"
-#include "chrome/common/notification_service.h"
 
 namespace policy {
 
+ConfigurationPolicyObserverRegistrar::ConfigurationPolicyObserverRegistrar() {}
+
+ConfigurationPolicyObserverRegistrar::~ConfigurationPolicyObserverRegistrar() {
+  RemoveAll();
+}
+
+void ConfigurationPolicyObserverRegistrar::Init(
+    ConfigurationPolicyProvider* provider) {
+  provider_ = provider;
+}
+
+void ConfigurationPolicyObserverRegistrar::AddObserver(
+    ConfigurationPolicyProvider::Observer* observer) {
+  observers_.push_back(observer);
+  provider_->AddObserver(observer);
+}
+
+void ConfigurationPolicyObserverRegistrar::RemoveObserver(
+    ConfigurationPolicyProvider::Observer* observer) {
+  std::remove(observers_.begin(), observers_.end(), observer);
+  provider_->RemoveObserver(observer);
+}
+
+void ConfigurationPolicyObserverRegistrar::RemoveAll() {
+  for (std::vector<ConfigurationPolicyProvider::Observer*>::iterator it =
+       observers_.begin(); it != observers_.end(); ++it) {
+    provider_->RemoveObserver(*it);
+  }
+  observers_.clear();
+}
+
+// Class ConfigurationPolicyProvider.
+
 ConfigurationPolicyProvider::ConfigurationPolicyProvider(
     const PolicyDefinitionList* policy_list)
     : policy_definition_list_(policy_list) {
@@ -16,13 +50,6 @@ ConfigurationPolicyProvider::ConfigurationPolicyProvider(
 
 ConfigurationPolicyProvider::~ConfigurationPolicyProvider() {}
 
-void ConfigurationPolicyProvider::NotifyStoreOfPolicyChange() {
-  NotificationService::current()->Notify(
-      NotificationType::POLICY_CHANGED,
-      Source<ConfigurationPolicyProvider>(this),
-      NotificationService::NoDetails());
-}
-
 void ConfigurationPolicyProvider::DecodePolicyValueTree(
     const DictionaryValue* policies,
     ConfigurationPolicyStoreInterface* store) {
diff --git a/chrome/browser/policy/configuration_policy_provider.h b/chrome/browser/policy/configuration_policy_provider.h
index b036eb3..5639235 100644
--- a/chrome/browser/policy/configuration_policy_provider.h
+++ b/chrome/browser/policy/configuration_policy_provider.h
@@ -21,6 +21,12 @@ namespace policy {
 // etc.) should implement a subclass of this class.
 class ConfigurationPolicyProvider {
  public:
+  class Observer {
+   public:
+    virtual ~Observer() {}
+    virtual void OnUpdatePolicy() = 0;
+  };
+
   // Used for static arrays of policy values that is used to initialize an
   // instance of the ConfigurationPolicyProvider.
   struct PolicyDefinitionList {
@@ -51,11 +57,6 @@ class ConfigurationPolicyProvider {
   // need to do asynchronous operations for initialization.
   virtual bool IsInitializationComplete() const { return true; }
 
-  // Called by the subclass provider at any time to indicate that the currently
-  // applied policy is not longer current. A policy refresh will be initiated as
-  // soon as possible.
-  virtual void NotifyStoreOfPolicyChange();
-
  protected:
   // Decodes the value tree and writes the configuration to the given |store|.
   void DecodePolicyValueTree(const DictionaryValue* policies,
@@ -66,6 +67,12 @@ class ConfigurationPolicyProvider {
   }
 
  private:
+  friend class ConfigurationPolicyObserverRegistrar;
+
+  virtual void AddObserver(ConfigurationPolicyProvider::Observer* observer) = 0;
+  virtual void RemoveObserver(
+      ConfigurationPolicyProvider::Observer* observer) = 0;
+
   // Contains the default mapping from policy values to the actual names.
   const ConfigurationPolicyProvider::PolicyDefinitionList*
       policy_definition_list_;
@@ -74,6 +81,22 @@ class ConfigurationPolicyProvider {
   DISALLOW_COPY_AND_ASSIGN(ConfigurationPolicyProvider);
 };
 
+// Manages observers for a ConfigurationPolicyProvider. Is used to register
+// observers, and automatically removes them upon destruction.
+class ConfigurationPolicyObserverRegistrar {
+ public:
+  ConfigurationPolicyObserverRegistrar();
+  ~ConfigurationPolicyObserverRegistrar();
+  void Init(ConfigurationPolicyProvider* provider);
+  void AddObserver(ConfigurationPolicyProvider::Observer* observer);
+  void RemoveObserver(ConfigurationPolicyProvider::Observer* observer);
+  void RemoveAll();
+ private:
+  ConfigurationPolicyProvider* provider_;
+  std::vector<ConfigurationPolicyProvider::Observer*> observers_;
+  DISALLOW_COPY_AND_ASSIGN(ConfigurationPolicyObserverRegistrar);
+};
+
 }  // namespace policy
 
 #endif  // CHROME_BROWSER_POLICY_CONFIGURATION_POLICY_PROVIDER_H_
diff --git a/chrome/browser/policy/device_management_policy_provider.cc b/chrome/browser/policy/device_management_policy_provider.cc
index d3d07b2..1a1f0c2 100644
--- a/chrome/browser/policy/device_management_policy_provider.cc
+++ b/chrome/browser/policy/device_management_policy_provider.cc
@@ -124,7 +124,7 @@ bool DeviceManagementPolicyProvider::IsInitializationComplete() const {
 void DeviceManagementPolicyProvider::HandlePolicyResponse(
     const em::DevicePolicyResponse& response) {
   if (cache_->SetPolicy(response))
-    NotifyStoreOfPolicyChange();
+    NotifyCloudPolicyUpdate();
   policy_request_pending_ = false;
   // Reset the error delay since policy fetching succeeded this time.
   policy_refresh_error_delay_ms_ = kPolicyRefreshErrorDelayInMilliseconds;
@@ -189,6 +189,22 @@ void DeviceManagementPolicyProvider::Shutdown() {
     token_fetcher_->Shutdown();
 }
 
+void DeviceManagementPolicyProvider::AddObserver(
+    ConfigurationPolicyProvider::Observer* observer) {
+  observer_list_.AddObserver(observer);
+}
+
+void DeviceManagementPolicyProvider::RemoveObserver(
+    ConfigurationPolicyProvider::Observer* observer) {
+  observer_list_.RemoveObserver(observer);
+}
+
+void DeviceManagementPolicyProvider::NotifyCloudPolicyUpdate() {
+  FOR_EACH_OBSERVER(ConfigurationPolicyProvider::Observer,
+                    observer_list_,
+                    OnUpdatePolicy());
+}
+
 void DeviceManagementPolicyProvider::Initialize(
     DeviceManagementBackend* backend,
     Profile* profile,
@@ -318,18 +334,10 @@ void DeviceManagementPolicyProvider::SetDeviceTokenFetcher(
 
 void DeviceManagementPolicyProvider::StopWaitingForInitialPolicies() {
   waiting_for_initial_policies_ = false;
-  // Send a CLOUD_POLICY_UPDATE notification to unblock ChromeOS logins that
-  // are waiting for an initial policy fetch to complete.
+  // Notify observers that initial policy fetch is complete.
   NotifyCloudPolicyUpdate();
 }
 
-void DeviceManagementPolicyProvider::NotifyCloudPolicyUpdate() const {
-  NotificationService::current()->Notify(
-       NotificationType::CLOUD_POLICY_UPDATE,
-       Source<DeviceManagementPolicyProvider>(this),
-       NotificationService::NoDetails());
-}
-
 // static
 std::string DeviceManagementPolicyProvider::GetDeviceManagementURL() {
   return CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
diff --git a/chrome/browser/policy/device_management_policy_provider.h b/chrome/browser/policy/device_management_policy_provider.h
index 369d874..17edf82 100644
--- a/chrome/browser/policy/device_management_policy_provider.h
+++ b/chrome/browser/policy/device_management_policy_provider.h
@@ -9,6 +9,7 @@
 #include <string>
 
 #include "base/file_path.h"
+#include "base/observer_list.h"
 #include "base/scoped_ptr.h"
 #include "base/time.h"
 #include "base/weak_ptr.h"
@@ -89,6 +90,10 @@ class DeviceManagementPolicyProvider
   // of initialization that requires the IOThread.
   void InitializeAfterIOThreadExists();
 
+  // ConfigurationPolicyProvider overrides:
+  virtual void AddObserver(ConfigurationPolicyProvider::Observer* observer);
+  virtual void RemoveObserver(ConfigurationPolicyProvider::Observer* observer);
+
   // Sends a request to the device manager backend to fetch policy if one isn't
   // already outstanding.
   void SendPolicyRequest();
@@ -105,8 +110,8 @@ class DeviceManagementPolicyProvider
 
   void StopWaitingForInitialPolicies();
 
-  // Send a CLOUD_POLICY_UPDATE notification.
-  void NotifyCloudPolicyUpdate() const;
+  // Notify observers about a policy update.
+  void NotifyCloudPolicyUpdate();
 
   // The path of the device token file.
   FilePath GetTokenPath();
@@ -128,6 +133,7 @@ class DeviceManagementPolicyProvider
   scoped_ptr<DeviceManagementPolicyCache> cache_;
   scoped_refptr<DeviceTokenFetcher> token_fetcher_;
   DeviceTokenFetcher::ObserverRegistrar registrar_;
+  ObserverList<ConfigurationPolicyProvider::Observer, true> observer_list_;
   FilePath storage_dir_;
   bool policy_request_pending_;
   bool refresh_task_pending_;
diff --git a/chrome/browser/policy/device_management_policy_provider_unittest.cc b/chrome/browser/policy/device_management_policy_provider_unittest.cc
index a4127b7..4886f9d 100644
--- a/chrome/browser/policy/device_management_policy_provider_unittest.cc
+++ b/chrome/browser/policy/device_management_policy_provider_unittest.cc
@@ -8,6 +8,7 @@
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/net/gaia/token_service.h"
 #include "chrome/browser/policy/configuration_policy_pref_store.h"
+#include "chrome/browser/policy/configuration_policy_provider.h"
 #include "chrome/browser/policy/device_management_policy_cache.h"
 #include "chrome/browser/policy/device_management_policy_provider.h"
 #include "chrome/browser/policy/mock_configuration_policy_store.h"
@@ -18,6 +19,7 @@
 #include "chrome/common/policy_constants.h"
 #include "chrome/test/testing_device_token_fetcher.h"
 #include "chrome/test/testing_profile.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 const char kTestToken[] = "device_policy_provider_test_auth_token";
@@ -28,6 +30,12 @@ using ::testing::_;
 using ::testing::InSequence;
 using ::testing::Mock;
 
+class MockConfigurationPolicyObserver
+    : public ConfigurationPolicyProvider::Observer {
+ public:
+  MOCK_METHOD0(OnUpdatePolicy, void());
+};
+
 class DeviceManagementPolicyProviderTest : public testing::Test {
  public:
   DeviceManagementPolicyProviderTest()
@@ -197,12 +205,11 @@ TEST_F(DeviceManagementPolicyProviderTest, SecondProvide) {
 // When policy is successfully fetched from the device management server, it
 // should force a policy refresh.
 TEST_F(DeviceManagementPolicyProviderTest, FetchTriggersRefresh) {
-  NotificationObserverMock observer;
-  NotificationRegistrar registrar;
-  registrar.Add(&observer,
-                NotificationType::POLICY_CHANGED,
-                NotificationService::AllSources());
-  EXPECT_CALL(observer, Observe(_, _, _)).Times(1);
+  MockConfigurationPolicyObserver observer;
+  ConfigurationPolicyObserverRegistrar registrar;
+  registrar.Init(provider_.get());
+  registrar.AddObserver(&observer);
+  EXPECT_CALL(observer, OnUpdatePolicy()).Times(1);
   SimulateSuccessfulInitialPolicyFetch();
 }
 
diff --git a/chrome/browser/policy/dummy_configuration_policy_provider.h b/chrome/browser/policy/dummy_configuration_policy_provider.h
index a26ec71..22ca152 100644
--- a/chrome/browser/policy/dummy_configuration_policy_provider.h
+++ b/chrome/browser/policy/dummy_configuration_policy_provider.h
@@ -21,6 +21,11 @@ class DummyConfigurationPolicyProvider : public ConfigurationPolicyProvider {
   virtual bool Provide(ConfigurationPolicyStoreInterface* store);
 
  private:
+  // ConfigurationPolicyProvider overrides:
+  virtual void AddObserver(ConfigurationPolicyProvider::Observer* observer) {}
+  virtual void RemoveObserver(
+      ConfigurationPolicyProvider::Observer* observer) {}
+
   DISALLOW_COPY_AND_ASSIGN(DummyConfigurationPolicyProvider);
 };
 
diff --git a/chrome/browser/policy/file_based_policy_loader.cc b/chrome/browser/policy/file_based_policy_loader.cc
index 4da6192..cc89d13 100644
--- a/chrome/browser/policy/file_based_policy_loader.cc
+++ b/chrome/browser/policy/file_based_policy_loader.cc
@@ -15,7 +15,7 @@ const int kSettleIntervalSeconds = 5;
 // delegate never reports a change to the ReloadObserver.
 const int kReloadIntervalMinutes = 15;
 
-}
+}  // namespace
 
 namespace policy {
 
diff --git a/chrome/browser/policy/mock_configuration_policy_provider.h b/chrome/browser/policy/mock_configuration_policy_provider.h
index aa2969e..5620f81 100644
--- a/chrome/browser/policy/mock_configuration_policy_provider.h
+++ b/chrome/browser/policy/mock_configuration_policy_provider.h
@@ -30,9 +30,12 @@ class MockConfigurationPolicyProvider : public ConfigurationPolicyProvider {
   virtual bool Provide(ConfigurationPolicyStoreInterface* store);
   virtual bool IsInitializationComplete() const;
 
-  MOCK_METHOD0(NotifyStoreOfPolicyChange, void());
-
  private:
+  // ConfigurationPolicyProvider overrides:
+  virtual void AddObserver(ConfigurationPolicyProvider::Observer* observer) {}
+  virtual void RemoveObserver(
+      ConfigurationPolicyProvider::Observer* observer) {}
+
   typedef std::map<ConfigurationPolicyType, Value*> PolicyMap;
 
   PolicyMap policy_map_;
diff --git a/chrome/common/notification_type.h b/chrome/common/notification_type.h
index d27f2f8..edbc371 100644
--- a/chrome/common/notification_type.h
+++ b/chrome/common/notification_type.h
@@ -1297,19 +1297,6 @@ class NotificationType {
     // |webkit_glue::PasswordForm|s that were affected.
     LOGINS_CHANGED,
 
-    // Configuration Policy ----------------------------------------------------
-    // This notification is sent whenever the administrator changes policy.
-    // The detail of this notification is not used.
-    POLICY_CHANGED,
-
-    // This notification is sent whenever the device token becomes available
-    // that the policy subsystem uses to fetch policy from the cloud.
-    DEVICE_TOKEN_AVAILABLE,
-
-    // This notification is sent whenever cloud policies are fetched and
-    // updated. The detail of this notification is not used.
-    CLOUD_POLICY_UPDATE,
-
     // Count (must be last) ----------------------------------------------------
     // Used to determine the number of notification types.  Not valid as
     // a type parameter when registering for or posting notifications.