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

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: chrome/browser/search_engines/template_url_service_sync_unittest.cc

Issue 10806065: Rewrite TemplateURLService's SyncableService implmentation to avoid sending ACTION_DELETEs to Sync. (Closed) Base URL: http://git.chromium.org/chromium/src.git@master
Patch Set: Cleanup new code Created 8 years, 5 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
« chrome/browser/search_engines/template_url_service.cc ('K') | « chrome/browser/search_engines/template_url_service.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/search_engines/template_url_service_sync_unittest.cc
diff --git a/chrome/browser/search_engines/template_url_service_sync_unittest.cc b/chrome/browser/search_engines/template_url_service_sync_unittest.cc
index fd034ee37bf4f2279c58949485f90d9e6183f3a5..c1174954b1c33072aabfe4b37e564802c69a165c 100644
--- a/chrome/browser/search_engines/template_url_service_sync_unittest.cc
+++ b/chrome/browser/search_engines/template_url_service_sync_unittest.cc
@@ -471,296 +471,6 @@ TEST_F(TemplateURLServiceSyncTest, UniquifyKeyword) {
EXPECT_EQ(NULL, model()->GetTemplateURLForKeyword(new_keyword));
}
-TEST_F(TemplateURLServiceSyncTest, SyncKeywordConflictNeitherAutoreplace) {
- // This tests cases where neither the sync nor the local TemplateURL are
- // marked safe_for_autoreplace.
-
- // Create a keyword that conflicts, and make it older. Sync keyword is
- // uniquified, and a syncer::SyncChange is added.
- string16 original_turl_keyword = ASCIIToUTF16("key1");
- TemplateURL* original_turl = CreateTestTemplateURL(original_turl_keyword,
- "http://key1.com", std::string(), 9000);
- model()->Add(original_turl);
- scoped_ptr<TemplateURL> sync_turl(CreateTestTemplateURL(original_turl_keyword,
- "http://new.com", "remote", 8999));
- syncer::SyncChangeList changes;
- EXPECT_TRUE(model()->ResolveSyncKeywordConflict(sync_turl.get(),
- original_turl, &changes));
- EXPECT_NE(original_turl_keyword, sync_turl->keyword());
- EXPECT_EQ(original_turl_keyword, original_turl->keyword());
- ASSERT_EQ(1U, changes.size());
- EXPECT_EQ("remote", GetGUID(changes[0].sync_data()));
- EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, changes[0].change_type());
- changes.clear();
- model()->Remove(original_turl);
-
- // Sync is newer. Original TemplateURL keyword is uniquified. A SyncChange
- // is added (which in a normal run would be deleted by PruneSyncChanges() when
- // the local GUID doesn't appear in the sync GUID list). Also ensure that
- // this does not change the safe_for_autoreplace flag or the TemplateURLID in
- // the original.
- original_turl = CreateTestTemplateURL(original_turl_keyword,
- "http://key1.com", "local", 9000);
- model()->Add(original_turl);
- TemplateURLID original_id = original_turl->id();
- sync_turl.reset(CreateTestTemplateURL(original_turl_keyword, "http://new.com",
- std::string(), 9001));
- EXPECT_TRUE(model()->ResolveSyncKeywordConflict(sync_turl.get(),
- original_turl, &changes));
- EXPECT_EQ(original_turl_keyword, sync_turl->keyword());
- EXPECT_NE(original_turl_keyword, original_turl->keyword());
- EXPECT_FALSE(original_turl->safe_for_autoreplace());
- EXPECT_EQ(original_id, original_turl->id());
- EXPECT_EQ(NULL, model()->GetTemplateURLForKeyword(original_turl_keyword));
- ASSERT_EQ(1U, changes.size());
- EXPECT_EQ("local", GetGUID(changes[0].sync_data()));
- EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, changes[0].change_type());
- changes.clear();
- model()->Remove(original_turl);
-
- // Equal times. Same result as above. Sync left alone, original uniquified so
- // sync_turl can fit.
- original_turl = CreateTestTemplateURL(original_turl_keyword,
- "http://key1.com", "local2", 9000);
- model()->Add(original_turl);
- sync_turl.reset(CreateTestTemplateURL(original_turl_keyword, "http://new.com",
- std::string(), 9000));
- EXPECT_TRUE(model()->ResolveSyncKeywordConflict(sync_turl.get(),
- original_turl, &changes));
- EXPECT_EQ(original_turl_keyword, sync_turl->keyword());
- EXPECT_NE(original_turl_keyword, original_turl->keyword());
- EXPECT_EQ(NULL, model()->GetTemplateURLForKeyword(original_turl_keyword));
- ASSERT_EQ(1U, changes.size());
- EXPECT_EQ("local2", GetGUID(changes[0].sync_data()));
- EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, changes[0].change_type());
- changes.clear();
- model()->Remove(original_turl);
-
- // Sync is newer, but original TemplateURL is created by policy, so it wins.
- // Sync keyword is uniquified, and a syncer::SyncChange is added.
- original_turl = CreateTestTemplateURL(original_turl_keyword,
- "http://key1.com", std::string(), 9000, false, true);
- model()->Add(original_turl);
- sync_turl.reset(CreateTestTemplateURL(original_turl_keyword, "http://new.com",
- "remote2", 9999));
- EXPECT_TRUE(model()->ResolveSyncKeywordConflict(sync_turl.get(),
- original_turl, &changes));
- EXPECT_NE(original_turl_keyword, sync_turl->keyword());
- EXPECT_EQ(original_turl_keyword, original_turl->keyword());
- EXPECT_EQ(NULL, model()->GetTemplateURLForKeyword(sync_turl->keyword()));
- ASSERT_EQ(1U, changes.size());
- EXPECT_EQ("remote2", GetGUID(changes[0].sync_data()));
- EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, changes[0].change_type());
- changes.clear();
- model()->Remove(original_turl);
-}
-
-TEST_F(TemplateURLServiceSyncTest, SyncKeywordConflictBothAutoreplace) {
- // This tests cases where both the sync and the local TemplateURL are marked
- // safe_for_autoreplace.
-
- // Create a keyword that conflicts, and make it older. SyncChange is added,
- // function returns false.
- string16 original_turl_keyword = ASCIIToUTF16("key1");
- TemplateURL* original_turl = CreateTestTemplateURL(original_turl_keyword,
- "http://key1.com", std::string(), 9000, true);
- model()->Add(original_turl);
- scoped_ptr<TemplateURL> sync_turl(CreateTestTemplateURL(original_turl_keyword,
- "http://new.com", "remote", 8999, true));
- syncer::SyncChangeList changes;
- EXPECT_FALSE(model()->ResolveSyncKeywordConflict(sync_turl.get(),
- original_turl, &changes));
- EXPECT_EQ(original_turl,
- model()->GetTemplateURLForKeyword(original_turl_keyword));
- ASSERT_EQ(1U, changes.size());
- EXPECT_EQ("remote", GetGUID(changes[0].sync_data()));
- EXPECT_EQ(syncer::SyncChange::ACTION_DELETE, changes[0].change_type());
- changes.clear();
- model()->Remove(original_turl);
-
- // Sync is newer. Original TemplateURL is removed from the model. A
- // syncer::SyncChange is added (which in a normal run would be deleted by
- // PruneSyncChanges() when the local GUID doesn't appear in the sync GUID
- // list).
- original_turl = CreateTestTemplateURL(original_turl_keyword,
- "http://key1.com", "local", 9000, true);
- model()->Add(original_turl);
- sync_turl.reset(CreateTestTemplateURL(original_turl_keyword, "http://new.com",
- std::string(), 9001, true));
- EXPECT_TRUE(model()->ResolveSyncKeywordConflict(sync_turl.get(),
- original_turl, &changes));
- EXPECT_EQ(original_turl_keyword, sync_turl->keyword());
- EXPECT_TRUE(model()->GetTemplateURLs().empty());
- ASSERT_EQ(1U, changes.size());
- EXPECT_EQ("local", GetGUID(changes[0].sync_data()));
- EXPECT_EQ(syncer::SyncChange::ACTION_DELETE, changes[0].change_type());
- changes.clear();
-
- // Equal times. Same result as above. Sync left alone, original removed so
- // sync_turl can fit.
- original_turl = CreateTestTemplateURL(original_turl_keyword,
- "http://key1.com", "local2", 9000, true);
- model()->Add(original_turl);
- sync_turl.reset(CreateTestTemplateURL(original_turl_keyword, "http://new.com",
- std::string(), 9000, true));
- EXPECT_TRUE(model()->ResolveSyncKeywordConflict(sync_turl.get(),
- original_turl, &changes));
- EXPECT_EQ(original_turl_keyword, sync_turl->keyword());
- EXPECT_TRUE(model()->GetTemplateURLs().empty());
- ASSERT_EQ(1U, changes.size());
- EXPECT_EQ("local2", GetGUID(changes[0].sync_data()));
- EXPECT_EQ(syncer::SyncChange::ACTION_DELETE, changes[0].change_type());
- changes.clear();
-
- // Sync is newer, but original TemplateURL is created by policy, so it wins.
- // syncer::SyncChange is added, function returns false.
- original_turl = CreateTestTemplateURL(original_turl_keyword,
- "http://key1.com", std::string(), 9000, true, true);
- model()->Add(original_turl);
- sync_turl.reset(CreateTestTemplateURL(original_turl_keyword, "http://new.com",
- "remote2", 9999, true));
- EXPECT_FALSE(model()->ResolveSyncKeywordConflict(sync_turl.get(),
- original_turl, &changes));
- EXPECT_EQ(original_turl,
- model()->GetTemplateURLForKeyword(original_turl_keyword));
- ASSERT_EQ(1U, changes.size());
- EXPECT_EQ("remote2", GetGUID(changes[0].sync_data()));
- EXPECT_EQ(syncer::SyncChange::ACTION_DELETE, changes[0].change_type());
- changes.clear();
- model()->Remove(original_turl);
-}
-
-TEST_F(TemplateURLServiceSyncTest, SyncKeywordConflictOneAutoreplace) {
- // This tests cases where either the sync or the local TemplateURL is marked
- // safe_for_autoreplace, but the other is not. Basically, we run the same
- // tests as in SyncKeywordConflictBothAutoreplace, but mark the keywords so as
- // to reverse the outcome of each test.
-
- // Create a keyword that conflicts, and make it older. Normally the local
- // TemplateURL would win this.
- string16 original_turl_keyword = ASCIIToUTF16("key1");
- TemplateURL* original_turl = CreateTestTemplateURL(original_turl_keyword,
- "http://key1.com", "local", 9000, true);
- model()->Add(original_turl);
- scoped_ptr<TemplateURL> sync_turl(CreateTestTemplateURL(original_turl_keyword,
- "http://new.com", std::string(), 8999));
- syncer::SyncChangeList changes;
- EXPECT_TRUE(model()->ResolveSyncKeywordConflict(sync_turl.get(),
- original_turl, &changes));
- EXPECT_EQ(original_turl_keyword, sync_turl->keyword());
- EXPECT_TRUE(model()->GetTemplateURLs().empty());
- ASSERT_EQ(1U, changes.size());
- EXPECT_EQ("local", GetGUID(changes[0].sync_data()));
- EXPECT_EQ(syncer::SyncChange::ACTION_DELETE, changes[0].change_type());
- changes.clear();
-
- // Sync is newer. Normally the sync TemplateURL would win this.
- original_turl = CreateTestTemplateURL(original_turl_keyword,
- "http://key1.com", std::string(), 9000);
- model()->Add(original_turl);
- sync_turl.reset(CreateTestTemplateURL(original_turl_keyword, "http://new.com",
- "remote", 9001, true));
- EXPECT_FALSE(model()->ResolveSyncKeywordConflict(sync_turl.get(),
- original_turl, &changes));
- EXPECT_EQ(original_turl,
- model()->GetTemplateURLForKeyword(original_turl_keyword));
- ASSERT_EQ(1U, changes.size());
- EXPECT_EQ("remote", GetGUID(changes[0].sync_data()));
- EXPECT_EQ(syncer::SyncChange::ACTION_DELETE, changes[0].change_type());
- changes.clear();
- model()->Remove(original_turl);
-
- // Equal times. Same result as above.
- original_turl = CreateTestTemplateURL(original_turl_keyword,
- "http://key1.com", std::string(), 9000);
- model()->Add(original_turl);
- sync_turl.reset(CreateTestTemplateURL(original_turl_keyword, "http://new.com",
- "remote2", 9000, true));
- EXPECT_FALSE(model()->ResolveSyncKeywordConflict(sync_turl.get(),
- original_turl, &changes));
- EXPECT_EQ(original_turl,
- model()->GetTemplateURLForKeyword(original_turl_keyword));
- ASSERT_EQ(1U, changes.size());
- EXPECT_EQ("remote2", GetGUID(changes[0].sync_data()));
- EXPECT_EQ(syncer::SyncChange::ACTION_DELETE, changes[0].change_type());
- changes.clear();
- model()->Remove(original_turl);
-
- // We don't run the "created by policy" test since URLs created by policy are
- // never safe_for_autoreplace.
-}
-
-TEST_F(TemplateURLServiceSyncTest, FindDuplicateOfSyncTemplateURL) {
- TemplateURL* original_turl =
- CreateTestTemplateURL(ASCIIToUTF16("key1"), "http://key1.com");
- model()->Add(original_turl);
-
- // No matches at all.
- scoped_ptr<TemplateURL> sync_turl(CreateTestTemplateURL(ASCIIToUTF16("key2"),
- "http://key2.com"));
- EXPECT_EQ(NULL, model()->FindDuplicateOfSyncTemplateURL(*sync_turl));
-
- // URL matches, but not keyword. No dupe.
- sync_turl.reset(CreateTestTemplateURL(ASCIIToUTF16("key2"),
- "http://key1.com"));
- EXPECT_EQ(NULL, model()->FindDuplicateOfSyncTemplateURL(*sync_turl));
-
- // Keyword matches, but not URL. No dupe.
- sync_turl.reset(CreateTestTemplateURL(ASCIIToUTF16("key1"),
- "http://key2.com"));
- EXPECT_EQ(NULL, model()->FindDuplicateOfSyncTemplateURL(*sync_turl));
-
- // Duplicate.
- sync_turl.reset(CreateTestTemplateURL(ASCIIToUTF16("key1"),
- "http://key1.com"));
- const TemplateURL* dupe_turl =
- model()->FindDuplicateOfSyncTemplateURL(*sync_turl);
- ASSERT_TRUE(dupe_turl);
- EXPECT_EQ(dupe_turl->keyword(), sync_turl->keyword());
- EXPECT_EQ(dupe_turl->url(), sync_turl->url());
-}
-
-TEST_F(TemplateURLServiceSyncTest, MergeSyncAndLocalURLDuplicates) {
- TemplateURL* original_turl = CreateTestTemplateURL(ASCIIToUTF16("key1"),
- "http://key1.com", std::string(), 9000);
- model()->Add(original_turl);
- TemplateURL* sync_turl = CreateTestTemplateURL(ASCIIToUTF16("key1"),
- "http://key1.com", std::string(), 9001);
- std::string original_guid = original_turl->sync_guid();
- syncer::SyncChangeList changes;
-
- // The sync TemplateURL is newer. It should replace the original TemplateURL
- // and a syncer::SyncChange should be added to the list.
- // Note that MergeSyncAndLocalURLDuplicates takes ownership of sync_turl.
- model()->MergeSyncAndLocalURLDuplicates(sync_turl, original_turl, &changes);
- TemplateURL* result = model()->GetTemplateURLForKeyword(ASCIIToUTF16("key1"));
- ASSERT_TRUE(result);
- EXPECT_EQ(9001, result->last_modified().ToTimeT());
- EXPECT_EQ(1U, changes.size());
- // We expect a change to delete the local entry.
- syncer::SyncChange change = changes.at(0);
- EXPECT_EQ(syncer::SyncChange::ACTION_DELETE, change.change_type());
- EXPECT_EQ(original_guid,
- change.sync_data().GetSpecifics().search_engine().sync_guid());
- changes.clear();
-
- // The sync TemplateURL is older. The existing TemplateURL should win and a
- // syncer::SyncChange should be added to the list.
- TemplateURL* sync_turl2 = CreateTestTemplateURL(ASCIIToUTF16("key1"),
- "http://key1.com", std::string(), 8999);
- std::string sync_guid = sync_turl2->sync_guid();
- model()->MergeSyncAndLocalURLDuplicates(sync_turl2, sync_turl, &changes);
- result = model()->GetTemplateURLForKeyword(ASCIIToUTF16("key1"));
- ASSERT_TRUE(result);
- EXPECT_EQ(9001, result->last_modified().ToTimeT());
- EXPECT_EQ(1U, changes.size());
- // We expect a change to update the sync entry.
- change = changes.at(0);
- EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, change.change_type());
- EXPECT_EQ(sync_guid,
- change.sync_data().GetSpecifics().search_engine().sync_guid());
-}
-
TEST_F(TemplateURLServiceSyncTest, StartSyncEmpty) {
model()->MergeDataAndStartSyncing(
syncer::SEARCH_ENGINES, syncer::SyncDataList(),
@@ -898,8 +608,10 @@ TEST_F(TemplateURLServiceSyncTest, MergeAddFromOlderSyncData) {
CreateInitialSyncData(), PassProcessor(),
CreateAndPassSyncErrorFactory());
- // The dupe results in a merge. The other two should be added to the model.
- EXPECT_EQ(5U, model()->GetAllSyncData(syncer::SEARCH_ENGINES).size());
+ // The dupe and conflict results in merges, as local values are always merged
+ // with sync values if there is a keyword conflict. The unique keyword should
+ // be added.
+ EXPECT_EQ(4U, model()->GetAllSyncData(syncer::SEARCH_ENGINES).size());
// The key1 duplicate results in the local copy winning. Ensure that Sync's
// copy was not added, and the local copy is pushed upstream to Sync as an
@@ -909,39 +621,39 @@ TEST_F(TemplateURLServiceSyncTest, MergeAddFromOlderSyncData) {
ASSERT_TRUE(processor()->contains_guid("key1"));
syncer::SyncChange key1_change = processor()->change_for_guid("key1");
EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, key1_change.change_type());
+ // The local sync_guid should no longer be found.
EXPECT_FALSE(model()->GetTemplateURLForGUID("aaa"));
- // The key2 keyword conflict results in the local copy winning, so ensure it
- // retains the original keyword, and that an update to the sync copy is pushed
- // upstream to Sync. Both TemplateURLs should be found locally, however.
- const TemplateURL* key2 = model()->GetTemplateURLForGUID("bbb");
- EXPECT_TRUE(key2);
+ // The key2 keyword conflict results in a merge, with the values of the local
+ // copy winning, so ensure it retains the original URL, and that an update to
+ // the sync guid is pushed upstream to Sync.
+ const TemplateURL* key2 = model()->GetTemplateURLForGUID("key2");
+ ASSERT_TRUE(key2);
EXPECT_EQ(ASCIIToUTF16("key2"), key2->keyword());
- EXPECT_TRUE(model()->GetTemplateURLForGUID("key2"));
// Check changes for the UPDATE.
ASSERT_TRUE(processor()->contains_guid("key2"));
syncer::SyncChange key2_change = processor()->change_for_guid("key2");
EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, key2_change.change_type());
- EXPECT_EQ("key2.com", GetKeyword(key2_change.sync_data()));
+ EXPECT_EQ("key2", GetKeyword(key2_change.sync_data()));
+ EXPECT_EQ("http://expected.com", GetURL(key2_change.sync_data()));
+ // The local sync_guid should no longer be found.
+ EXPECT_FALSE(model()->GetTemplateURLForGUID("bbb"));
// The last TemplateURL should have had no conflicts and was just added. It
// should not have replaced the third local TemplateURL.
EXPECT_TRUE(model()->GetTemplateURLForGUID("ccc"));
EXPECT_TRUE(model()->GetTemplateURLForGUID("key3"));
- // Two UPDATEs and two ADDs.
- EXPECT_EQ(4U, processor()->change_list_size());
- // Two ADDs should be pushed up to Sync.
- ASSERT_TRUE(processor()->contains_guid("bbb"));
- EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
- processor()->change_for_guid("bbb").change_type());
+ // Two UPDATEs and one ADD.
+ EXPECT_EQ(3U, processor()->change_list_size());
+ // One ADDs should be pushed up to Sync.
ASSERT_TRUE(processor()->contains_guid("ccc"));
EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
processor()->change_for_guid("ccc").change_type());
}
TEST_F(TemplateURLServiceSyncTest, MergeAddFromNewerSyncData) {
- // GUIDs all differ, so this is data to be added from Sync, but the timestamps
+ // GUIDs all differ, so Sync may overtake some entries, but the timestamps
// from Sync are newer. Set up the local data so that one is a dupe, one has a
// conflicting keyword, and the last has no conflicts (a clean ADD).
model()->Add(CreateTestTemplateURL(ASCIIToUTF16("key1"), "http://key1.com",
@@ -957,8 +669,9 @@ TEST_F(TemplateURLServiceSyncTest, MergeAddFromNewerSyncData) {
CreateInitialSyncData(), PassProcessor(),
CreateAndPassSyncErrorFactory());
- // The dupe results in a merge. The other two should be added to the model.
- EXPECT_EQ(5U, model()->GetAllSyncData(syncer::SEARCH_ENGINES).size());
+ // The dupe and keyword conflict results in merges. The unique keyword be
+ // added to the model.
+ EXPECT_EQ(4U, model()->GetAllSyncData(syncer::SEARCH_ENGINES).size());
// The key1 duplicate results in Sync's copy winning. Ensure that Sync's
// copy replaced the local copy.
@@ -966,26 +679,21 @@ TEST_F(TemplateURLServiceSyncTest, MergeAddFromNewerSyncData) {
EXPECT_FALSE(model()->GetTemplateURLForGUID("aaa"));
// The key2 keyword conflict results in Sync's copy winning, so ensure it
- // retains the original keyword. The local copy should get a uniquified
- // keyword. Both TemplateURLs should be found locally.
+ // retains the original keyword and is added. The local copy should be
+ // removed.
const TemplateURL* key2_sync = model()->GetTemplateURLForGUID("key2");
ASSERT_TRUE(key2_sync);
EXPECT_EQ(ASCIIToUTF16("key2"), key2_sync->keyword());
- const TemplateURL* key2_local = model()->GetTemplateURLForGUID("bbb");
- ASSERT_TRUE(key2_local);
- EXPECT_EQ(ASCIIToUTF16("expected.com"), key2_local->keyword());
+ EXPECT_FALSE(model()->GetTemplateURLForGUID("bbb"));
// The last TemplateURL should have had no conflicts and was just added. It
// should not have replaced the third local TemplateURL.
EXPECT_TRUE(model()->GetTemplateURLForGUID("ccc"));
EXPECT_TRUE(model()->GetTemplateURLForGUID("key3"));
- // Two ADDs.
- EXPECT_EQ(2U, processor()->change_list_size());
- // Two ADDs should be pushed up to Sync.
- ASSERT_TRUE(processor()->contains_guid("bbb"));
- EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
- processor()->change_for_guid("bbb").change_type());
+ // One ADD.
+ EXPECT_EQ(1U, processor()->change_list_size());
+ // One ADDs should be pushed up to Sync.
ASSERT_TRUE(processor()->contains_guid("ccc"));
EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
processor()->change_for_guid("ccc").change_type());
@@ -1135,37 +843,6 @@ TEST_F(TemplateURLServiceSyncTest, ProcessChangesWithConflictsLocalWins) {
processor()->change_for_guid("key1").change_type());
}
-TEST_F(TemplateURLServiceSyncTest, RemoveUpdatedURLOnConflict) {
- // Updating a local replaceable URL to have the same keyword as a local
- // non-replaceable URL should result in the former being removed from the
- // model entirely.
- syncer::SyncDataList initial_data;
- scoped_ptr<TemplateURL> turl(CreateTestTemplateURL(ASCIIToUTF16("sync"),
- "http://sync.com", "sync", 100, true));
- initial_data.push_back(
- TemplateURLService::CreateSyncDataFromTemplateURL(*turl));
- model()->MergeDataAndStartSyncing(syncer::SEARCH_ENGINES, initial_data,
- PassProcessor(), CreateAndPassSyncErrorFactory());
-
- TemplateURL* new_turl =
- CreateTestTemplateURL(ASCIIToUTF16("local"), "http://local.com", "local");
- model()->Add(new_turl);
-
- syncer::SyncChangeList changes;
- changes.push_back(CreateTestSyncChange(syncer::SyncChange::ACTION_UPDATE,
- CreateTestTemplateURL(ASCIIToUTF16("local"), "http://sync.com", "sync",
- 110, true)));
- model()->ProcessSyncChanges(FROM_HERE, changes);
-
- EXPECT_EQ(1U, model()->GetTemplateURLs().size());
- EXPECT_EQ("local",
- model()->GetTemplateURLForKeyword(ASCIIToUTF16("local"))->sync_guid());
- EXPECT_EQ(1U, processor()->change_list_size());
- ASSERT_TRUE(processor()->contains_guid("sync"));
- EXPECT_EQ(syncer::SyncChange::ACTION_DELETE,
- processor()->change_for_guid("sync").change_type());
-}
-
TEST_F(TemplateURLServiceSyncTest, ProcessTemplateURLChange) {
// Ensure that ProcessTemplateURLChange is called and pushes the correct
// changes to Sync whenever local changes are made to TemplateURLs.
@@ -1291,8 +968,9 @@ TEST_F(TemplateURLServiceSyncTest, AutogeneratedKeywordConflicts) {
// try to create conflict with ones in the model.
string16 google_keyword(net::StripWWW(ASCIIToUTF16(GURL(
UIThreadSearchTermsData(profile_a()).GoogleBaseURLValue()).host())));
- TemplateURL* google = CreateTestTemplateURL(google_keyword,
- "{google:baseURL}1/search?q={searchTerms}");
+ const std::string local_google_url =
+ "{google:baseURL}1/search?q={searchTerms}";
+ TemplateURL* google = CreateTestTemplateURL(google_keyword, local_google_url);
model()->Add(google);
TemplateURL* other =
CreateTestTemplateURL(ASCIIToUTF16("other.com"), "http://other.com/foo");
@@ -1302,34 +980,45 @@ TEST_F(TemplateURLServiceSyncTest, AutogeneratedKeywordConflicts) {
"{google:baseURL}2/search?q={searchTerms}", "sync1", 50));
initial_data.push_back(
CreateCustomSyncData(*turl, true, turl->url(), turl->sync_guid()));
+ const std::string synced_other_url = "http://other.com/search?q={searchTerms}";
turl.reset(CreateTestTemplateURL(ASCIIToUTF16("sync2"),
- "http://other.com/search?q={searchTerms}", "sync2", 150));
+ synced_other_url, "sync2", 150));
initial_data.push_back(
CreateCustomSyncData(*turl, true, turl->url(), turl->sync_guid()));
+
+ // Before we merge the data, grab the local sync_guids so we can ensure that
+ // they've been replaced.
+ const std::string local_google_guid = google->sync_guid();
+ const std::string local_other_guid = other->sync_guid();
+
model()->MergeDataAndStartSyncing(syncer::SEARCH_ENGINES, initial_data,
PassProcessor(), CreateAndPassSyncErrorFactory());
// In this case, the conflicts should be handled just like any other keyword
// conflicts -- the later-modified TemplateURL is assumed to be authoritative.
- EXPECT_EQ(google_keyword, google->keyword());
- EXPECT_EQ(google_keyword + ASCIIToUTF16("_"),
- model()->GetTemplateURLForGUID("sync1")->keyword());
- EXPECT_EQ(ASCIIToUTF16("other.com_"), other->keyword());
+ // Since the initial TemplateURLs were local only, they should be merged with
+ // the sync TemplateURLs (GUIDs transferred over).
+ EXPECT_FALSE(model()->GetTemplateURLForGUID(local_google_guid));
+ ASSERT_TRUE(model()->GetTemplateURLForGUID("sync1"));
+ EXPECT_EQ(google_keyword, model()->GetTemplateURLForGUID("sync1")->keyword());
+ EXPECT_FALSE(model()->GetTemplateURLForGUID(local_other_guid));
+ ASSERT_TRUE(model()->GetTemplateURLForGUID("sync2"));
EXPECT_EQ(ASCIIToUTF16("other.com"),
model()->GetTemplateURLForGUID("sync2")->keyword());
// Both synced URLs should have associated UPDATEs, since both needed their
- // keywords to be generated (and sync1 needed conflict resolution as well).
- EXPECT_GE(processor()->change_list_size(), 2U);
+ // keywords to be generated.
+ EXPECT_EQ(processor()->change_list_size(), 2U);
ASSERT_TRUE(processor()->contains_guid("sync1"));
syncer::SyncChange sync1_change = processor()->change_for_guid("sync1");
EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, sync1_change.change_type());
- EXPECT_EQ(google_keyword + ASCIIToUTF16("_"),
- UTF8ToUTF16(GetKeyword(sync1_change.sync_data())));
+ EXPECT_EQ(google_keyword, UTF8ToUTF16(GetKeyword(sync1_change.sync_data())));
+ EXPECT_EQ(local_google_url, GetURL(sync1_change.sync_data()));
ASSERT_TRUE(processor()->contains_guid("sync2"));
syncer::SyncChange sync2_change = processor()->change_for_guid("sync2");
EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, sync2_change.change_type());
EXPECT_EQ("other.com", GetKeyword(sync2_change.sync_data()));
+ EXPECT_EQ(synced_other_url, GetURL(sync2_change.sync_data()));
}
TEST_F(TemplateURLServiceSyncTest, TwoAutogeneratedKeywordsUsingGoogleBaseURL) {
@@ -1358,6 +1047,7 @@ TEST_F(TemplateURLServiceSyncTest, TwoAutogeneratedKeywordsUsingGoogleBaseURL) {
TemplateURL* keyword2 = model()->GetTemplateURLForKeyword(google_keyword);
ASSERT_FALSE(keyword2 == NULL);
EXPECT_EQ("key2", keyword2->sync_guid());
+
EXPECT_GE(processor()->change_list_size(), 2U);
ASSERT_TRUE(processor()->contains_guid("key1"));
syncer::SyncChange key1_change = processor()->change_for_guid("key1");
@@ -1851,16 +1541,19 @@ TEST_F(TemplateURLServiceSyncTest, LocalDefaultWinsConflict) {
model()->MergeDataAndStartSyncing(syncer::SEARCH_ENGINES, initial_data,
PassProcessor(), CreateAndPassSyncErrorFactory());
- // The conflicting TemplateURL should be added, but it should have lost
- // conflict resolution against the default.
- EXPECT_EQ(4U, model()->GetAllSyncData(syncer::SEARCH_ENGINES).size());
- const TemplateURL* winner = model()->GetTemplateURLForGUID("whateverguid");
+ // Since the local default was not yet synced, it should be merged with the
+ // conflicting TemplateURL. However, it's values should have been preserved
+ // since it would have won conflict resolution due to being the default.
+ EXPECT_EQ(3U, model()->GetAllSyncData(syncer::SEARCH_ENGINES).size());
+ const TemplateURL* winner = model()->GetTemplateURLForGUID("key1");
ASSERT_TRUE(winner);
EXPECT_EQ(model()->GetDefaultSearchProvider(), winner);
EXPECT_EQ(keyword, winner->keyword());
- const TemplateURL* loser = model()->GetTemplateURLForGUID("key1");
- ASSERT_TRUE(loser);
- EXPECT_EQ(ASCIIToUTF16("key1.com"), loser->keyword());
+
+ // There is no loser, as the two were merged together. The local sync_guid
+ // should no longer be found in the model.
+ const TemplateURL* loser = model()->GetTemplateURLForGUID("whateverguid");
+ ASSERT_FALSE(loser);
}
TEST_F(TemplateURLServiceSyncTest, DeleteBogusData) {
« chrome/browser/search_engines/template_url_service.cc ('K') | « chrome/browser/search_engines/template_url_service.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698