Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3409)

Unified Diff: chrome/browser/sync/profile_sync_service_autofill_unittest.cc

Issue 8184001: The AutofillProfileSyncableService's lifetime should be managed by the WebDataService (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Ensure that destruction occurs on the DB thread Created 9 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/sync/profile_sync_service_autofill_unittest.cc
diff --git a/chrome/browser/sync/profile_sync_service_autofill_unittest.cc b/chrome/browser/sync/profile_sync_service_autofill_unittest.cc
index d196ce408a8143175648886aed4374d5c6c163e4..b7f3725b5245f365271868ff70726679273f76d0 100644
--- a/chrome/browser/sync/profile_sync_service_autofill_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_autofill_unittest.cc
@@ -8,6 +8,7 @@
#include "testing/gtest/include/gtest/gtest.h"
+#include "base/bind.h"
#include "base/callback.h"
#include "base/location.h"
#include "base/memory/ref_counted.h"
@@ -60,7 +61,6 @@ using browser_sync::AutofillChangeProcessor;
using browser_sync::AutofillDataTypeController;
using browser_sync::AutofillModelAssociator;
using browser_sync::AutofillProfileDataTypeController;
-using browser_sync::AutofillProfileSyncableService;
using browser_sync::DataTypeController;
using browser_sync::GenericChangeProcessor;
using browser_sync::SyncableServiceAdapter;
@@ -184,7 +184,7 @@ ACTION_P3(MakeAutofillProfileSyncComponents, service, wd, dtc) {
if (!BrowserThread::CurrentlyOn(BrowserThread::DB))
return ProfileSyncFactory::SyncComponents(NULL, NULL);
AutofillProfileSyncableService* sync_service =
- new AutofillProfileSyncableService(wd, service->profile());
+ wd->GetAutofillTable()->GetSyncableService(wd, service->profile());
sync_api::UserShare* user_share = service->GetUserShare();
GenericChangeProcessor* change_processor =
new GenericChangeProcessor(sync_service, dtc, user_share);
@@ -215,14 +215,13 @@ class AutofillEntryFactory : public AbstractAutofillFactory {
ProfileSyncFactory* factory,
ProfileMock* profile,
ProfileSyncService* service) {
- return new AutofillDataTypeController(factory,
- profile);
+ return new AutofillDataTypeController(factory, profile);
}
void SetExpectation(ProfileSyncFactoryMock* factory,
- ProfileSyncService* service,
- WebDatabase* wd,
- DataTypeController* dtc) {
+ ProfileSyncService* service,
+ WebDatabase* wd,
+ DataTypeController* dtc) {
EXPECT_CALL(*factory, CreateAutofillSyncComponents(_,_,_)).
WillOnce(MakeAutofillSyncComponents(service, wd, dtc));
}
@@ -234,14 +233,13 @@ class AutofillProfileFactory : public AbstractAutofillFactory {
ProfileSyncFactory* factory,
ProfileMock* profile,
ProfileSyncService* service) {
- return new AutofillProfileDataTypeController(factory,
- profile);
+ return new AutofillProfileDataTypeController(factory, profile);
}
void SetExpectation(ProfileSyncFactoryMock* factory,
- ProfileSyncService* service,
- WebDatabase* wd,
- DataTypeController* dtc) {
+ ProfileSyncService* service,
+ WebDatabase* wd,
+ DataTypeController* dtc) {
EXPECT_CALL(*factory, CreateAutofillProfileSyncComponents(_,_,_)).
WillOnce(MakeAutofillProfileSyncComponents(service, wd, dtc));
}
@@ -262,7 +260,9 @@ template <class T> class AddAutofillTask;
class ProfileSyncServiceAutofillTest : public AbstractProfileSyncServiceTest {
protected:
- ProfileSyncServiceAutofillTest() {}
+ ProfileSyncServiceAutofillTest()
+ : autofill_table_created_or_destroyed_(false, false) {
+ }
AutofillProfileFactory profile_factory_;
AutofillEntryFactory entry_factory_;
@@ -281,7 +281,14 @@ class ProfileSyncServiceAutofillTest : public AbstractProfileSyncServiceTest {
virtual void SetUp() {
AbstractProfileSyncServiceTest::SetUp();
profile_.CreateRequestContext();
- web_database_.reset(new WebDatabaseFake(&autofill_table_));
+
+ // The |autofill_table_| must be constructed on the DB thread.
+ BrowserThread::PostTask(BrowserThread::DB, FROM_HERE,
+ base::Bind(&ProfileSyncServiceAutofillTest::CreateAutofillTable,
+ base::Unretained(this)));
+ ASSERT_TRUE(autofill_table_created_or_destroyed_.Wait());
+
+ web_database_.reset(new WebDatabaseFake(autofill_table_.get()));
web_data_service_ = new WebDataServiceFake(web_database_.get());
personal_data_manager_ = static_cast<PersonalDataManagerMock*>(
PersonalDataManagerFactory::GetInstance()->SetTestingFactoryAndUse(
@@ -303,10 +310,29 @@ class ProfileSyncServiceAutofillTest : public AbstractProfileSyncServiceTest {
virtual void TearDown() {
service_.reset();
notification_service_->TearDown();
+
+ // The |autofill_table_| must be destroyed on the DB thread.
+ BrowserThread::PostTask(BrowserThread::DB, FROM_HERE,
+ base::Bind(&ProfileSyncServiceAutofillTest::DestroyAutofillTable,
+ base::Unretained(this)));
+ ASSERT_TRUE(autofill_table_created_or_destroyed_.Wait());
+
profile_.ResetRequestContext();
AbstractProfileSyncServiceTest::TearDown();
}
+ void CreateAutofillTable() {
+ ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::DB));
+ autofill_table_.reset(new AutofillTableMock);
+ autofill_table_created_or_destroyed_.Signal();
+ }
+
+ void DestroyAutofillTable() {
+ ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::DB));
+ autofill_table_.reset();
+ autofill_table_created_or_destroyed_.Signal();
+ }
+
void StartSyncService(Task* task,
bool will_fail_association,
syncable::ModelType type) {
@@ -335,8 +361,7 @@ class ProfileSyncServiceAutofillTest : public AbstractProfileSyncServiceTest {
WillRepeatedly(Return(true));
// We need tokens to get the tests going
- token_service_->IssueAuthTokenForTest(
- GaiaConstants::kSyncService, "token");
+ token_service_->IssueAuthTokenForTest(GaiaConstants::kSyncService, "token");
EXPECT_CALL(profile_, GetTokenService()).
WillRepeatedly(Return(token_service_.get()));
@@ -365,7 +390,7 @@ class ProfileSyncServiceAutofillTest : public AbstractProfileSyncServiceTest {
bool AddAutofillSyncNode(const AutofillProfile& profile) {
sync_api::WriteTransaction trans(FROM_HERE, service_->GetUserShare());
sync_api::ReadNode autofill_root(&trans);
- if (!autofill_root.InitByTagLookup(browser_sync::kAutofillProfileTag))
+ if (!autofill_root.InitByTagLookup(kAutofillProfileTag))
return false;
sync_api::WriteNode node(&trans);
std::string tag = profile.guid();
@@ -421,7 +446,7 @@ class ProfileSyncServiceAutofillTest : public AbstractProfileSyncServiceTest {
std::vector<AutofillProfile>* profiles) {
sync_api::ReadTransaction trans(FROM_HERE, service_->GetUserShare());
sync_api::ReadNode autofill_root(&trans);
- if (!autofill_root.InitByTagLookup(browser_sync::kAutofillProfileTag))
+ if (!autofill_root.InitByTagLookup(kAutofillProfileTag))
return false;
int64 child_id = autofill_root.GetFirstChildId();
@@ -443,9 +468,9 @@ class ProfileSyncServiceAutofillTest : public AbstractProfileSyncServiceTest {
}
void SetIdleChangeProcessorExpectations() {
- EXPECT_CALL(autofill_table_, RemoveFormElement(_, _)).Times(0);
- EXPECT_CALL(autofill_table_, GetAutofillTimestamps(_, _, _)).Times(0);
- EXPECT_CALL(autofill_table_, UpdateAutofillEntries(_)).Times(0);
+ EXPECT_CALL(*autofill_table_, RemoveFormElement(_, _)).Times(0);
+ EXPECT_CALL(*autofill_table_, GetAutofillTimestamps(_, _, _)).Times(0);
+ EXPECT_CALL(*autofill_table_, UpdateAutofillEntries(_)).Times(0);
}
static AutofillEntry MakeAutofillEntry(const char* name,
@@ -474,7 +499,8 @@ class ProfileSyncServiceAutofillTest : public AbstractProfileSyncServiceTest {
scoped_refptr<ThreadNotificationService> notification_service_;
ProfileMock profile_;
- AutofillTableMock autofill_table_;
+ scoped_ptr<AutofillTableMock> autofill_table_;
+ WaitableEvent autofill_table_created_or_destroyed_;
scoped_ptr<WebDatabaseFake> web_database_;
scoped_refptr<WebDataService> web_data_service_;
PersonalDataManagerMock* personal_data_manager_;
@@ -484,7 +510,7 @@ template <class T>
class AddAutofillTask : public Task {
public:
AddAutofillTask(ProfileSyncServiceAutofillTest* test,
- const std::vector<T>& entries)
+ const std::vector<T>& entries)
: test_(test), entries_(entries), success_(false) {
}
@@ -640,8 +666,9 @@ TEST_F(ProfileSyncServiceAutofillTest, FailModelAssociation) {
}
TEST_F(ProfileSyncServiceAutofillTest, EmptyNativeEmptySync) {
- EXPECT_CALL(autofill_table_, GetAllAutofillEntries(_)).WillOnce(Return(true));
- EXPECT_CALL(autofill_table_, GetAutofillProfiles(_)).WillOnce(Return(true));
+ EXPECT_CALL(*autofill_table_,
+ GetAllAutofillEntries(_)).WillOnce(Return(true));
+ EXPECT_CALL(*autofill_table_, GetAutofillProfiles(_)).WillOnce(Return(true));
SetIdleChangeProcessorExpectations();
CreateRootTask task(this, syncable::AUTOFILL);
EXPECT_CALL(*personal_data_manager_, Refresh());
@@ -657,9 +684,9 @@ TEST_F(ProfileSyncServiceAutofillTest, EmptyNativeEmptySync) {
TEST_F(ProfileSyncServiceAutofillTest, HasNativeEntriesEmptySync) {
std::vector<AutofillEntry> entries;
entries.push_back(MakeAutofillEntry("foo", "bar", 1));
- EXPECT_CALL(autofill_table_, GetAllAutofillEntries(_)).
+ EXPECT_CALL(*autofill_table_, GetAllAutofillEntries(_)).
WillOnce(DoAll(SetArgumentPointee<0>(entries), Return(true)));
- EXPECT_CALL(autofill_table_, GetAutofillProfiles(_)).WillOnce(Return(true));
+ EXPECT_CALL(*autofill_table_, GetAutofillProfiles(_)).WillOnce(Return(true));
SetIdleChangeProcessorExpectations();
CreateRootTask task(this, syncable::AUTOFILL);
EXPECT_CALL(*personal_data_manager_, Refresh());
@@ -685,7 +712,7 @@ TEST_F(ProfileSyncServiceAutofillTest, HasProfileEmptySync) {
"91601", "US", "12345678910");
profiles.push_back(profile0);
expected_profiles.push_back(*profile0);
- EXPECT_CALL(autofill_table_, GetAutofillProfiles(_)).
+ EXPECT_CALL(*autofill_table_, GetAutofillProfiles(_)).
WillOnce(DoAll(SetArgumentPointee<0>(profiles), Return(true)));
EXPECT_CALL(*personal_data_manager_, Refresh());
SetIdleChangeProcessorExpectations();
@@ -705,9 +732,9 @@ TEST_F(ProfileSyncServiceAutofillTest, HasNativeWithDuplicatesEmptySync) {
entries.push_back(MakeAutofillEntry("foo", "bar", 1));
entries.push_back(MakeAutofillEntry("dup", "", 2));
entries.push_back(MakeAutofillEntry("dup", "", 3));
- EXPECT_CALL(autofill_table_, GetAllAutofillEntries(_)).
+ EXPECT_CALL(*autofill_table_, GetAllAutofillEntries(_)).
WillOnce(DoAll(SetArgumentPointee<0>(entries), Return(true)));
- EXPECT_CALL(autofill_table_, GetAutofillProfiles(_)).WillOnce(Return(true));
+ EXPECT_CALL(*autofill_table_, GetAutofillProfiles(_)).WillOnce(Return(true));
SetIdleChangeProcessorExpectations();
CreateRootTask task(this, syncable::AUTOFILL);
EXPECT_CALL(*personal_data_manager_, Refresh());
@@ -726,17 +753,17 @@ TEST_F(ProfileSyncServiceAutofillTest, HasNativeHasSyncNoMerge) {
std::vector<AutofillEntry> native_entries;
native_entries.push_back(native_entry);
- EXPECT_CALL(autofill_table_, GetAllAutofillEntries(_)).
+ EXPECT_CALL(*autofill_table_, GetAllAutofillEntries(_)).
WillOnce(DoAll(SetArgumentPointee<0>(native_entries), Return(true)));
- EXPECT_CALL(autofill_table_, GetAutofillProfiles(_)).WillOnce(Return(true));
+ EXPECT_CALL(*autofill_table_, GetAutofillProfiles(_)).WillOnce(Return(true));
std::vector<AutofillEntry> sync_entries;
sync_entries.push_back(sync_entry);
AddAutofillTask<AutofillEntry> task(this, sync_entries);
- EXPECT_CALL(autofill_table_, UpdateAutofillEntries(ElementsAre(sync_entry))).
+ EXPECT_CALL(*autofill_table_, UpdateAutofillEntries(ElementsAre(sync_entry))).
WillOnce(Return(true));
EXPECT_CALL(*personal_data_manager_, Refresh());
@@ -764,15 +791,15 @@ TEST_F(ProfileSyncServiceAutofillTest, HasNativeHasSyncMergeEntry) {
std::vector<AutofillEntry> native_entries;
native_entries.push_back(native_entry);
- EXPECT_CALL(autofill_table_, GetAllAutofillEntries(_)).
+ EXPECT_CALL(*autofill_table_, GetAllAutofillEntries(_)).
WillOnce(DoAll(SetArgumentPointee<0>(native_entries), Return(true)));
- EXPECT_CALL(autofill_table_, GetAutofillProfiles(_)).WillOnce(Return(true));
+ EXPECT_CALL(*autofill_table_, GetAutofillProfiles(_)).WillOnce(Return(true));
std::vector<AutofillEntry> sync_entries;
sync_entries.push_back(sync_entry);
AddAutofillTask<AutofillEntry> task(this, sync_entries);
- EXPECT_CALL(autofill_table_,
+ EXPECT_CALL(*autofill_table_,
UpdateAutofillEntries(ElementsAre(merged_entry))).WillOnce(Return(true));
EXPECT_CALL(*personal_data_manager_, Refresh());
StartSyncService(&task, false, syncable::AUTOFILL);
@@ -802,14 +829,14 @@ TEST_F(ProfileSyncServiceAutofillTest, HasNativeHasSyncMergeProfile) {
std::vector<AutofillProfile*> native_profiles;
native_profiles.push_back(native_profile);
- EXPECT_CALL(autofill_table_, GetAutofillProfiles(_)).
+ EXPECT_CALL(*autofill_table_, GetAutofillProfiles(_)).
WillOnce(DoAll(SetArgumentPointee<0>(native_profiles), Return(true)));
std::vector<AutofillProfile> sync_profiles;
sync_profiles.push_back(sync_profile);
AddAutofillTask<AutofillProfile> task(this, sync_profiles);
- EXPECT_CALL(autofill_table_, UpdateAutofillProfile(_)).
+ EXPECT_CALL(*autofill_table_, UpdateAutofillProfile(_)).
WillOnce(Return(true));
EXPECT_CALL(*personal_data_manager_, Refresh());
StartSyncService(&task, false, syncable::AUTOFILL_PROFILE);
@@ -841,16 +868,16 @@ TEST_F(ProfileSyncServiceAutofillTest, MergeProfileWithDifferentGuid) {
std::vector<AutofillProfile*> native_profiles;
native_profiles.push_back(native_profile);
- EXPECT_CALL(autofill_table_, GetAutofillProfiles(_)).
+ EXPECT_CALL(*autofill_table_, GetAutofillProfiles(_)).
WillOnce(DoAll(SetArgumentPointee<0>(native_profiles), Return(true)));
std::vector<AutofillProfile> sync_profiles;
sync_profiles.push_back(sync_profile);
AddAutofillTask<AutofillProfile> task(this, sync_profiles);
- EXPECT_CALL(autofill_table_, AddAutofillProfile(_)).
+ EXPECT_CALL(*autofill_table_, AddAutofillProfile(_)).
WillOnce(Return(true));
- EXPECT_CALL(autofill_table_, RemoveAutofillProfile(native_guid)).
+ EXPECT_CALL(*autofill_table_, RemoveAutofillProfile(native_guid)).
WillOnce(Return(true));
EXPECT_CALL(*personal_data_manager_, Refresh());
StartSyncService(&task, false, syncable::AUTOFILL_PROFILE);
@@ -865,8 +892,9 @@ TEST_F(ProfileSyncServiceAutofillTest, MergeProfileWithDifferentGuid) {
}
TEST_F(ProfileSyncServiceAutofillTest, ProcessUserChangeAddEntry) {
- EXPECT_CALL(autofill_table_, GetAllAutofillEntries(_)).WillOnce(Return(true));
- EXPECT_CALL(autofill_table_, GetAutofillProfiles(_)).WillOnce(Return(true));
+ EXPECT_CALL(*autofill_table_,
+ GetAllAutofillEntries(_)).WillOnce(Return(true));
+ EXPECT_CALL(*autofill_table_, GetAutofillProfiles(_)).WillOnce(Return(true));
EXPECT_CALL(*personal_data_manager_, Refresh());
SetIdleChangeProcessorExpectations();
CreateRootTask task(this, syncable::AUTOFILL);
@@ -876,7 +904,7 @@ TEST_F(ProfileSyncServiceAutofillTest, ProcessUserChangeAddEntry) {
AutofillEntry added_entry(MakeAutofillEntry("added", "entry", 1));
std::vector<base::Time> timestamps(added_entry.timestamps());
- EXPECT_CALL(autofill_table_, GetAutofillTimestamps(_, _, _)).
+ EXPECT_CALL(*autofill_table_, GetAutofillTimestamps(_, _, _)).
WillOnce(DoAll(SetArgumentPointee<2>(timestamps), Return(true)));
AutofillChangeList changes;
@@ -895,7 +923,7 @@ TEST_F(ProfileSyncServiceAutofillTest, ProcessUserChangeAddEntry) {
}
TEST_F(ProfileSyncServiceAutofillTest, ProcessUserChangeAddProfile) {
- EXPECT_CALL(autofill_table_, GetAutofillProfiles(_)).WillOnce(Return(true));
+ EXPECT_CALL(*autofill_table_, GetAutofillProfiles(_)).WillOnce(Return(true));
EXPECT_CALL(*personal_data_manager_, Refresh());
SetIdleChangeProcessorExpectations();
CreateRootTask task(this, syncable::AUTOFILL_PROFILE);
@@ -927,9 +955,9 @@ TEST_F(ProfileSyncServiceAutofillTest, ProcessUserChangeUpdateEntry) {
std::vector<AutofillEntry> original_entries;
original_entries.push_back(original_entry);
- EXPECT_CALL(autofill_table_, GetAllAutofillEntries(_)).
+ EXPECT_CALL(*autofill_table_, GetAllAutofillEntries(_)).
WillOnce(DoAll(SetArgumentPointee<0>(original_entries), Return(true)));
- EXPECT_CALL(autofill_table_, GetAutofillProfiles(_)).WillOnce(Return(true));
+ EXPECT_CALL(*autofill_table_, GetAutofillProfiles(_)).WillOnce(Return(true));
EXPECT_CALL(*personal_data_manager_, Refresh());
CreateRootTask task(this, syncable::AUTOFILL);
StartSyncService(&task, false, syncable::AUTOFILL);
@@ -938,7 +966,7 @@ TEST_F(ProfileSyncServiceAutofillTest, ProcessUserChangeUpdateEntry) {
AutofillEntry updated_entry(MakeAutofillEntry("my", "entry", 1, 2));
std::vector<base::Time> timestamps(updated_entry.timestamps());
- EXPECT_CALL(autofill_table_, GetAutofillTimestamps(_, _, _)).
+ EXPECT_CALL(*autofill_table_, GetAutofillTimestamps(_, _, _)).
WillOnce(DoAll(SetArgumentPointee<2>(timestamps), Return(true)));
AutofillChangeList changes;
@@ -963,9 +991,9 @@ TEST_F(ProfileSyncServiceAutofillTest, ProcessUserChangeRemoveEntry) {
std::vector<AutofillEntry> original_entries;
original_entries.push_back(original_entry);
- EXPECT_CALL(autofill_table_, GetAllAutofillEntries(_)).
+ EXPECT_CALL(*autofill_table_, GetAllAutofillEntries(_)).
WillOnce(DoAll(SetArgumentPointee<0>(original_entries), Return(true)));
- EXPECT_CALL(autofill_table_, GetAutofillProfiles(_)).WillOnce(Return(true));
+ EXPECT_CALL(*autofill_table_, GetAutofillProfiles(_)).WillOnce(Return(true));
EXPECT_CALL(*personal_data_manager_, Refresh());
CreateRootTask task(this, syncable::AUTOFILL);
StartSyncService(&task, false, syncable::AUTOFILL);
@@ -1000,7 +1028,7 @@ TEST_F(ProfileSyncServiceAutofillTest, ProcessUserChangeRemoveProfile) {
std::vector<AutofillProfile*> native_profiles;
native_profiles.push_back(native_profile);
- EXPECT_CALL(autofill_table_, GetAutofillProfiles(_)).
+ EXPECT_CALL(*autofill_table_, GetAutofillProfiles(_)).
WillOnce(DoAll(SetArgumentPointee<0>(native_profiles), Return(true)));
std::vector<AutofillProfile> sync_profiles;
@@ -1024,8 +1052,9 @@ TEST_F(ProfileSyncServiceAutofillTest, ProcessUserChangeRemoveProfile) {
}
TEST_F(ProfileSyncServiceAutofillTest, ProcessUserChangeError) {
- EXPECT_CALL(autofill_table_, GetAllAutofillEntries(_)).WillOnce(Return(true));
- EXPECT_CALL(autofill_table_, GetAutofillProfiles(_)).WillOnce(Return(true));
+ EXPECT_CALL(*autofill_table_,
+ GetAllAutofillEntries(_)).WillOnce(Return(true));
+ EXPECT_CALL(*autofill_table_, GetAutofillProfiles(_)).WillOnce(Return(true));
EXPECT_CALL(*personal_data_manager_, Refresh());
CreateRootTask task(this, syncable::AUTOFILL);
StartSyncService(&task, false, syncable::AUTOFILL);
@@ -1059,9 +1088,10 @@ TEST_F(ProfileSyncServiceAutofillTest, ProcessUserChangeError) {
// Crashy, http://crbug.com/57884
TEST_F(ProfileSyncServiceAutofillTest, DISABLED_ServerChangeRace) {
- EXPECT_CALL(autofill_table_, GetAllAutofillEntries(_)).WillOnce(Return(true));
- EXPECT_CALL(autofill_table_, GetAutofillProfiles(_)).WillOnce(Return(true));
- EXPECT_CALL(autofill_table_, UpdateAutofillEntries(_)).
+ EXPECT_CALL(*autofill_table_,
+ GetAllAutofillEntries(_)).WillOnce(Return(true));
+ EXPECT_CALL(*autofill_table_, GetAutofillProfiles(_)).WillOnce(Return(true));
+ EXPECT_CALL(*autofill_table_, UpdateAutofillEntries(_)).
WillRepeatedly(Return(true));
EXPECT_CALL(*personal_data_manager_, Refresh()).Times(3);
CreateRootTask task(this, syncable::AUTOFILL);

Powered by Google App Engine
This is Rietveld 408576698