diff options
19 files changed, 413 insertions, 62 deletions
diff --git a/chrome/browser/sync/glue/autofill_change_processor.cc b/chrome/browser/sync/glue/autofill_change_processor.cc index 9e7e174..c96fd4a 100755 --- a/chrome/browser/sync/glue/autofill_change_processor.cc +++ b/chrome/browser/sync/glue/autofill_change_processor.cc @@ -18,11 +18,14 @@ namespace browser_sync { AutofillChangeProcessor::AutofillChangeProcessor( AutofillModelAssociator* model_associator, + WebDatabase* web_database, UnrecoverableErrorHandler* error_handler) : ChangeProcessor(error_handler), model_associator_(model_associator), + web_database_(web_database), observing_(false) { DCHECK(model_associator); + DCHECK(web_database); DCHECK(error_handler); DCHECK(ChromeThread::CurrentlyOn(ChromeThread::DB)); StartObserving(); @@ -35,12 +38,10 @@ void AutofillChangeProcessor::Observe(NotificationType type, DCHECK(running()); DCHECK(NotificationType::AUTOFILL_ENTRIES_CHANGED == type); DCHECK(ChromeThread::CurrentlyOn(ChromeThread::DB)); - if (!observing_ || !web_data_service_.get()) { + if (!observing_) { return; } - DCHECK(web_data_service_->GetDatabase()); - AutofillChangeList* changes = Details<AutofillChangeList>(details).ptr(); sync_api::WriteTransaction trans(share_handle()); @@ -70,10 +71,10 @@ void AutofillChangeProcessor::Observe(NotificationType type, } std::vector<base::Time> timestamps; - if (!web_data_service_->GetDatabase()->GetAutofillTimestamps( - change->key().name(), - change->key().value(), - ×tamps)) { + if (!web_database_->GetAutofillTimestamps( + change->key().name(), + change->key().value(), + ×tamps)) { LOG(ERROR) << "Failed to get timestamps."; error_handler()->OnUnrecoverableError(); return; @@ -112,7 +113,7 @@ void AutofillChangeProcessor::Observe(NotificationType type, } std::vector<base::Time> timestamps; - if (!web_data_service_->GetDatabase()->GetAutofillTimestamps( + if (!web_database_->GetAutofillTimestamps( change->key().name(), change->key().value(), ×tamps)) { @@ -155,8 +156,6 @@ void AutofillChangeProcessor::ApplyChangesFromSyncModel( const sync_api::SyncManager::ChangeRecord* changes, int change_count) { DCHECK(ChromeThread::CurrentlyOn(ChromeThread::DB)); - DCHECK(web_data_service_.get()); - DCHECK(web_data_service_->GetDatabase()); if (!running()) return; StopObserving(); @@ -186,8 +185,8 @@ void AutofillChangeProcessor::ApplyChangesFromSyncModel( sync_node.GetAutofillSpecifics()); if (sync_api::SyncManager::ChangeRecord::ACTION_DELETE == changes[i].action) { - if (!web_data_service_->GetDatabase()->RemoveFormElement( - UTF8ToUTF16(autofill.name()), UTF8ToUTF16(autofill.value()))) { + if (!web_database_->RemoveFormElement( + UTF8ToUTF16(autofill.name()), UTF8ToUTF16(autofill.value()))) { LOG(ERROR) << "Could not remove autofill node."; error_handler()->OnUnrecoverableError(); return; @@ -206,7 +205,7 @@ void AutofillChangeProcessor::ApplyChangesFromSyncModel( new_entries.push_back(new_entry); } } - if (!web_data_service_->GetDatabase()->UpdateAutofillEntries(new_entries)) { + if (!web_database_->UpdateAutofillEntries(new_entries)) { LOG(ERROR) << "Could not update autofill entries."; error_handler()->OnUnrecoverableError(); return; @@ -232,15 +231,13 @@ bool AutofillChangeProcessor::WriteAutofill( void AutofillChangeProcessor::StartImpl(Profile* profile) { - DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI)); - web_data_service_ = profile->GetWebDataService(Profile::IMPLICIT_ACCESS); + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::DB)); observing_ = true; } void AutofillChangeProcessor::StopImpl() { DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI)); observing_ = false; - web_data_service_.release(); } diff --git a/chrome/browser/sync/glue/autofill_change_processor.h b/chrome/browser/sync/glue/autofill_change_processor.h index 7468544..e482a90 100755 --- a/chrome/browser/sync/glue/autofill_change_processor.h +++ b/chrome/browser/sync/glue/autofill_change_processor.h @@ -12,7 +12,7 @@ #include "chrome/common/notification_observer.h" #include "chrome/common/notification_registrar.h" -class WebDataService; +class WebDatabase; namespace browser_sync { @@ -26,6 +26,7 @@ class AutofillChangeProcessor : public ChangeProcessor, public NotificationObserver { public: AutofillChangeProcessor(AutofillModelAssociator* model_associator, + WebDatabase* web_database, UnrecoverableErrorHandler* error_handler); virtual ~AutofillChangeProcessor() {} @@ -54,14 +55,14 @@ class AutofillChangeProcessor : public ChangeProcessor, void StartObserving(); void StopObserving(); - // The model we are processing changes from. Non-NULL when |running_| is true. - // We keep a reference to web data service instead of the database because - // WebDatabase is not refcounted. - scoped_refptr<WebDataService> web_data_service_; - // The two models should be associated according to this ModelAssociator. AutofillModelAssociator* model_associator_; + // The model we are processing changes from. This is owned by the + // WebDataService which is kept alive by our data type controller + // holding a reference. + WebDatabase* web_database_; + NotificationRegistrar notification_registrar_; bool observing_; diff --git a/chrome/browser/sync/glue/autofill_data_type_controller.cc b/chrome/browser/sync/glue/autofill_data_type_controller.cc index eaf163a..5d72603 100755 --- a/chrome/browser/sync/glue/autofill_data_type_controller.cc +++ b/chrome/browser/sync/glue/autofill_data_type_controller.cc @@ -7,6 +7,7 @@ #include "base/task.h" #include "base/time.h" #include "chrome/browser/chrome_thread.h" +#include "chrome/browser/profile.h" #include "chrome/browser/sync/glue/autofill_change_processor.h" #include "chrome/browser/sync/glue/autofill_data_type_controller.h" #include "chrome/browser/sync/glue/autofill_model_associator.h" @@ -19,12 +20,15 @@ namespace browser_sync { AutofillDataTypeController::AutofillDataTypeController( ProfileSyncFactory* profile_sync_factory, + Profile* profile, ProfileSyncService* sync_service) : profile_sync_factory_(profile_sync_factory), + profile_(profile), sync_service_(sync_service), state_(NOT_RUNNING), merge_allowed_(false) { DCHECK(profile_sync_factory); + DCHECK(profile); DCHECK(sync_service); } @@ -44,9 +48,8 @@ void AutofillDataTypeController::Start(bool merge_allowed, start_callback_.reset(start_callback); merge_allowed_ = merge_allowed; - WebDataService *service = - sync_service_->profile()->GetWebDataService(Profile::IMPLICIT_ACCESS); - if (service && service->IsDatabaseLoaded()) { + web_data_service_ = profile_->GetWebDataService(Profile::IMPLICIT_ACCESS); + if (web_data_service_.get() && web_data_service_->IsDatabaseLoaded()) { ChromeThread::PostTask(ChromeThread::DB, FROM_HERE, NewRunnableMethod( this, @@ -95,7 +98,10 @@ void AutofillDataTypeController::StartImpl(bool merge_allowed) { // No additional services need to be started before we can proceed // with model association. ProfileSyncFactory::SyncComponents sync_components = - profile_sync_factory_->CreateAutofillSyncComponents(sync_service_, this); + profile_sync_factory_->CreateAutofillSyncComponents( + sync_service_, + web_data_service_->GetDatabase(), + this); model_associator_.reset(sync_components.model_associator); change_processor_.reset(sync_components.change_processor); bool needs_merge = model_associator_->ChromeModelHasUserCreatedNodes() && @@ -119,6 +125,9 @@ void AutofillDataTypeController::StartImpl(bool merge_allowed) { return; } + sync_service_->ActivateDataType(this, change_processor_.get()); + state_ = RUNNING; + StartDone(first_run ? OK_FIRST_RUN : OK); } @@ -138,11 +147,6 @@ void AutofillDataTypeController::StartDoneImpl( LOG(INFO) << "Autofill data type controller StartDoneImpl called."; DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI)); - if (result == OK_FIRST_RUN || result == OK) { - sync_service_->ActivateDataType(this, change_processor_.get()); - state_ = RUNNING; - } - start_callback_->Run(result); start_callback_.reset(); } diff --git a/chrome/browser/sync/glue/autofill_data_type_controller.h b/chrome/browser/sync/glue/autofill_data_type_controller.h index 6500c0b..cc11a400 100755 --- a/chrome/browser/sync/glue/autofill_data_type_controller.h +++ b/chrome/browser/sync/glue/autofill_data_type_controller.h @@ -10,6 +10,7 @@ #include "chrome/browser/sync/profile_sync_service.h" #include "chrome/browser/sync/glue/data_type_controller.h" +class Profile; class ProfileSyncFactory; class ProfileSyncService; @@ -25,6 +26,7 @@ class AutofillDataTypeController : public DataTypeController, public: AutofillDataTypeController( ProfileSyncFactory* profile_sync_factory, + Profile* profile, ProfileSyncService* sync_service); virtual ~AutofillDataTypeController(); @@ -70,10 +72,11 @@ class AutofillDataTypeController : public DataTypeController, void OnUnrecoverableErrorImpl(); ProfileSyncFactory* profile_sync_factory_; + Profile* profile_; ProfileSyncService* sync_service_; - State state_; + scoped_refptr<WebDataService> web_data_service_; scoped_ptr<AssociatorInterface> model_associator_; scoped_ptr<ChangeProcessor> change_processor_; scoped_ptr<StartCallback> start_callback_; diff --git a/chrome/browser/sync/glue/autofill_model_associator.cc b/chrome/browser/sync/glue/autofill_model_associator.cc index da26816..a7c4f7b 100755 --- a/chrome/browser/sync/glue/autofill_model_associator.cc +++ b/chrome/browser/sync/glue/autofill_model_associator.cc @@ -10,7 +10,6 @@ #include "chrome/browser/sync/profile_sync_service.h" #include "chrome/browser/sync/protocol/autofill_specifics.pb.h" #include "chrome/browser/webdata/web_database.h" -#include "chrome/browser/webdata/web_data_service.h" #include "net/base/escape.h" namespace browser_sync { @@ -19,12 +18,15 @@ const char kAutofillTag[] = "google_chrome_autofill"; AutofillModelAssociator::AutofillModelAssociator( ProfileSyncService* sync_service, + WebDatabase* web_database, UnrecoverableErrorHandler* error_handler) : sync_service_(sync_service), + web_database_(web_database), error_handler_(error_handler), autofill_node_id_(sync_api::kInvalidId), ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) { DCHECK(sync_service_); + DCHECK(web_database_); DCHECK(error_handler_); } @@ -32,23 +34,25 @@ bool AutofillModelAssociator::AssociateModels() { LOG(INFO) << "Associating Autofill Models"; DCHECK(ChromeThread::CurrentlyOn(ChromeThread::DB)); - sync_api::WriteTransaction trans( - sync_service_->backend()->GetUserShareHandle()); - WebDataService* web_data_service = - sync_service_->profile()->GetWebDataService(Profile::EXPLICIT_ACCESS); - if (!web_data_service) { - LOG(ERROR) << "Could not get the web data service."; + // TODO(zork): Attempt to load the model association from storage. + std::vector<AutofillEntry> entries; + if (!web_database_->GetAllAutofillEntries(&entries)) { + LOG(ERROR) << "Could not get the autofill entries."; return false; } - std::vector<AutofillEntry> entries; - if (!web_data_service->GetDatabase()->GetAllAutofillEntries(&entries)) { - LOG(ERROR) << "Could not get the autofill entries."; + int64 root_id; + if (!GetSyncIdForTaggedNode(kAutofillTag, &root_id)) { + sync_service_->OnUnrecoverableError(); + LOG(ERROR) << "Server did not create the top-level autofill node. We " + << "might be running against an out-of-date server."; return false; } + sync_api::WriteTransaction trans( + sync_service_->backend()->GetUserShareHandle()); sync_api::ReadNode autofill_root(&trans); - if (!autofill_root.InitByTagLookup(kAutofillTag)) { + if (!autofill_root.InitByIdLookup(root_id)) { error_handler_->OnUnrecoverableError(); LOG(ERROR) << "Server did not create the top-level autofill node. We " << "might be running against an out-of-date server."; @@ -137,7 +141,8 @@ bool AutofillModelAssociator::AssociateModels() { sync_child_id = sync_child_node.GetSuccessorId(); } - if (!web_data_service->GetDatabase()->UpdateAutofillEntries(new_entries)) { + if (new_entries.size() && + !web_database_->UpdateAutofillEntries(new_entries)) { LOG(ERROR) << "Failed to update autofill entries."; sync_service_->OnUnrecoverableError(); return false; diff --git a/chrome/browser/sync/glue/autofill_model_associator.h b/chrome/browser/sync/glue/autofill_model_associator.h index d22fcba..53de4bc 100755 --- a/chrome/browser/sync/glue/autofill_model_associator.h +++ b/chrome/browser/sync/glue/autofill_model_associator.h @@ -18,6 +18,7 @@ #include "chrome/browser/webdata/autofill_entry.h" class ProfileSyncService; +class WebDatabase; namespace browser_sync { @@ -36,6 +37,7 @@ class AutofillModelAssociator public: static syncable::ModelType model_type() { return syncable::AUTOFILL; } AutofillModelAssociator(ProfileSyncService* sync_service, + WebDatabase* web_database, UnrecoverableErrorHandler* error_handler); virtual ~AutofillModelAssociator() { } @@ -77,13 +79,17 @@ class AutofillModelAssociator // Returns whether a node with the given permanent tag was found and update // |sync_id| with that node's id. - bool GetSyncIdForTaggedNode(const std::string& tag, int64* sync_id); + virtual bool GetSyncIdForTaggedNode(const std::string& tag, int64* sync_id); static std::string KeyToTag(const string16& name, const string16& value); static bool MergeTimestamps(const sync_pb::AutofillSpecifics& autofill, const std::vector<base::Time>& timestamps, std::vector<base::Time>* new_timestamps); + protected: + // Returns sync service instance. + ProfileSyncService* sync_service() { return sync_service_; } + private: typedef std::map<AutofillKey, int64> AutofillToSyncIdMap; typedef std::map<int64, AutofillKey> SyncIdToAutofillMap; @@ -96,6 +102,7 @@ class AutofillModelAssociator void PersistAssociations(); ProfileSyncService* sync_service_; + WebDatabase* web_database_; UnrecoverableErrorHandler* error_handler_; int64 autofill_node_id_; diff --git a/chrome/browser/sync/glue/data_type_controller.h b/chrome/browser/sync/glue/data_type_controller.h index 4f72ae2..a074889 100644 --- a/chrome/browser/sync/glue/data_type_controller.h +++ b/chrome/browser/sync/glue/data_type_controller.h @@ -94,7 +94,6 @@ class DataTypeController friend class ChromeThread; friend class DeleteTask<DataTypeController>; friend class ShutdownTask; - }; } // namespace browser_sync diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc index e774b85..63dbe73 100644 --- a/chrome/browser/sync/glue/sync_backend_host.cc +++ b/chrome/browser/sync/glue/sync_backend_host.cc @@ -136,6 +136,8 @@ void SyncBackendHost::Shutdown(bool sync_disabled) { void SyncBackendHost::ActivateDataType( DataTypeController* data_type_controller, ChangeProcessor* change_processor) { + // TODO(skrul): Add some kind of lock here that prevents concurrent + // calls. // Ensure that the given data type is in the PASSIVE group. browser_sync::ModelSafeRoutingInfo::iterator i = registrar_.routing_info.find(data_type_controller->type()); diff --git a/chrome/browser/sync/glue/sync_backend_host.h b/chrome/browser/sync/glue/sync_backend_host.h index 5024f89..92baeca 100644 --- a/chrome/browser/sync/glue/sync_backend_host.h +++ b/chrome/browser/sync/glue/sync_backend_host.h @@ -145,6 +145,7 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar { registrar_.workers[GROUP_UI] = new UIModelWorker(frontend_loop_); registrar_.routing_info[syncable::BOOKMARKS] = GROUP_PASSIVE; registrar_.routing_info[syncable::PREFERENCES] = GROUP_PASSIVE; + registrar_.routing_info[syncable::AUTOFILL] = GROUP_PASSIVE; core_thread_.message_loop()->PostTask(FROM_HERE, NewRunnableMethod(core_.get(), diff --git a/chrome/browser/sync/profile_sync_factory.h b/chrome/browser/sync/profile_sync_factory.h index c027b0f..b11856b 100644 --- a/chrome/browser/sync/profile_sync_factory.h +++ b/chrome/browser/sync/profile_sync_factory.h @@ -12,6 +12,7 @@ #include "chrome/browser/sync/glue/model_associator.h" class ProfileSyncService; +class WebDatabase; namespace browser_sync { class DataTypeManager; @@ -50,6 +51,7 @@ class ProfileSyncFactory { // by the caller. virtual SyncComponents CreateAutofillSyncComponents( ProfileSyncService* profile_sync_service, + WebDatabase* web_database, browser_sync::UnrecoverableErrorHandler* error_handler) = 0; // Instantiates both a model associator and change processor for the diff --git a/chrome/browser/sync/profile_sync_factory_impl.cc b/chrome/browser/sync/profile_sync_factory_impl.cc index 093cfbe..a1454c3 100644 --- a/chrome/browser/sync/profile_sync_factory_impl.cc +++ b/chrome/browser/sync/profile_sync_factory_impl.cc @@ -50,7 +50,7 @@ ProfileSyncService* ProfileSyncFactoryImpl::CreateProfileSyncService() { // explicitly enabled. if (command_line_->HasSwitch(switches::kEnableSyncAutofill)) { pss->RegisterDataTypeController( - new AutofillDataTypeController(this, pss)); + new AutofillDataTypeController(this, profile_, pss)); } // Bookmark sync is enabled by default. Register unless explicitly @@ -78,12 +78,15 @@ DataTypeManager* ProfileSyncFactoryImpl::CreateDataTypeManager( ProfileSyncFactory::SyncComponents ProfileSyncFactoryImpl::CreateAutofillSyncComponents( ProfileSyncService* profile_sync_service, + WebDatabase* web_database, browser_sync::UnrecoverableErrorHandler* error_handler) { AutofillModelAssociator* model_associator = new AutofillModelAssociator(profile_sync_service, + web_database, error_handler); AutofillChangeProcessor* change_processor = new AutofillChangeProcessor(model_associator, + web_database, error_handler); return SyncComponents(model_associator, change_processor); } diff --git a/chrome/browser/sync/profile_sync_factory_impl.h b/chrome/browser/sync/profile_sync_factory_impl.h index d193a59..94d6028 100644 --- a/chrome/browser/sync/profile_sync_factory_impl.h +++ b/chrome/browser/sync/profile_sync_factory_impl.h @@ -24,6 +24,7 @@ class ProfileSyncFactoryImpl : public ProfileSyncFactory { virtual SyncComponents CreateAutofillSyncComponents( ProfileSyncService* profile_sync_service, + WebDatabase* web_database, browser_sync::UnrecoverableErrorHandler* error_handler); virtual SyncComponents CreateBookmarkSyncComponents( diff --git a/chrome/browser/sync/profile_sync_factory_mock.h b/chrome/browser/sync/profile_sync_factory_mock.h index 1e09271..77ee37f 100644 --- a/chrome/browser/sync/profile_sync_factory_mock.h +++ b/chrome/browser/sync/profile_sync_factory_mock.h @@ -28,9 +28,10 @@ class ProfileSyncFactoryMock : public ProfileSyncFactory { browser_sync::DataTypeManager*( const browser_sync::DataTypeController::TypeMap& controllers)); - MOCK_METHOD2(CreateAutofillSyncComponents, + MOCK_METHOD3(CreateAutofillSyncComponents, SyncComponents( ProfileSyncService* profile_sync_service, + WebDatabase* web_database, browser_sync::UnrecoverableErrorHandler* error_handler)); MOCK_METHOD1(CreateBookmarkSyncComponents, SyncComponents(ProfileSyncService* profile_sync_service)); diff --git a/chrome/browser/sync/profile_sync_service_autofill_unittest.cc b/chrome/browser/sync/profile_sync_service_autofill_unittest.cc new file mode 100644 index 0000000..a91ec3b --- /dev/null +++ b/chrome/browser/sync/profile_sync_service_autofill_unittest.cc @@ -0,0 +1,316 @@ +// Copyright (c) 2010 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. + +#include <vector> + +#include "testing/gtest/include/gtest/gtest.h" + +#include "base/ref_counted.h" +#include "base/string16.h" +#include "base/time.h" +#include "base/waitable_event.h" +#include "chrome/browser/sync/engine/syncapi.h" +#include "chrome/browser/sync/glue/autofill_change_processor.h" +#include "chrome/browser/sync/glue/autofill_data_type_controller.h" +#include "chrome/browser/sync/glue/autofill_model_associator.h" +#include "chrome/browser/sync/glue/sync_backend_host.h" +#include "chrome/browser/sync/profile_sync_factory.h" +#include "chrome/browser/sync/profile_sync_factory_mock.h" +#include "chrome/browser/sync/profile_sync_service.h" +#include "chrome/browser/sync/profile_sync_test_util.h" +#include "chrome/browser/sync/protocol/autofill_specifics.pb.h" +#include "chrome/browser/sync/test_profile_sync_service.h" +#include "chrome/browser/webdata/autofill_entry.h" +#include "chrome/browser/webdata/web_database.h" +#include "chrome/common/notification_service.h" +#include "chrome/test/testing_profile.h" +#include "testing/gmock/include/gmock/gmock.h" + +using base::Time; +using base::WaitableEvent; +using browser_sync::AutofillChangeProcessor; +using browser_sync::AutofillDataTypeController; +using browser_sync::AutofillModelAssociator; +using browser_sync::SyncBackendHost; +using browser_sync::UnrecoverableErrorHandler; +using sync_api::SyncManager; +using testing::_; +using testing::DoAll; +using testing::DoDefault; +using testing::Return; +using testing::SetArgumentPointee; + +class TestAutofillModelAssociator + : public TestModelAssociator<AutofillModelAssociator> { + public: + TestAutofillModelAssociator(ProfileSyncService* service, + WebDatabase* web_database, + UnrecoverableErrorHandler* error_handler) + : TestModelAssociator<AutofillModelAssociator>( + service, web_database, error_handler) { + } +}; + +class WebDatabaseMock : public WebDatabase { + public: + MOCK_METHOD2(RemoveFormElement, + bool(const string16& name, const string16& value)); + MOCK_METHOD1(GetAllAutofillEntries, + bool(std::vector<AutofillEntry>* entries)); + MOCK_METHOD3(GetAutofillTimestamps, + bool(const string16& name, + const string16& value, + std::vector<base::Time>* timestamps)); + MOCK_METHOD1(UpdateAutofillEntries, + bool(const std::vector<AutofillEntry>& entries)); +}; + +class ProfileMock : public TestingProfile { + public: + MOCK_METHOD1(GetWebDataService, WebDataService*(ServiceAccessType access)); +}; + +class DBThreadNotificationService : + public base::RefCountedThreadSafe<DBThreadNotificationService> { + public: + DBThreadNotificationService() : done_event_(false, false) {} + + void Init() { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI)); + ChromeThread::PostTask( + ChromeThread::DB, + FROM_HERE, + NewRunnableMethod(this, &DBThreadNotificationService::InitTask)); + done_event_.Wait(); + } + + void TearDown() { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI)); + ChromeThread::PostTask( + ChromeThread::DB, + FROM_HERE, + NewRunnableMethod(this, &DBThreadNotificationService::TearDownTask)); + done_event_.Wait(); + } + + private: + friend class base::RefCountedThreadSafe<DBThreadNotificationService>; + + void InitTask() { + service_.reset(new NotificationService()); + done_event_.Signal(); + } + + void TearDownTask() { + service_.reset(NULL); + done_event_.Signal(); + } + + WaitableEvent done_event_; + scoped_ptr<NotificationService> service_; +}; + +class WebDataServiceFake : public WebDataService { + public: + virtual bool IsDatabaseLoaded() { + return true; + } + + // Note that we inject the WebDatabase through the + // ProfileSyncFactory mock. + virtual WebDatabase* GetDatabase() { + return NULL; + } +}; + +ACTION(QuitUIMessageLoop) { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI)); + MessageLoop::current()->Quit(); +} + +ACTION_P3(MakeAutofillSyncComponents, service, wd, dtc) { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::DB)); + AutofillModelAssociator* model_associator = + new TestAutofillModelAssociator(service, wd, dtc); + AutofillChangeProcessor* change_processor = + new AutofillChangeProcessor(model_associator, wd, service); + return ProfileSyncFactory::SyncComponents(model_associator, change_processor); +} + +class ProfileSyncServiceAutofillTest : public testing::Test { + protected: + ProfileSyncServiceAutofillTest() + : ui_thread_(ChromeThread::UI, &message_loop_), + db_thread_(ChromeThread::DB), + done_event_(false, false) { + } + + virtual void SetUp() { + web_data_service_ = new WebDataServiceFake(); + db_thread_.Start(); + + notification_service_ = new DBThreadNotificationService(); + notification_service_->Init(); + } + + virtual void TearDown() { + service_.reset(); + notification_service_->TearDown(); + db_thread_.Stop(); + MessageLoop::current()->RunAllPending(); + } + + void StartSyncService() { + if (!service_.get()) { + service_.reset(new TestProfileSyncService(&factory_, &profile_, false)); + service_->AddObserver(&observer_); + AutofillDataTypeController* data_type_controller = + new AutofillDataTypeController(&factory_, + &profile_, + service_.get()); + + EXPECT_CALL(factory_, CreateAutofillSyncComponents(_, _, _)). + WillOnce(MakeAutofillSyncComponents(service_.get(), + &web_database_, + data_type_controller)); + EXPECT_CALL(factory_, CreateDataTypeManager(_)). + WillOnce(MakeDataTypeManager()); + + EXPECT_CALL(profile_, GetWebDataService(_)). + WillOnce(Return(web_data_service_.get())); + + // State changes once for the backend init and once for startup done. + EXPECT_CALL(observer_, OnStateChanged()). + WillOnce(DoDefault()). + WillOnce(QuitUIMessageLoop()); + service_->RegisterDataTypeController(data_type_controller); + service_->Initialize(); + MessageLoop::current()->Run(); + } + } + + void GetAutofillEntriesFromSyncDB(std::vector<AutofillEntry>* entries) { + sync_api::ReadTransaction trans(service_->backend()->GetUserShareHandle()); + int64 autofill_root_id = + GetFakeServerTaggedNode(&trans, "google_chrome_autofill"); + if (autofill_root_id == sync_api::kInvalidId) + return; + + sync_api::ReadNode autofill_root(&trans); + if (!autofill_root.InitByIdLookup(autofill_root_id)) + return; + + int64 child_id = autofill_root.GetFirstChildId(); + while (child_id != sync_api::kInvalidId) { + sync_api::ReadNode child_node(&trans); + if (!child_node.InitByIdLookup(child_id)) + return; + + const sync_pb::AutofillSpecifics& autofill( + child_node.GetAutofillSpecifics()); + AutofillKey key(UTF8ToUTF16(autofill.name()), + UTF8ToUTF16(autofill.value())); + std::vector<base::Time> timestamps; + int timestamps_count = autofill.usage_timestamp_size(); + for (int i = 0; i < timestamps_count; ++i) { + timestamps.push_back(Time::FromInternalValue( + autofill.usage_timestamp(i))); + } + entries->push_back(AutofillEntry(key, timestamps)); + child_id = child_node.GetSuccessorId(); + } + } + + int64 GetFakeServerTaggedNode(sync_api::ReadTransaction* trans, + const std::string& tag) { + std::wstring tag_wide; + if (!UTF8ToWide(tag.c_str(), tag.length(), &tag_wide)) + return sync_api::kInvalidId; + + sync_api::ReadNode root(trans); + root.InitByRootLookup(); + + int64 last_child_id = sync_api::kInvalidId; + for (int64 id = root.GetFirstChildId(); id != sync_api::kInvalidId; /***/) { + sync_api::ReadNode child(trans); + child.InitByIdLookup(id); + last_child_id = id; + if (tag_wide == child.GetTitle()) { + return id; + } + id = child.GetSuccessorId(); + } + + return sync_api::kInvalidId; + } + + void SetIdleChangeProcessorExpectations() { + EXPECT_CALL(web_database_, RemoveFormElement(_, _)).Times(0); + EXPECT_CALL(web_database_, GetAutofillTimestamps(_, _, _)).Times(0); + EXPECT_CALL(web_database_, UpdateAutofillEntries(_)).Times(0); + } + + static AutofillEntry MakeAutofillEntry(const char* name, + const char* value, + time_t timestamp) { + std::vector<Time> timestamps; + timestamps.push_back(Time::FromTimeT(timestamp)); + return AutofillEntry( + AutofillKey(ASCIIToUTF16(name), ASCIIToUTF16(value)), timestamps); + } + + + MessageLoopForUI message_loop_; + ChromeThread ui_thread_; + ChromeThread db_thread_; + WaitableEvent done_event_; + scoped_refptr<DBThreadNotificationService> notification_service_; + + scoped_ptr<TestProfileSyncService> service_; + ProfileMock profile_; + ProfileSyncFactoryMock factory_; + ProfileSyncServiceObserverMock observer_; + WebDatabaseMock web_database_; + scoped_refptr<WebDataService> web_data_service_; +}; + +// TODO(sync): Test unrecoverable error during MA. + +TEST_F(ProfileSyncServiceAutofillTest, EmptyNativeEmptySync) { + EXPECT_CALL(web_database_, GetAllAutofillEntries(_)).WillOnce(Return(true)); + SetIdleChangeProcessorExpectations(); + StartSyncService(); + std::vector<AutofillEntry> sync_entries; + GetAutofillEntriesFromSyncDB(&sync_entries); + EXPECT_EQ(0U, sync_entries.size()); +} + +TEST_F(ProfileSyncServiceAutofillTest, HasNativeEmptySync) { + std::vector<AutofillEntry> entries; + entries.push_back(MakeAutofillEntry("foo", "bar", 1)); + EXPECT_CALL(web_database_, GetAllAutofillEntries(_)). + WillOnce(DoAll(SetArgumentPointee<0>(entries), Return(true))); + SetIdleChangeProcessorExpectations(); + StartSyncService(); + std::vector<AutofillEntry> sync_entries; + GetAutofillEntriesFromSyncDB(&sync_entries); + ASSERT_EQ(1U, entries.size()); + EXPECT_TRUE(entries[0] == sync_entries[0]); +} + +TEST_F(ProfileSyncServiceAutofillTest, HasNativeWithDuplicatesEmptySync) { + // There is buggy autofill code that allows duplicate name/value + // pairs to exist in the database with separate pair_ids. + std::vector<AutofillEntry> entries; + entries.push_back(MakeAutofillEntry("foo", "bar", 1)); + entries.push_back(MakeAutofillEntry("dup", "", 2)); + entries.push_back(MakeAutofillEntry("dup", "", 3)); + EXPECT_CALL(web_database_, GetAllAutofillEntries(_)). + WillOnce(DoAll(SetArgumentPointee<0>(entries), Return(true))); + SetIdleChangeProcessorExpectations(); + StartSyncService(); + std::vector<AutofillEntry> sync_entries; + GetAutofillEntriesFromSyncDB(&sync_entries); + EXPECT_EQ(2U, sync_entries.size()); +} diff --git a/chrome/browser/sync/profile_sync_service_startup_unittest.cc b/chrome/browser/sync/profile_sync_service_startup_unittest.cc index 6b65540..851f9b9 100644 --- a/chrome/browser/sync/profile_sync_service_startup_unittest.cc +++ b/chrome/browser/sync/profile_sync_service_startup_unittest.cc @@ -10,6 +10,7 @@ #include "chrome/browser/sync/glue/data_type_manager.h" #include "chrome/browser/sync/glue/data_type_manager_mock.h" #include "chrome/browser/sync/profile_sync_factory_mock.h" +#include "chrome/browser/sync/profile_sync_test_util.h" #include "chrome/browser/sync/test_profile_sync_service.h" #include "chrome/common/pref_names.h" #include "chrome/test/testing_profile.h" @@ -34,11 +35,6 @@ ACTION_P(InvokeCallback, callback_result) { #define SKIP_MACOSX(test) test #endif -class ProfileSyncServiceObserverMock : public ProfileSyncServiceObserver { - public: - MOCK_METHOD0(OnStateChanged, void()); -}; - class ProfileSyncServiceStartupTest : public testing::Test { public: ProfileSyncServiceStartupTest() diff --git a/chrome/browser/sync/profile_sync_test_util.h b/chrome/browser/sync/profile_sync_test_util.h index 14fcaec..65b5263 100644 --- a/chrome/browser/sync/profile_sync_test_util.h +++ b/chrome/browser/sync/profile_sync_test_util.h @@ -5,6 +5,7 @@ #ifndef CHROME_BROWSER_SYNC_PROFILE_SYNC_TEST_UTIL_H_ #define CHROME_BROWSER_SYNC_PROFILE_SYNC_TEST_UTIL_H_ +#include "chrome/browser/webdata/web_database.h" #include "chrome/browser/sync/glue/bookmark_change_processor.h" #include "chrome/browser/sync/glue/bookmark_data_type_controller.h" #include "chrome/browser/sync/glue/bookmark_model_associator.h" @@ -28,6 +29,12 @@ class TestModelAssociator : public ModelAssociatorImpl { : ModelAssociatorImpl(service) { } + TestModelAssociator(ProfileSyncService* service, + WebDatabase* web_database, + browser_sync::UnrecoverableErrorHandler* error_handler) + : ModelAssociatorImpl(service, web_database, error_handler) { + } + virtual bool GetSyncIdForTaggedNode(const std::string& tag, int64* sync_id) { std::wstring tag_wide; if (!UTF8ToWide(tag.c_str(), tag.length(), &tag_wide)) { @@ -74,4 +81,9 @@ class TestModelAssociator : public ModelAssociatorImpl { ~TestModelAssociator() {} }; +class ProfileSyncServiceObserverMock : public ProfileSyncServiceObserver { + public: + MOCK_METHOD0(OnStateChanged, void()); +}; + #endif // CHROME_BROWSER_SYNC_PROFILE_SYNC_TEST_UTIL_H_ diff --git a/chrome/browser/webdata/web_data_service.h b/chrome/browser/webdata/web_data_service.h index c6887f4..004532a 100644 --- a/chrome/browser/webdata/web_data_service.h +++ b/chrome/browser/webdata/web_data_service.h @@ -448,10 +448,12 @@ class WebDataService void set_failed_init(bool value) { failed_init_ = value; } #endif - bool IsDatabaseLoaded(); - WebDatabase* GetDatabase(); + virtual bool IsDatabaseLoaded(); + virtual WebDatabase* GetDatabase(); protected: + virtual ~WebDataService(); + friend class TemplateURLModelTest; friend class TemplateURLModelTestingProfile; friend class WebDataServiceTest; @@ -480,8 +482,6 @@ class WebDataService typedef GenericRequest2<std::vector<const TemplateURL*>, std::vector<TemplateURL*> > SetKeywordsRequest; - ~WebDataService(); - // Invoked on the main thread if initializing the db fails. void DBInitFailed(sql::InitStatus init_status); diff --git a/chrome/browser/webdata/web_database.h b/chrome/browser/webdata/web_database.h index 6d1a99d..06727e9 100644 --- a/chrome/browser/webdata/web_database.h +++ b/chrome/browser/webdata/web_database.h @@ -44,7 +44,7 @@ struct IE7PasswordInfo; class WebDatabase { public: WebDatabase(); - ~WebDatabase(); + virtual ~WebDatabase(); // Initialize the database given a name. The name defines where the sqlite // file is. If this returns an error code, no other method should be called. @@ -201,20 +201,20 @@ class WebDatabase { bool RemoveFormElementForID(int64 pair_id); // Removes row from the autofill tables for the given |name| |value| pair. - bool RemoveFormElement(const string16& name, const string16& value); + virtual bool RemoveFormElement(const string16& name, const string16& value); // Retrieves all of the entries in the autofill table. - bool GetAllAutofillEntries(std::vector<AutofillEntry>* entries); + virtual bool GetAllAutofillEntries(std::vector<AutofillEntry>* entries); // Retrieves a single entry from the autofill table. - bool GetAutofillTimestamps(const string16& name, + virtual bool GetAutofillTimestamps(const string16& name, const string16& value, std::vector<base::Time>* timestamps); // Replaces existing autofill entries with the entries supplied in // the argument. If the entry does not already exist, it will be // added. - bool UpdateAutofillEntries(const std::vector<AutofillEntry>& entries); + virtual bool UpdateAutofillEntries(const std::vector<AutofillEntry>& entries); // Records a single AutoFill profile in the autofill_profiles table. bool AddAutoFillProfile(const AutoFillProfile& profile); diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index 287065e..53a7c65 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -840,6 +840,7 @@ 'browser/sync/profile_sync_factory_impl_unittest.cc', 'browser/sync/profile_sync_factory_mock.cc', 'browser/sync/profile_sync_factory_mock.h', + 'browser/sync/profile_sync_service_autofill_unittest.cc', 'browser/sync/profile_sync_service_preference_unittest.cc', 'browser/sync/profile_sync_test_util.h', 'browser/sync/sync_setup_wizard_unittest.cc', |