Replace WebDatabaseObserver with callbacks

TBR=tim@chromium.org (sync/glue changes)
BUG=230920

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

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

chrome/browser/sync/glue/autofill_data_type_controller.cc

@@ -40,16 +40,11 @@ void AutofillDataTypeController::WebDatabaseLoaded() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK_EQ(MODEL_STARTING, state());
 
-  if (web_data_service_.get())
-    web_data_service_->RemoveDBObserver(this);
-
   OnModelLoaded();
 }
 
 AutofillDataTypeController::~AutofillDataTypeController() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (web_data_service_.get())
-    web_data_service_->RemoveDBObserver(this);
 }
 
 bool AutofillDataTypeController::PostTaskOnBackendThread(
@@ -63,13 +58,17 @@ bool AutofillDataTypeController::StartModels() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK_EQ(MODEL_STARTING, state());
 
-  web_data_service_ =
+  autofill::AutofillWebDataService* web_data_service =
       autofill::AutofillWebDataService::FromBrowserContext(profile());
-  if (web_data_service_->IsDatabaseLoaded()) {
+
+  if (!web_data_service)
+    return false;
+
+  if (web_data_service->IsDatabaseLoaded()) {
     return true;
   } else {
-    if (web_data_service_.get())
-      web_data_service_->AddDBObserver(this);
+    web_data_service->RegisterDBLoadedCallback(base::Bind(
+        &AutofillDataTypeController::WebDatabaseLoaded, this));
     return false;
   }
 }
@@ -78,8 +77,6 @@ void AutofillDataTypeController::StopModels() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(state() == STOPPING || state() == NOT_RUNNING || state() == DISABLED);
   DVLOG(1) << "AutofillDataTypeController::StopModels() : State = " << state();
-  if (web_data_service_.get())
-    web_data_service_->RemoveDBObserver(this);
 }
 
 void AutofillDataTypeController::StartAssociating(
@@ -89,6 +86,8 @@ void AutofillDataTypeController::StartAssociating(
   ProfileSyncService* sync = ProfileSyncServiceFactory::GetForProfile(
       profile());
   DCHECK(sync);
+  scoped_refptr<autofill::AutofillWebDataService> web_data_service =
+      autofill::AutofillWebDataService::FromBrowserContext(profile());
   bool cull_expired_entries = sync->current_experiments().autofill_culling;
   // First, post the update task to the DB thread, which guarantees us it
   // would run before anything StartAssociating does (e.g.
@@ -98,15 +97,17 @@ void AutofillDataTypeController::StartAssociating(
       base::Bind(
           &AutofillDataTypeController::UpdateAutofillCullingSettings,
           this,
-          cull_expired_entries));
+          cull_expired_entries,
+          web_data_service));
   NonUIDataTypeController::StartAssociating(start_callback);
 }
 
 void AutofillDataTypeController::UpdateAutofillCullingSettings(
-    bool cull_expired_entries) {
+    bool cull_expired_entries,
+    scoped_refptr<autofill::AutofillWebDataService> web_data_service) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
   AutocompleteSyncableService* service =
-      AutocompleteSyncableService::FromWebDataService(web_data_service_.get());
+      AutocompleteSyncableService::FromWebDataService(web_data_service);
   if (!service) {
     DVLOG(1) << "Can't update culling, no AutocompleteSyncableService.";
     return;

chrome/browser/sync/glue/autofill_data_type_controller.h

@@ -11,7 +11,6 @@
 #include "base/gtest_prod_util.h"
 #include "base/memory/ref_counted.h"
 #include "chrome/browser/sync/glue/non_ui_data_type_controller.h"
-#include "components/webdata/common/web_database_observer.h"
 
 namespace autofill {
 class AutofillWebDataService;
@@ -21,8 +20,7 @@ namespace browser_sync {
 
 // A class that manages the startup and shutdown of autofill sync.
 class AutofillDataTypeController
-    : public NonUIDataTypeController,
-      public WebDatabaseObserver {
+    : public NonUIDataTypeController {
  public:
   AutofillDataTypeController(
       ProfileSyncComponentsFactory* profile_sync_factory,
@@ -37,9 +35,6 @@ class AutofillDataTypeController
   // 163431 is addressed / implemented.
   virtual void StartAssociating(const StartCallback& start_callback) OVERRIDE;
 
-  // WebDatabaseObserver implementation.
-  virtual void WebDatabaseLoaded() OVERRIDE;
-
  protected:
   virtual ~AutofillDataTypeController();
 
@@ -57,9 +52,11 @@ class AutofillDataTypeController
 
   // Self-invoked on the DB thread to call the AutocompleteSyncableService with
   // an updated value of autofill culling settings.
-  void UpdateAutofillCullingSettings(bool cull_expired_entries);
+  void UpdateAutofillCullingSettings(bool cull_expired_entries,
+      scoped_refptr<autofill::AutofillWebDataService> web_data_service);
 
-  scoped_refptr<autofill::AutofillWebDataService> web_data_service_;
+  // Callback once WebDatabase has loaded.
+  void WebDatabaseLoaded();
 
   DISALLOW_COPY_AND_ASSIGN(AutofillDataTypeController);
 };

chrome/browser/sync/glue/autofill_data_type_controller_unittest.cc

@@ -21,7 +21,6 @@
 #include "chrome/test/base/profile_mock.h"
 #include "components/autofill/browser/webdata/autofill_webdata_service.h"
 #include "components/webdata/common/web_data_service_test_util.h"
-#include "components/webdata/common/web_database_observer.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/notification_types.h"
@@ -61,34 +60,24 @@ class FakeWebDataService : public AutofillWebDataService {
   FakeWebDataService()
       : AutofillWebDataService(),
         is_database_loaded_(false),
-        observer_(NULL) {}
+        db_loaded_callback_(base::Callback<void(void)>()){}
 
   // Mark the database as loaded and send out the appropriate notification.
   void LoadDatabase() {
     StartSyncableService();
     is_database_loaded_ = true;
-    if (observer_)
-      observer_->WebDatabaseLoaded();
+
+    if (!db_loaded_callback_.is_null())
+      db_loaded_callback_.Run();
   }
 
   virtual bool IsDatabaseLoaded() OVERRIDE {
     return is_database_loaded_;
   }
 
-  // Note: this implementation violates the contract for AddDBObserver (which
-  // should support having multiple observers at the same time), however, it
-  // is the simplest thing that works for the purpose of the unit test, which
-  // only registers one observer.
-  virtual void AddDBObserver(WebDatabaseObserver* observer) OVERRIDE {
-    DCHECK(!observer_);
-    observer_ = observer;
-  }
-
-  virtual void RemoveDBObserver(WebDatabaseObserver* observer) OVERRIDE {
-    if (!observer_)
-      return;
-    DCHECK_EQ(observer_, observer);
-    observer_ = NULL;
+  virtual void RegisterDBLoadedCallback(
+      const base::Callback<void(void)>& callback) OVERRIDE {
+    db_loaded_callback_ = callback;
   }
 
   void StartSyncableService() {
@@ -131,7 +120,7 @@ class FakeWebDataService : public AutofillWebDataService {
 
   bool is_database_loaded_;
   NoOpAutofillBackend autofill_backend_;
-  WebDatabaseObserver* observer_;
+  base::Callback<void(void)> db_loaded_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeWebDataService);
 };

chrome/browser/sync/glue/autofill_profile_data_type_controller.cc

@@ -28,7 +28,8 @@ AutofillProfileDataTypeController::AutofillProfileDataTypeController(
     : NonUIDataTypeController(profile_sync_factory,
                               profile,
                               sync_service),
-      personal_data_(NULL) {}
+      personal_data_(NULL),
+      callback_registered_(false) {}
 
 syncer::ModelType AutofillProfileDataTypeController::type() const {
   return syncer::AUTOFILL_PROFILE;
@@ -41,8 +42,6 @@ syncer::ModelSafeGroup
 
 void AutofillProfileDataTypeController::WebDatabaseLoaded() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (web_data_service_.get())
-    web_data_service_->RemoveDBObserver(this);
   OnModelLoaded();
 }
 
@@ -51,15 +50,19 @@ void AutofillProfileDataTypeController::OnPersonalDataChanged() {
   DCHECK_EQ(state(), MODEL_STARTING);
 
   personal_data_->RemoveObserver(this);
-  web_data_service_ = AutofillWebDataService::FromBrowserContext(profile());
+  autofill::AutofillWebDataService* web_data_service =
+      autofill::AutofillWebDataService::FromBrowserContext(profile());
 
-  if (!web_data_service_.get())
+  if (!web_data_service)
     return;
 
-  if (web_data_service_->IsDatabaseLoaded())
+  if (web_data_service->IsDatabaseLoaded()) {
     OnModelLoaded();
-  else
-    web_data_service_->AddDBObserver(this);
+  } else  if (!callback_registered_) {
+     web_data_service->RegisterDBLoadedCallback(base::Bind(
+        &AutofillProfileDataTypeController::WebDatabaseLoaded, this));
+     callback_registered_ = true;
+  }
 }
 
 AutofillProfileDataTypeController::~AutofillProfileDataTypeController() {}
@@ -84,25 +87,28 @@ bool AutofillProfileDataTypeController::StartModels() {
     return false;
   }
 
-  web_data_service_ = AutofillWebDataService::FromBrowserContext(profile());
+  autofill::AutofillWebDataService* web_data_service =
+      AutofillWebDataService::FromBrowserContext(profile());
 
-  if (!web_data_service_.get())
+  if (!web_data_service)
     return false;
 
-  if (web_data_service_->IsDatabaseLoaded())
+  if (web_data_service->IsDatabaseLoaded())
     return true;
 
-  web_data_service_->AddDBObserver(this);
+  if (!callback_registered_) {
+     web_data_service->RegisterDBLoadedCallback(base::Bind(
+        &AutofillProfileDataTypeController::WebDatabaseLoaded, this));
+     callback_registered_ = true;
+  }
+
   return false;
 }
 
 void AutofillProfileDataTypeController::StopModels() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(state() == STOPPING || state() == NOT_RUNNING);
 
-  if (web_data_service_.get())
-    web_data_service_->RemoveDBObserver(this);
-
   personal_data_->RemoveObserver(this);
 }
 

chrome/browser/sync/glue/autofill_profile_data_type_controller.h

@@ -10,18 +10,15 @@
 #include "base/scoped_observer.h"
 #include "chrome/browser/sync/glue/non_ui_data_type_controller.h"
 #include "components/autofill/browser/personal_data_manager_observer.h"
-#include "components/webdata/common/web_database_observer.h"
 
 namespace autofill {
-class AutofillWebDataService;
 class PersonalDataManager;
 }  // namespace autofill
 
 namespace browser_sync {
 
 class AutofillProfileDataTypeController
     : public NonUIDataTypeController,
-      public WebDatabaseObserver,
       public autofill::PersonalDataManagerObserver {
  public:
   AutofillProfileDataTypeController(
@@ -33,9 +30,6 @@ class AutofillProfileDataTypeController
   virtual syncer::ModelType type() const OVERRIDE;
   virtual syncer::ModelSafeGroup model_safe_group() const OVERRIDE;
 
-  // WebDatabaseObserver implementation.
-  virtual void WebDatabaseLoaded() OVERRIDE;
-
   // PersonalDataManagerObserver implementation:
   virtual void OnPersonalDataChanged() OVERRIDE;
 
@@ -50,8 +44,11 @@ class AutofillProfileDataTypeController
   virtual void StopModels() OVERRIDE;
 
  private:
+  // Callback to notify that WebDatabase has loaded.
+  void WebDatabaseLoaded();
+
   autofill::PersonalDataManager* personal_data_;
-  scoped_refptr<autofill::AutofillWebDataService> web_data_service_;
+  bool callback_registered_;
 
   DISALLOW_COPY_AND_ASSIGN(AutofillProfileDataTypeController);
 };

components/webdata.gypi

@@ -54,7 +54,6 @@
       'sources': [
         'webdata/common/web_database.cc',
         'webdata/common/web_database.h',
-        'webdata/common/web_database_observer.h',
         'webdata/common/web_database_service.cc',
         'webdata/common/web_database_service.h',
         'webdata/common/web_database_table.cc',

components/webdata/common/web_data_service_base.cc

@@ -26,30 +26,19 @@ using content::BrowserThread;
 WebDataServiceBase::WebDataServiceBase(scoped_refptr<WebDatabaseService> wdbs,
                                        const ProfileErrorCallback& callback)
     : wdbs_(wdbs),
-      db_loaded_(false),
       profile_error_callback_(callback) {
   // WebDataService requires DB thread if instantiated.
   // Set WebDataServiceFactory::GetInstance()->SetTestingFactory(&profile, NULL)
   // if you do not want to instantiate WebDataService in your test.
   DCHECK(BrowserThread::IsWellKnownThread(BrowserThread::DB));
 }
 
-void WebDataServiceBase::WebDatabaseLoaded() {
-  db_loaded_ = true;
-}
-
-void WebDataServiceBase::WebDatabaseLoadFailed(sql::InitStatus status) {
-  if (!profile_error_callback_.is_null())
-    profile_error_callback_.Run(status);
-}
-
 void WebDataServiceBase::ShutdownOnUIThread() {
-  db_loaded_ = false;
 }
 
 void WebDataServiceBase::Init() {
   DCHECK(wdbs_.get());
-  wdbs_->AddObserver(this);
+  wdbs_->RegisterDBErrorCallback(profile_error_callback_);
   wdbs_->LoadDatabase();
 }
 
@@ -76,19 +65,16 @@ content::NotificationSource WebDataServiceBase::GetNotificationSource() {
 }
 
 bool WebDataServiceBase::IsDatabaseLoaded() {
-  return db_loaded_;
+  if (!wdbs_)
+    return false;
+  return wdbs_->db_loaded();
 }
 
-void WebDataServiceBase::AddDBObserver(WebDatabaseObserver* observer) {
-  if (!wdbs_.get())
-    return;
-  wdbs_->AddObserver(observer);
-}
-
-void WebDataServiceBase::RemoveDBObserver(WebDatabaseObserver* observer) {
-  if (!wdbs_.get())
+void WebDataServiceBase::RegisterDBLoadedCallback(
+    const base::Callback<void(void)>& callback) {
+  if (!wdbs_)
     return;
-  wdbs_->RemoveObserver(observer);
+  wdbs_->RegisterDBLoadedCallback(callback);
 }
 
 WebDatabase* WebDataServiceBase::GetDatabase() {