Remove content::NotificationObserver dependency from most Prefs code.

Instead of using content::NotificationObserver, introduce specific type-safe observer classes and update users to use them. In a very large number of cases this was the users' only reason for being a content::NotificationObserver and they would have a lot of boiler-plate code such as a DCHECK on the notification type and unpacking of the generic NotificationDetails types, so this change removes a bunch of boilerplate and introduces more type safety. This is part of enabling more of the Prefs code to live in base/prefs/. TBR=ben@chromium.org,brettw@chromium.org BUG=155525 Review URL: https://chromiumcodereview.appspot.com/11345008 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@165414 0039d316-1c4b-4281-b951-d872f2087c98
280723148 · Nov 1, 2012 · a6a7ced · a6a7ced
1 parent 38d455b
commit a6a7ced
Show file tree

Hide file tree

Showing 194 changed files with 1,437 additions and 1,516 deletions.
diff --git a/base/base.gyp b/base/base.gyp
@@ -329,6 +329,7 @@
         'prefs/pref_value_map.h',
         'prefs/public/pref_change_registrar.cc',
         'prefs/public/pref_change_registrar.h',
+        'prefs/public/pref_observer.h',
         'prefs/public/pref_service_base.h',
         'prefs/value_map_pref_store.cc',
         'prefs/value_map_pref_store.h',

diff --git a/base/prefs/public/pref_change_registrar.cc b/base/prefs/public/pref_change_registrar.cc
@@ -22,8 +22,7 @@ void PrefChangeRegistrar::Init(PrefServiceBase* service) {
   service_ = service;
 }
 
-void PrefChangeRegistrar::Add(const char* path,
-                              content::NotificationObserver* obs) {
+void PrefChangeRegistrar::Add(const char* path, PrefObserver* obs) {
   if (!service_) {
     NOTREACHED();
     return;
@@ -37,8 +36,7 @@ void PrefChangeRegistrar::Add(const char* path,
   service_->AddPrefObserver(path, obs);
 }
 
-void PrefChangeRegistrar::Remove(const char* path,
-                                 content::NotificationObserver* obs) {
+void PrefChangeRegistrar::Remove(const char* path, PrefObserver* obs) {
   if (!service_) {
     NOTREACHED();
     return;

diff --git a/base/prefs/public/pref_change_registrar.h b/base/prefs/public/pref_change_registrar.h
@@ -11,12 +11,9 @@
 #include "base/basictypes.h"
 #include "base/prefs/base_prefs_export.h"
 
+class PrefObserver;
 class PrefServiceBase;
 
-namespace content {
-class NotificationObserver;
-}
-
 // Automatically manages the registration of one or more pref change observers
 // with a PrefStore. Functions much like NotificationRegistrar, but specifically
 // manages observers of preference changes. When the Registrar is destroyed,
@@ -34,13 +31,11 @@ class BASE_PREFS_EXPORT PrefChangeRegistrar {
   // object. All registered observers will be automatically unregistered
   // when the registrar's destructor is called unless the observer has been
   // explicitly removed by a call to Remove beforehand.
-  void Add(const char* path,
-           content::NotificationObserver* obs);
+  void Add(const char* path, PrefObserver* obs);
 
   // Removes a preference observer that has previously been added with a call to
   // Add.
-  void Remove(const char* path,
-              content::NotificationObserver* obs);
+  void Remove(const char* path, PrefObserver* obs);
 
   // Removes all observers that have been previously added with a call to Add.
   void RemoveAll();
@@ -55,8 +50,7 @@ class BASE_PREFS_EXPORT PrefChangeRegistrar {
   bool IsManaged();
 
  private:
-  typedef std::pair<std::string, content::NotificationObserver*>
-      ObserverRegistration;
+  typedef std::pair<std::string, PrefObserver*> ObserverRegistration;
 
   std::set<ObserverRegistration> observers_;
   PrefServiceBase* service_;

diff --git a/base/prefs/public/pref_change_registrar_unittest.cc b/base/prefs/public/pref_change_registrar_unittest.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/prefs/public/pref_change_registrar.h"
+#include "base/prefs/public/pref_observer.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_pref_service.h"
@@ -18,16 +19,24 @@ using testing::Eq;
 
 namespace {
 
+// TODO(joi): Use PrefObserverMock once it moves to base/prefs/.
+class MockPrefObserver : public PrefObserver {
+ public:
+  virtual ~MockPrefObserver() {}
+
+  MOCK_METHOD2(OnPreferenceChanged, void(PrefServiceBase*, const std::string&));
+};
+
 // A mock provider that allows us to capture pref observer changes.
 class MockPrefService : public TestingPrefService {
  public:
   MockPrefService() {}
   virtual ~MockPrefService() {}
 
   MOCK_METHOD2(AddPrefObserver,
-               void(const char*, content::NotificationObserver*));
+               void(const char*, PrefObserver*));
   MOCK_METHOD2(RemovePrefObserver,
-               void(const char*, content::NotificationObserver*));
+               void(const char*, PrefObserver*));
 };
 
 }  // namespace
@@ -40,17 +49,17 @@ class PrefChangeRegistrarTest : public testing::Test {
  protected:
   virtual void SetUp();
 
-  content::NotificationObserver* observer() const { return observer_.get(); }
+  PrefObserver* observer() const { return observer_.get(); }
   MockPrefService* service() const { return service_.get(); }
 
  private:
   scoped_ptr<MockPrefService> service_;
-  scoped_ptr<content::MockNotificationObserver> observer_;
+  scoped_ptr<MockPrefObserver> observer_;
 };
 
 void PrefChangeRegistrarTest::SetUp() {
   service_.reset(new MockPrefService());
-  observer_.reset(new content::MockNotificationObserver());
+  observer_.reset(new MockPrefObserver());
 }
 
 TEST_F(PrefChangeRegistrarTest, AddAndRemove) {
@@ -137,7 +146,7 @@ class ObserveSetOfPreferencesTest : public testing::Test {
   }
 
   PrefChangeRegistrar* CreatePrefChangeRegistrar(
-        content::NotificationObserver* observer) {
+      PrefObserver* observer) {
     PrefChangeRegistrar* pref_set = new PrefChangeRegistrar();
     pref_set->Init(pref_service_.get());
     pref_set->Add(prefs::kHomePage, observer);
@@ -180,27 +189,23 @@ TEST_F(ObserveSetOfPreferencesTest, Observe) {
   using testing::_;
   using testing::Mock;
 
-  content::MockNotificationObserver observer;
+  MockPrefObserver observer;
   scoped_ptr<PrefChangeRegistrar> pref_set(
       CreatePrefChangeRegistrar(&observer));
 
-  EXPECT_CALL(observer,
-              Observe(int(chrome::NOTIFICATION_PREF_CHANGED),
-                      content::Source<PrefService>(pref_service_.get()),
-                      PrefNameDetails(prefs::kHomePage)));
+  EXPECT_CALL(observer, OnPreferenceChanged(pref_service_.get(),
+                                            prefs::kHomePage));
   pref_service_->SetUserPref(prefs::kHomePage,
                              Value::CreateStringValue("http://crbug.com"));
   Mock::VerifyAndClearExpectations(&observer);
 
-  EXPECT_CALL(observer,
-              Observe(int(chrome::NOTIFICATION_PREF_CHANGED),
-                      content::Source<PrefService>(pref_service_.get()),
-                      PrefNameDetails(prefs::kHomePageIsNewTabPage)));
+  EXPECT_CALL(observer, OnPreferenceChanged(pref_service_.get(),
+                                            prefs::kHomePageIsNewTabPage));
   pref_service_->SetUserPref(prefs::kHomePageIsNewTabPage,
                              Value::CreateBooleanValue(true));
   Mock::VerifyAndClearExpectations(&observer);
 
-  EXPECT_CALL(observer, Observe(_, _, _)).Times(0);
+  EXPECT_CALL(observer, OnPreferenceChanged(_, _)).Times(0);
   pref_service_->SetUserPref(prefs::kApplicationLocale,
                              Value::CreateStringValue("en_US.utf8"));
   Mock::VerifyAndClearExpectations(&observer);

diff --git a/base/prefs/public/pref_observer.h b/base/prefs/public/pref_observer.h
@@ -0,0 +1,19 @@
+// Copyright (c) 2012 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 BASE_PREFS_PUBLIC_PREF_OBSERVER_H_
+#define BASE_PREFS_PUBLIC_PREF_OBSERVER_H_
+
+#include <string>
+
+class PrefServiceBase;
+
+// TODO(joi): Switch to base::Callback and remove this.
+class PrefObserver {
+ public:
+  virtual void OnPreferenceChanged(PrefServiceBase* service,
+                                   const std::string& pref_name) = 0;
+};
+
+#endif  // BASE_PREFS_PUBLIC_PREF_OBSERVER_H_
diff --git a/base/prefs/public/pref_service_base.h b/base/prefs/public/pref_service_base.h
@@ -20,14 +20,14 @@
 
 namespace content {
 class BrowserContext;
-class NotificationObserver;
 }
 
 namespace subtle {
 class PrefMemberBase;
 }
 
 class FilePath;
+class PrefObserver;
 class Profile;
 class TabContents;
 
@@ -259,14 +259,12 @@ class PrefServiceBase {
   // These are protected so they can only be accessed by the friend
   // classes listed above.
   //
-  // If the pref at the given path changes, we call the observer's Observe
-  // method with PREF_CHANGED. Note that observers should not call these methods
-  // directly but rather use a PrefChangeRegistrar to make sure the observer
-  // gets cleaned up properly.
-  virtual void AddPrefObserver(const char* path,
-                               content::NotificationObserver* obs) = 0;
-  virtual void RemovePrefObserver(const char* path,
-                                  content::NotificationObserver* obs) = 0;
+  // If the pref at the given path changes, we call the observer's
+  // OnPreferenceChanged method. Note that observers should not call
+  // these methods directly but rather use a PrefChangeRegistrar to
+  // make sure the observer gets cleaned up properly.
+  virtual void AddPrefObserver(const char* path, PrefObserver* obs) = 0;
+  virtual void RemovePrefObserver(const char* path, PrefObserver* obs) = 0;
 };
 
 #endif  // BASE_PREFS_PUBLIC_PREF_SERVICE_BASE_H_
diff --git a/chrome/browser/api/prefs/pref_member.cc b/chrome/browser/api/prefs/pref_member.cc
@@ -8,7 +8,6 @@
 #include "base/location.h"
 #include "base/prefs/public/pref_service_base.h"
 #include "base/value_conversions.h"
-#include "chrome/common/chrome_notification_types.h"
 
 using base::MessageLoopProxy;
 
@@ -26,7 +25,7 @@ PrefMemberBase::~PrefMemberBase() {
 
 void PrefMemberBase::Init(const char* pref_name,
                           PrefServiceBase* prefs,
-                          content::NotificationObserver* observer) {
+                          PrefObserver* observer) {
   DCHECK(pref_name);
   DCHECK(prefs);
   DCHECK(pref_name_.empty());  // Check that Init is only called once.
@@ -57,14 +56,12 @@ void PrefMemberBase::MoveToThread(
   internal()->MoveToThread(message_loop);
 }
 
-void PrefMemberBase::Observe(int type,
-                             const content::NotificationSource& source,
-                             const content::NotificationDetails& details) {
+void PrefMemberBase::OnPreferenceChanged(PrefServiceBase* service,
+                                         const std::string& pref_name) {
   VerifyValuePrefName();
-  DCHECK(chrome::NOTIFICATION_PREF_CHANGED == type);
   UpdateValueFromPref();
   if (!setting_value_ && observer_)
-    observer_->Observe(type, source, details);
+    observer_->OnPreferenceChanged(service, pref_name);
 }
 
 void PrefMemberBase::UpdateValueFromPref() const {

diff --git a/chrome/browser/api/prefs/pref_member.h b/chrome/browser/api/prefs/pref_member.h
@@ -32,14 +32,14 @@
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop_proxy.h"
+#include "base/prefs/public/pref_observer.h"
 #include "base/values.h"
-#include "content/public/browser/notification_observer.h"
 
 class PrefServiceBase;
 
 namespace subtle {
 
-class PrefMemberBase : public content::NotificationObserver {
+class PrefMemberBase : public PrefObserver {
  protected:
   class Internal : public base::RefCountedThreadSafe<Internal> {
    public:
@@ -91,7 +91,7 @@ class PrefMemberBase : public content::NotificationObserver {
 
   // See PrefMember<> for description.
   void Init(const char* pref_name, PrefServiceBase* prefs,
-            content::NotificationObserver* observer);
+            PrefObserver* observer);
 
   virtual void CreateInternal() const = 0;
 
@@ -100,10 +100,9 @@ class PrefMemberBase : public content::NotificationObserver {
 
   void MoveToThread(const scoped_refptr<base::MessageLoopProxy>& message_loop);
 
-  // content::NotificationObserver
-  virtual void Observe(int type,
-                       const content::NotificationSource& source,
-                       const content::NotificationDetails& details) OVERRIDE;
+  // PrefObserver
+  virtual void OnPreferenceChanged(PrefServiceBase* service,
+                                   const std::string& pref_name) OVERRIDE;
 
   void VerifyValuePrefName() const {
     DCHECK(!pref_name_.empty());
@@ -127,7 +126,7 @@ class PrefMemberBase : public content::NotificationObserver {
  private:
   // Ordered the members to compact the class instance.
   std::string pref_name_;
-  content::NotificationObserver* observer_;
+  PrefObserver* observer_;
   PrefServiceBase* prefs_;
 
  protected:
@@ -153,7 +152,7 @@ class PrefMember : public subtle::PrefMemberBase {
   // don't want any notifications of changes.
   // This method should only be called on the UI thread.
   void Init(const char* pref_name, PrefServiceBase* prefs,
-            content::NotificationObserver* observer) {
+            PrefObserver* observer) {
     subtle::PrefMemberBase::Init(pref_name, prefs, observer);
   }
 

diff --git a/chrome/browser/api/prefs/pref_member_unittest.cc b/chrome/browser/api/prefs/pref_member_unittest.cc
@@ -77,21 +77,17 @@ class GetPrefValueCallback
   bool value_;
 };
 
-class PrefMemberTestClass : public content::NotificationObserver {
+class PrefMemberTestClass : public PrefObserver {
  public:
   explicit PrefMemberTestClass(PrefService* prefs)
       : observe_cnt_(0), prefs_(prefs) {
     str_.Init(kStringPref, prefs, this);
   }
 
-  virtual void Observe(int type,
-                       const content::NotificationSource& source,
-                       const content::NotificationDetails& details) {
-    DCHECK(chrome::NOTIFICATION_PREF_CHANGED == type);
-    PrefService* prefs_in = content::Source<PrefService>(source).ptr();
-    EXPECT_EQ(prefs_in, prefs_);
-    std::string* pref_name_in = content::Details<std::string>(details).ptr();
-    EXPECT_EQ(*pref_name_in, kStringPref);
+  virtual void OnPreferenceChanged(PrefServiceBase* service,
+                                   const std::string& pref_name) OVERRIDE {
+    EXPECT_EQ(service, prefs_);
+    EXPECT_EQ(pref_name, kStringPref);
     EXPECT_EQ(str_.GetValue(), prefs_->GetString(kStringPref));
     ++observe_cnt_;
   }

diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc
@@ -290,14 +290,10 @@ void AutofillManager::RenderViewCreated(content::RenderViewHost* host) {
   UpdatePasswordGenerationState(host, true);
 }
 
-void AutofillManager::Observe(
-    int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
+void AutofillManager::OnPreferenceChanged(PrefServiceBase* service,
+                                          const std::string& pref_name) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK_EQ(chrome::NOTIFICATION_PREF_CHANGED, type);
-  std::string* pref = content::Details<std::string>(details).ptr();
-  DCHECK(prefs::kPasswordGenerationEnabled == *pref);
+  DCHECK(prefs::kPasswordGenerationEnabled == pref_name);
   UpdatePasswordGenerationState(web_contents()->GetRenderViewHost(), false);
 }