Refactor notifications from webdata

Use a simplified, typed system


BUG=181277


Review URL: https://chromiumcodereview.appspot.com/12476031

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

4 files changed, 41 insertions, 48 deletions

diff --git a/components/autofill/browser/autofill_manager_unittest.cc b/components/autofill/browser/autofill_manager_unittest.cc
index 2f9e5f5..e88906b 100644
--- a/components/autofill/browser/autofill_manager_unittest.cc
+++ b/components/autofill/browser/autofill_manager_unittest.cc
@@ -706,6 +706,10 @@ class AutofillManagerTest : public ChromeRenderViewHostTestHarness {
     file_thread_.Stop();
     ChromeRenderViewHostTestHarness::TearDown();
     io_thread_.Stop();
+
+    // Remove the BrowserContext so TestPersonalDataManager does not need to
+    // care about removing self as an observer in destruction.
+    personal_data_.SetBrowserContext(NULL);
   }
 
   virtual TestingProfile* CreateProfile() {
diff --git a/components/autofill/browser/autofill_metrics_unittest.cc b/components/autofill/browser/autofill_metrics_unittest.cc
index f214e47..887b19a 100644
--- a/components/autofill/browser/autofill_metrics_unittest.cc
+++ b/components/autofill/browser/autofill_metrics_unittest.cc
@@ -281,7 +281,7 @@ class AutofillMetricsTest : public ChromeRenderViewHostTestHarness {
   content::TestBrowserThread io_thread_;
 
   scoped_ptr<TestAutofillManager> autofill_manager_;
-  TestPersonalDataManager personal_data_;
+  scoped_ptr<TestPersonalDataManager> personal_data_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(AutofillMetricsTest);
@@ -309,11 +309,13 @@ void AutofillMetricsTest::SetUp() {
   ChromeRenderViewHostTestHarness::SetUp();
   io_thread_.StartIOThread();
   autofill::TabAutofillManagerDelegate::CreateForWebContents(web_contents());
-  personal_data_.SetBrowserContext(profile);
+
+  personal_data_.reset(new TestPersonalDataManager());
+  personal_data_->SetBrowserContext(profile);
   autofill_manager_.reset(new TestAutofillManager(
       web_contents(),
       autofill::TabAutofillManagerDelegate::FromWebContents(web_contents()),
-      &personal_data_));
+      personal_data_.get()));
 
   file_thread_.Start();
 }
@@ -324,6 +326,7 @@ void AutofillMetricsTest::TearDown() {
   // AutofillManager is tied to the lifetime of the WebContents, so it must
   // be destroyed at the destruction of the WebContents.
   autofill_manager_.reset();
+  personal_data_.reset();
   profile()->ResetRequestContext();
   file_thread_.Stop();
   ChromeRenderViewHostTestHarness::TearDown();
@@ -339,7 +342,7 @@ scoped_ptr<ConfirmInfoBarDelegate> AutofillMetricsTest::CreateDelegate(
   return AutofillCCInfoBarDelegate::CreateForTesting(
       metric_logger,
       base::Bind(&TestPersonalDataManager::SaveImportedCreditCard,
-                 base::Unretained(&personal_data_), credit_card));
+                 base::Unretained(personal_data_.get()), credit_card));
 }
 
 // Test that we log quality metrics appropriately.
@@ -1013,30 +1016,30 @@ TEST_F(AutofillMetricsTest, QualityMetricsWithExperimentId) {
 // Test that the profile count is logged correctly.
 TEST_F(AutofillMetricsTest, StoredProfileCount) {
   // The metric should be logged when the profiles are first loaded.
-  EXPECT_CALL(*personal_data_.metric_logger(),
+  EXPECT_CALL(*personal_data_->metric_logger(),
               LogStoredProfileCount(2)).Times(1);
-  personal_data_.LoadProfiles();
+  personal_data_->LoadProfiles();
 
   // The metric should only be logged once.
-  EXPECT_CALL(*personal_data_.metric_logger(),
+  EXPECT_CALL(*personal_data_->metric_logger(),
               LogStoredProfileCount(::testing::_)).Times(0);
-  personal_data_.LoadProfiles();
+  personal_data_->LoadProfiles();
 }
 
 // Test that we correctly log when Autofill is enabled.
 TEST_F(AutofillMetricsTest, AutofillIsEnabledAtStartup) {
-  personal_data_.set_autofill_enabled(true);
-  EXPECT_CALL(*personal_data_.metric_logger(),
+  personal_data_->set_autofill_enabled(true);
+  EXPECT_CALL(*personal_data_->metric_logger(),
               LogIsAutofillEnabledAtStartup(true)).Times(1);
-  personal_data_.Init(profile());
+  personal_data_->Init(profile());
 }
 
 // Test that we correctly log when Autofill is disabled.
 TEST_F(AutofillMetricsTest, AutofillIsDisabledAtStartup) {
-  personal_data_.set_autofill_enabled(false);
-  EXPECT_CALL(*personal_data_.metric_logger(),
+  personal_data_->set_autofill_enabled(false);
+  EXPECT_CALL(*personal_data_->metric_logger(),
               LogIsAutofillEnabledAtStartup(false)).Times(1);
-  personal_data_.Init(profile());
+  personal_data_->Init(profile());
 }
 
 // Test that we log the number of Autofill suggestions when filling a form.
@@ -1139,7 +1142,7 @@ TEST_F(AutofillMetricsTest, CreditCardInfoBar) {
   {
     scoped_ptr<ConfirmInfoBarDelegate> infobar(CreateDelegate(&metric_logger));
     ASSERT_TRUE(infobar);
-    EXPECT_CALL(personal_data_, SaveImportedCreditCard(_));
+    EXPECT_CALL(*personal_data_, SaveImportedCreditCard(_));
     EXPECT_CALL(metric_logger,
         LogCreditCardInfoBarMetric(AutofillMetrics::INFOBAR_ACCEPTED)).Times(1);
     EXPECT_CALL(metric_logger,
diff --git a/components/autofill/browser/personal_data_manager.cc b/components/autofill/browser/personal_data_manager.cc
index 58b8e17..f05242f 100644
--- a/components/autofill/browser/personal_data_manager.cc
+++ b/components/autofill/browser/personal_data_manager.cc
@@ -9,6 +9,7 @@
 #include <iterator>
 
 #include "base/logging.h"
+#include "base/memory/ref_counted.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
@@ -131,15 +132,20 @@ void PersonalDataManager::Init(BrowserContext* browser_context) {
   LoadProfiles();
   LoadCreditCards();
 
-  notification_registrar_.Add(
-      this,
-      chrome::NOTIFICATION_AUTOFILL_MULTIPLE_CHANGED,
-      autofill_data->GetNotificationSource());
+  autofill_data->AddObserver(this);
 }
 
 PersonalDataManager::~PersonalDataManager() {
   CancelPendingQuery(&pending_profiles_query_);
   CancelPendingQuery(&pending_creditcards_query_);
+
+  if (!browser_context_)
+    return;
+
+  scoped_refptr<AutofillWebDataService> autofill_data(
+      AutofillWebDataService::FromBrowserContext(browser_context_));
+  if (autofill_data.get())
+    autofill_data->RemoveObserver(this);
 }
 
 void PersonalDataManager::OnWebDataServiceRequestDone(
@@ -182,6 +188,10 @@ void PersonalDataManager::OnWebDataServiceRequestDone(
   }
 }
 
+void PersonalDataManager::AutofillMultipleChanged() {
+  Refresh();
+}
+
 void PersonalDataManager::AddObserver(PersonalDataManagerObserver* observer) {
   observers_.AddObserver(observer);
 }
@@ -191,22 +201,6 @@ void PersonalDataManager::RemoveObserver(
   observers_.RemoveObserver(observer);
 }
 
-void PersonalDataManager::Observe(int type,
-                                  const content::NotificationSource& source,
-                                  const content::NotificationDetails& details) {
-  DCHECK_EQ(type, chrome::NOTIFICATION_AUTOFILL_MULTIPLE_CHANGED);
-
-  if (DCHECK_IS_ON()) {
-    scoped_refptr<AutofillWebDataService> autofill_data(
-        AutofillWebDataService::FromBrowserContext(browser_context_));
-
-    DCHECK(autofill_data.get() &&
-           autofill_data->GetNotificationSource() == source);
-  }
-
-  Refresh();
-}
-
 bool PersonalDataManager::ImportFormData(
     const FormStructure& form,
     const CreditCard** imported_credit_card) {
diff --git a/components/autofill/browser/personal_data_manager.h b/components/autofill/browser/personal_data_manager.h
index 12f991e..4bde6b3 100644
--- a/components/autofill/browser/personal_data_manager.h
+++ b/components/autofill/browser/personal_data_manager.h
@@ -14,11 +14,10 @@
 #include "base/observer_list.h"
 #include "base/string16.h"
 #include "chrome/browser/api/webdata/web_data_service_consumer.h"
+#include "chrome/browser/webdata/autofill_web_data_service_observer.h"
 #include "components/autofill/browser/autofill_profile.h"
 #include "components/autofill/browser/credit_card.h"
 #include "components/autofill/browser/field_types.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
 
 class AutofillMetrics;
 class FormStructure;
@@ -37,7 +36,7 @@ class BrowserContext;
 // This class also stores the profiles loaded from the database for use during
 // Autofill.
 class PersonalDataManager : public WebDataServiceConsumer,
-                            public content::NotificationObserver {
+                            public AutofillWebDataServiceObserverOnUIThread {
  public:
   // A pair of GUID and variant index. Represents a single FormGroup and a
   // specific data variant.
@@ -54,19 +53,15 @@ class PersonalDataManager : public WebDataServiceConsumer,
       WebDataServiceBase::Handle h,
       const WDTypedResult* result) OVERRIDE;
 
+  // AutofillWebDataServiceObserverOnUIThread:
+  virtual void AutofillMultipleChanged() OVERRIDE;
+
   // Adds a listener to be notified of PersonalDataManager events.
   virtual void AddObserver(PersonalDataManagerObserver* observer);
 
   // Removes |observer| as an observer of this PersonalDataManager.
   virtual void RemoveObserver(PersonalDataManagerObserver* observer);
 
-  // content::NotificationObserver:
-  // Observes "batch" changes made by Sync and refreshes data from the
-  // WebDataServiceBase in response.
-  virtual void Observe(int type,
-                       const content::NotificationSource& source,
-                       const content::NotificationDetails& details) OVERRIDE;
-
   // Scans the given |form| for importable Autofill data. If the form includes
   // sufficient address data, it is immediately imported. If the form includes
   // sufficient credit card data, it is stored into |credit_card|, so that we
@@ -273,9 +268,6 @@ class PersonalDataManager : public WebDataServiceConsumer,
   // Whether we have already logged the number of profiles this session.
   mutable bool has_logged_profile_count_;
 
-  // Manages registration lifetime for NotificationObserver implementation.
-  content::NotificationRegistrar notification_registrar_;
-
   DISALLOW_COPY_AND_ASSIGN(PersonalDataManager);
 };