diff --git a/chrome/browser/sync/glue/autofill_data_type_controller.cc b/chrome/browser/sync/glue/autofill_data_type_controller.cc index 20f0c7be2b20..ba5a1be4fc88 100644 --- a/chrome/browser/sync/glue/autofill_data_type_controller.cc +++ b/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 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 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; diff --git a/chrome/browser/sync/glue/autofill_data_type_controller.h b/chrome/browser/sync/glue/autofill_data_type_controller.h index 7fc275fcd628..bb513d3f4861 100644 --- a/chrome/browser/sync/glue/autofill_data_type_controller.h +++ b/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 web_data_service); - scoped_refptr web_data_service_; + // Callback once WebDatabase has loaded. + void WebDatabaseLoaded(); DISALLOW_COPY_AND_ASSIGN(AutofillDataTypeController); }; diff --git a/chrome/browser/sync/glue/autofill_data_type_controller_unittest.cc b/chrome/browser/sync/glue/autofill_data_type_controller_unittest.cc index 1f7238658096..ce020f96c543 100644 --- a/chrome/browser/sync/glue/autofill_data_type_controller_unittest.cc +++ b/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()){} // 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& 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 db_loaded_callback_; DISALLOW_COPY_AND_ASSIGN(FakeWebDataService); }; diff --git a/chrome/browser/sync/glue/autofill_profile_data_type_controller.cc b/chrome/browser/sync/glue/autofill_profile_data_type_controller.cc index 479fe764fc07..a48073737ee0 100644 --- a/chrome/browser/sync/glue/autofill_profile_data_type_controller.cc +++ b/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,15 +87,21 @@ 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; } @@ -100,9 +109,6 @@ 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); } diff --git a/chrome/browser/sync/glue/autofill_profile_data_type_controller.h b/chrome/browser/sync/glue/autofill_profile_data_type_controller.h index d49a2f55cf01..3e0074833baa 100644 --- a/chrome/browser/sync/glue/autofill_profile_data_type_controller.h +++ b/chrome/browser/sync/glue/autofill_profile_data_type_controller.h @@ -10,10 +10,8 @@ #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 @@ -21,7 +19,6 @@ 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 web_data_service_; + bool callback_registered_; DISALLOW_COPY_AND_ASSIGN(AutofillProfileDataTypeController); }; diff --git a/components/webdata.gypi b/components/webdata.gypi index d91842224387..f728c2110100 100644 --- a/components/webdata.gypi +++ b/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', diff --git a/components/webdata/common/web_data_service_base.cc b/components/webdata/common/web_data_service_base.cc index 209e612e1b86..c00a6871cb96 100644 --- a/components/webdata/common/web_data_service_base.cc +++ b/components/webdata/common/web_data_service_base.cc @@ -26,7 +26,6 @@ using content::BrowserThread; WebDataServiceBase::WebDataServiceBase(scoped_refptr 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) @@ -34,22 +33,12 @@ WebDataServiceBase::WebDataServiceBase(scoped_refptr wdbs, 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& callback) { + if (!wdbs_) return; - wdbs_->RemoveObserver(observer); + wdbs_->RegisterDBLoadedCallback(callback); } WebDatabase* WebDataServiceBase::GetDatabase() { diff --git a/components/webdata/common/web_data_service_base.h b/components/webdata/common/web_data_service_base.h index 9e0867dd8fa0..11d5d3eaf54a 100644 --- a/components/webdata/common/web_data_service_base.h +++ b/components/webdata/common/web_data_service_base.h @@ -9,7 +9,6 @@ #include "base/files/file_path.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" -#include "components/webdata/common/web_database_observer.h" #include "components/webdata/common/webdata_export.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/notification_source.h" @@ -25,8 +24,7 @@ class Thread; // Base for WebDataService class hierarchy. class WEBDATA_EXPORT WebDataServiceBase - : public WebDatabaseObserver, - public base::RefCountedThreadSafe { public: // All requests return an opaque handle of the following type. @@ -50,10 +48,6 @@ class WEBDATA_EXPORT WebDataServiceBase WebDataServiceBase(scoped_refptr wdbs, const ProfileErrorCallback& callback); - // WebDatabaseObserver implementation. - virtual void WebDatabaseLoaded() OVERRIDE; - virtual void WebDatabaseLoadFailed(sql::InitStatus status) OVERRIDE; - // Cancel any pending request. You need to call this method if your // WebDataServiceConsumer is about to be deleted. virtual void CancelRequest(Handle h); @@ -76,8 +70,13 @@ class WEBDATA_EXPORT WebDataServiceBase // Unloads the database permanently and shuts down service. void ShutdownDatabase(); - virtual void AddDBObserver(WebDatabaseObserver* observer); - virtual void RemoveDBObserver(WebDatabaseObserver* observer); + // Register a callback to be notified that the database has loaded. Multiple + // callbacks may be registered, and each will be called at most once + // (following a successful database load), then cleared. + // Note: if the database load is already complete, then the callback will NOT + // be stored or called. + virtual void RegisterDBLoadedCallback( + const base::Callback& callback); // Returns true if the database load has completetd successfully, and // ShutdownOnUIThread has not yet been called. @@ -94,9 +93,6 @@ class WEBDATA_EXPORT WebDataServiceBase // Our database service. scoped_refptr wdbs_; - // True if we've received a notification that the WebDatabase has loaded. - bool db_loaded_; - private: friend struct content::BrowserThread::DeleteOnThread< content::BrowserThread::UI>; diff --git a/components/webdata/common/web_database_observer.h b/components/webdata/common/web_database_observer.h deleted file mode 100644 index 5ea80026afed..000000000000 --- a/components/webdata/common/web_database_observer.h +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright (c) 2013 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 COMPONENTS_WEBDATA_COMMON_WEB_DATABASE_OBSERVER_H_ -#define COMPONENTS_WEBDATA_COMMON_WEB_DATABASE_OBSERVER_H_ - -#include "components/webdata/common/webdata_export.h" -#include "sql/init_status.h" - -// WebDatabase loads asynchronously on the DB thread. Clients on the UI thread -// can use this interface to be notified when the load is complete, or if it -// fails. -class WEBDATA_EXPORT WebDatabaseObserver { - public: - // Called when DB has been loaded successfully. - virtual void WebDatabaseLoaded() = 0; - // Called when load failed. |status| contains error code. - virtual void WebDatabaseLoadFailed(sql::InitStatus status) {}; - - protected: - virtual ~WebDatabaseObserver() {} -}; - - -#endif // COMPONENTS_WEBDATA_COMMON_WEB_DATABASE_OBSERVER_H_ diff --git a/components/webdata/common/web_database_service.cc b/components/webdata/common/web_database_service.cc index a1b18895e84c..88a7c8934c20 100644 --- a/components/webdata/common/web_database_service.cc +++ b/components/webdata/common/web_database_service.cc @@ -40,7 +40,8 @@ class WebDatabaseService::BackendDelegate : WebDatabaseService::WebDatabaseService( const base::FilePath& path) : path_(path), - weak_ptr_factory_(this) { + weak_ptr_factory_(this), + db_loaded_(false) { // WebDatabaseService should be instantiated on UI thread. DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); // WebDatabaseService requires DB thread if instantiated. @@ -68,6 +69,7 @@ void WebDatabaseService::LoadDatabase() { } void WebDatabaseService::UnloadDatabase() { + db_loaded_ = false; if (!wds_backend_.get()) return; BrowserThread::PostTask(BrowserThread::DB, FROM_HERE, @@ -76,9 +78,12 @@ void WebDatabaseService::UnloadDatabase() { } void WebDatabaseService::ShutdownDatabase() { + db_loaded_ = false; + weak_ptr_factory_.InvalidateWeakPtrs(); + loaded_callbacks_.clear(); + error_callbacks_.clear(); if (!wds_backend_.get()) return; - weak_ptr_factory_.InvalidateWeakPtrs(); BrowserThread::PostTask(BrowserThread::DB, FROM_HERE, Bind(&WebDataServiceBackend::ShutdownDatabase, wds_backend_, false)); @@ -140,24 +145,33 @@ void WebDatabaseService::CancelRequest(WebDataServiceBase::Handle h) { wds_backend_->request_manager()->CancelRequest(h); } -void WebDatabaseService::AddObserver(WebDatabaseObserver* observer) { - observer_list_.AddObserver(observer); +void WebDatabaseService::RegisterDBLoadedCallback( + const base::Callback& callback) { + loaded_callbacks_.push_back(callback); } -void WebDatabaseService::RemoveObserver(WebDatabaseObserver* observer) { - observer_list_.RemoveObserver(observer); +void WebDatabaseService::RegisterDBErrorCallback( + const base::Callback& callback) { + error_callbacks_.push_back(callback); } void WebDatabaseService::OnDatabaseLoadDone(sql::InitStatus status) { if (status == sql::INIT_OK) { - // Notify that the database has been initialized. - FOR_EACH_OBSERVER(WebDatabaseObserver, - observer_list_, - WebDatabaseLoaded()); + db_loaded_ = true; + + for (size_t i = 0; i < loaded_callbacks_.size(); i++) { + if (!loaded_callbacks_[i].is_null()) + loaded_callbacks_[i].Run(); + } + + loaded_callbacks_.clear(); } else { // Notify that the database load failed. - FOR_EACH_OBSERVER(WebDatabaseObserver, - observer_list_, - WebDatabaseLoadFailed(status)); + for (size_t i = 0; i < error_callbacks_.size(); i++) { + if (!error_callbacks_[i].is_null()) + error_callbacks_[i].Run(status); + } + + error_callbacks_.clear(); } } diff --git a/components/webdata/common/web_database_service.h b/components/webdata/common/web_database_service.h index cd3ceae53940..67e95e735081 100644 --- a/components/webdata/common/web_database_service.h +++ b/components/webdata/common/web_database_service.h @@ -93,8 +93,22 @@ class WEBDATA_EXPORT WebDatabaseService // somewhere else. virtual void CancelRequest(WebDataServiceBase::Handle h); - void AddObserver(WebDatabaseObserver* observer); - void RemoveObserver(WebDatabaseObserver* observer); + // Register a callback to be notified that the database has loaded. Multiple + // callbacks may be registered, and each will be called at most once + // (following a successful database load), then cleared. + // Note: if the database load is already complete, then the callback will NOT + // be stored or called. + void RegisterDBLoadedCallback(const base::Callback& callback); + + // Register a callback to be notified that the database has failed to load. + // Multiple callbacks may be registered, and each will be called at most once + // (following a database load failure), then cleared. + // Note: if the database load is already complete, then the callback will NOT + // be stored or called. + void RegisterDBErrorCallback( + const base::Callback& callback); + + bool db_loaded() { return db_loaded_; }; private: class BackendDelegate; @@ -116,10 +130,24 @@ class WEBDATA_EXPORT WebDatabaseService // PostTask on DB thread may outlive us. scoped_refptr wds_backend_; - ObserverList observer_list_; - // All vended weak pointers are invalidated in ShutdownDatabase(). base::WeakPtrFactory weak_ptr_factory_; + + // Types for managing DB loading callbacks. + typedef base::Callback DBLoadedCallback; + typedef std::vector LoadedCallbacks; + + typedef base::Callback DBLoadErrorCallback; + typedef std::vector ErrorCallbacks; + + // Callbacks to be called once the DB has loaded. + LoadedCallbacks loaded_callbacks_; + + // Callbacks to be called if the DB has failed to load. + ErrorCallbacks error_callbacks_; + + // True if the WebDatabase has loaded. + bool db_loaded_; }; #endif // COMPONENTS_WEBDATA_COMMON_WEB_DATABASE_SERVICE_H_