Commit 14b26d20 authored by stevet@chromium.org's avatar stevet@chromium.org

Rewrite TemplateURLService's SyncableService implmentation to avoid sending ACTION_DELETEs to Sync.

Update unit tests to reflect these changes.

BUG=140732
TEST=

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@151391 0039d316-1c4b-4281-b951-d872f2087c98
parent 5b729260
...@@ -194,8 +194,15 @@ std::string OldBaseURLSearchTermsData::GoogleBaseURLValue() const { ...@@ -194,8 +194,15 @@ std::string OldBaseURLSearchTermsData::GoogleBaseURLValue() const {
return old_base_url; return old_base_url;
} }
} // namespace // Returns true if |turl|'s GUID is not found inside |sync_data|. This is to be
// used in MergeDataAndStartSyncing to differentiate between TemplateURLs from
// Sync and TemplateURLs that were initially local, assuming |sync_data| is the
// |initial_sync_data| parameter.
bool IsFromSync(const TemplateURL* turl, const SyncDataMap& sync_data) {
return (sync_data.find(turl->sync_guid()) != sync_data.end());
}
} // namespace
class TemplateURLService::LessWithPrefix { class TemplateURLService::LessWithPrefix {
public: public:
...@@ -1012,9 +1019,12 @@ syncer::SyncError TemplateURLService::ProcessSyncChanges( ...@@ -1012,9 +1019,12 @@ syncer::SyncError TemplateURLService::ProcessSyncChanges(
LOG(ERROR) << "Trying to add an existing TemplateURL."; LOG(ERROR) << "Trying to add an existing TemplateURL.";
continue; continue;
} }
std::string guid = turl->sync_guid(); const std::string guid = turl->sync_guid();
if (!existing_keyword_turl || ResolveSyncKeywordConflict(turl.get(), if (existing_keyword_turl) {
existing_keyword_turl, &new_changes)) { // Resolve any conflicts so we can safely add the new entry.
ResolveSyncKeywordConflict(turl.get(), existing_keyword_turl,
&new_changes);
}
// Force the local ID to kInvalidTemplateURLID so we can add it. // Force the local ID to kInvalidTemplateURLID so we can add it.
TemplateURLData data(turl->data()); TemplateURLData data(turl->data());
data.id = kInvalidTemplateURLID; data.id = kInvalidTemplateURLID;
...@@ -1022,7 +1032,6 @@ syncer::SyncError TemplateURLService::ProcessSyncChanges( ...@@ -1022,7 +1032,6 @@ syncer::SyncError TemplateURLService::ProcessSyncChanges(
// Possibly set the newly added |turl| as the default search provider. // Possibly set the newly added |turl| as the default search provider.
SetDefaultSearchProviderIfNewlySynced(guid); SetDefaultSearchProviderIfNewlySynced(guid);
}
} else if (iter->change_type() == syncer::SyncChange::ACTION_UPDATE) { } else if (iter->change_type() == syncer::SyncChange::ACTION_UPDATE) {
if (!existing_turl) { if (!existing_turl) {
NOTREACHED() << "Unexpected sync change state."; NOTREACHED() << "Unexpected sync change state.";
...@@ -1032,16 +1041,11 @@ syncer::SyncError TemplateURLService::ProcessSyncChanges( ...@@ -1032,16 +1041,11 @@ syncer::SyncError TemplateURLService::ProcessSyncChanges(
LOG(ERROR) << "Trying to update a non-existent TemplateURL."; LOG(ERROR) << "Trying to update a non-existent TemplateURL.";
continue; continue;
} }
// Possibly resolve a keyword conflict if they have the same keywords but if (existing_keyword_turl && (existing_keyword_turl != existing_turl)) {
// are not the same entry. // Resolve any conflicts with other entries so we can safely update the
if (existing_keyword_turl && (existing_keyword_turl != existing_turl) && // keyword.
!ResolveSyncKeywordConflict(turl.get(), existing_keyword_turl, ResolveSyncKeywordConflict(turl.get(), existing_keyword_turl,
&new_changes)) { &new_changes);
// Note that because we're processing changes, this Remove() call won't
// generate an ACTION_DELETE; but ResolveSyncKeywordConflict() did
// already, so we should be OK.
Remove(existing_turl);
continue;
} }
UIThreadSearchTermsData search_terms_data(existing_turl->profile()); UIThreadSearchTermsData search_terms_data(existing_turl->profile());
if (UpdateNoNotify(existing_turl, *turl, search_terms_data)) if (UpdateNoNotify(existing_turl, *turl, search_terms_data))
...@@ -1129,6 +1133,7 @@ syncer::SyncError TemplateURLService::MergeDataAndStartSyncing( ...@@ -1129,6 +1133,7 @@ syncer::SyncError TemplateURLService::MergeDataAndStartSyncing(
} }
if (local_turl) { if (local_turl) {
DCHECK(IsFromSync(local_turl, sync_data_map));
// This local search engine is already synced. If the timestamp differs // This local search engine is already synced. If the timestamp differs
// from Sync, we need to update locally or to the cloud. Note that if the // from Sync, we need to update locally or to the cloud. Note that if the
// timestamps are equal, we touch neither. // timestamps are equal, we touch neither.
...@@ -1151,36 +1156,12 @@ syncer::SyncError TemplateURLService::MergeDataAndStartSyncing( ...@@ -1151,36 +1156,12 @@ syncer::SyncError TemplateURLService::MergeDataAndStartSyncing(
} }
local_data_map.erase(iter->first); local_data_map.erase(iter->first);
} else { } else {
// The search engine from the cloud has not been synced locally, but there // The search engine from the cloud has not been synced locally. Merge it
// might be a local search engine that is a duplicate that needs to be // into our local model. This will handle any conflicts with local (and
// merged. // already-synced) TemplateURLs. It will prefer to keep entries from Sync
TemplateURL* dupe_turl = FindDuplicateOfSyncTemplateURL(*sync_turl); // over not-yet-synced TemplateURLs.
if (dupe_turl) { MergeInSyncTemplateURL(sync_turl.get(), sync_data_map, &new_changes,
// Merge duplicates and remove the processed local TURL from the map. &local_data_map);
std::string old_guid = dupe_turl->sync_guid();
MergeSyncAndLocalURLDuplicates(sync_turl.release(), dupe_turl,
&new_changes);
local_data_map.erase(old_guid);
} else {
std::string guid = sync_turl->sync_guid();
// Keyword conflict is possible in this case. Resolve it first before
// adding the new TemplateURL. Note that we don't remove the local TURL
// from local_data_map in this case as it may still need to be pushed to
// the cloud. We also explicitly don't resolve conflicts against
// extension keywords; see comments in ProcessSyncChanges().
TemplateURL* existing_keyword_turl =
FindNonExtensionTemplateURLForKeyword(sync_turl->keyword());
if (!existing_keyword_turl || ResolveSyncKeywordConflict(
sync_turl.get(), existing_keyword_turl, &new_changes)) {
// Force the local ID to kInvalidTemplateURLID so we can add it.
TemplateURLData data(sync_turl->data());
data.id = kInvalidTemplateURLID;
Add(new TemplateURL(profile_, data));
// Possibly set the newly added |turl| as the default search provider.
SetDefaultSearchProviderIfNewlySynced(guid);
}
}
} }
} }
...@@ -2324,125 +2305,131 @@ string16 TemplateURLService::UniquifyKeyword(const TemplateURL& turl, ...@@ -2324,125 +2305,131 @@ string16 TemplateURLService::UniquifyKeyword(const TemplateURL& turl,
return keyword_candidate; return keyword_candidate;
} }
bool TemplateURLService::ResolveSyncKeywordConflict( bool TemplateURLService::IsLocalTemplateURLBetter(
TemplateURL* sync_turl, const TemplateURL* local_turl,
TemplateURL* local_turl, const TemplateURL* sync_turl) {
DCHECK(GetTemplateURLForGUID(local_turl->sync_guid()));
return local_turl->last_modified() > sync_turl->last_modified() ||
local_turl->created_by_policy() ||
local_turl== GetDefaultSearchProvider();
}
void TemplateURLService::ResolveSyncKeywordConflict(
TemplateURL* unapplied_sync_turl,
TemplateURL* applied_sync_turl,
syncer::SyncChangeList* change_list) { syncer::SyncChangeList* change_list) {
DCHECK(loaded_); DCHECK(loaded_);
DCHECK(sync_turl); DCHECK(unapplied_sync_turl);
DCHECK(local_turl); DCHECK(applied_sync_turl);
DCHECK(sync_turl->sync_guid() != local_turl->sync_guid());
DCHECK(!local_turl->IsExtensionKeyword());
DCHECK(change_list); DCHECK(change_list);
DCHECK_EQ(applied_sync_turl->keyword(), unapplied_sync_turl->keyword());
const bool local_is_better = DCHECK(!applied_sync_turl->IsExtensionKeyword());
(local_turl->last_modified() > sync_turl->last_modified()) ||
local_turl->created_by_policy() || // Both |unapplied_sync_turl| and |applied_sync_turl| are known to Sync, so
(local_turl == GetDefaultSearchProvider()); // don't delete either of them. Instead, determine which is "better" and
const bool can_replace_local = CanReplace(local_turl); // uniquify the other one, sending an update to the server for the updated
if (CanReplace(sync_turl) && (local_is_better || !can_replace_local)) { // entry.
syncer::SyncData sync_data = CreateSyncDataFromTemplateURL(*sync_turl); const bool applied_turl_is_better =
change_list->push_back(syncer::SyncChange(FROM_HERE, IsLocalTemplateURLBetter(applied_sync_turl, unapplied_sync_turl);
syncer::SyncChange::ACTION_DELETE, TemplateURL* loser = applied_turl_is_better ?
sync_data)); unapplied_sync_turl : applied_sync_turl;
return false; string16 new_keyword = UniquifyKeyword(*loser, false);
}
if (can_replace_local) {
// Since we're processing sync changes, the upcoming Remove() won't generate
// an ACTION_DELETE. We need to do it manually to keep the server in sync
// with us. Note that if we're being called from
// MergeDataAndStartSyncing(), and this TemplateURL was pre-existing rather
// than having just been brought down, then this is wrong, because the
// server doesn't yet know about this entity; but in this case,
// PruneSyncChanges() will prune out the ACTION_DELETE we create here.
syncer::SyncData sync_data = CreateSyncDataFromTemplateURL(*local_turl);
change_list->push_back(syncer::SyncChange(FROM_HERE,
syncer::SyncChange::ACTION_DELETE,
sync_data));
Remove(local_turl);
} else if (local_is_better) {
string16 new_keyword = UniquifyKeyword(*sync_turl, false);
DCHECK(!GetTemplateURLForKeyword(new_keyword)); DCHECK(!GetTemplateURLForKeyword(new_keyword));
sync_turl->data_.SetKeyword(new_keyword); if (applied_turl_is_better) {
// If we update the cloud TURL, we need to push an update back to sync // Just set the keyword of |unapplied_sync_turl|. The caller is responsible
// informing it that something has changed. // for adding or updating unapplied_sync_turl in the local model.
syncer::SyncData sync_data = CreateSyncDataFromTemplateURL(*sync_turl); unapplied_sync_turl->data_.SetKeyword(new_keyword);
change_list->push_back(syncer::SyncChange(FROM_HERE,
syncer::SyncChange::ACTION_UPDATE,
sync_data));
} else { } else {
string16 new_keyword = UniquifyKeyword(*local_turl, false); // Update |applied_sync_turl| in the local model with the new keyword.
TemplateURLData data(local_turl->data()); TemplateURLData data(applied_sync_turl->data());
data.SetKeyword(new_keyword); data.SetKeyword(new_keyword);
TemplateURL new_turl(local_turl->profile(), data); TemplateURL new_turl(applied_sync_turl->profile(), data);
UIThreadSearchTermsData search_terms_data(local_turl->profile()); UIThreadSearchTermsData search_terms_data(applied_sync_turl->profile());
if (UpdateNoNotify(local_turl, new_turl, search_terms_data)) if (UpdateNoNotify(applied_sync_turl, new_turl, search_terms_data))
NotifyObservers(); NotifyObservers();
// Since we're processing sync changes, the UpdateNoNotify() above didn't }
// generate an ACTION_UPDATE. We need to do it manually to keep the server // The losing TemplateURL should have their keyword updated. Send a change to
// in sync with us. Note that if we're being called from // the server to reflect this change.
// MergeDataAndStartSyncing(), and this TemplateURL was pre-existing rather syncer::SyncData sync_data = CreateSyncDataFromTemplateURL(*loser);
// than having just been brought down, then this is wrong, because the
// server won't know about this entity until it processes the ACTION_ADD our
// caller will later generate; but in this case, PruneSyncChanges() will
// prune out the ACTION_UPDATE we create here.
syncer::SyncData sync_data = CreateSyncDataFromTemplateURL(*local_turl);
change_list->push_back(syncer::SyncChange(FROM_HERE, change_list->push_back(syncer::SyncChange(FROM_HERE,
syncer::SyncChange::ACTION_UPDATE, syncer::SyncChange::ACTION_UPDATE,
sync_data)); sync_data));
}
return true;
} }
TemplateURL* TemplateURLService::FindDuplicateOfSyncTemplateURL( void TemplateURLService::MergeInSyncTemplateURL(
const TemplateURL& sync_turl) {
TemplateURL* existing_turl = GetTemplateURLForKeyword(sync_turl.keyword());
return existing_turl && (existing_turl->url() == sync_turl.url()) ?
existing_turl : NULL;
}
void TemplateURLService::MergeSyncAndLocalURLDuplicates(
TemplateURL* sync_turl, TemplateURL* sync_turl,
TemplateURL* local_turl, const SyncDataMap& sync_data,
syncer::SyncChangeList* change_list) { syncer::SyncChangeList* change_list,
DCHECK(loaded_); SyncDataMap* local_data) {
DCHECK(sync_turl); DCHECK(sync_turl);
DCHECK(local_turl); DCHECK(!GetTemplateURLForGUID(sync_turl->sync_guid()));
DCHECK(change_list); DCHECK(IsFromSync(sync_turl, sync_data));
scoped_ptr<TemplateURL> scoped_sync_turl(sync_turl);
if (sync_turl->last_modified() > local_turl->last_modified()) {
// Fully replace local_url with Sync's copy. Note that because use Add
// rather than ResetTemplateURL, |sync_url| is added with a fresh
// TemplateURLID. We don't need to sync the new ID back to the server since
// it's only relevant locally.
bool delete_default = (local_turl == GetDefaultSearchProvider());
DCHECK(!delete_default || !is_default_search_managed_);
if (delete_default)
default_search_provider_ = NULL;
// See comments in ResolveSyncKeywordConflict() regarding generating an TemplateURL* conflicting_turl =
// ACTION_DELETE manually since Remove() won't do it. FindNonExtensionTemplateURLForKeyword(sync_turl->keyword());
syncer::SyncData sync_data = CreateSyncDataFromTemplateURL(*local_turl); bool should_add_sync_turl = true;
change_list->push_back(syncer::SyncChange(FROM_HERE,
syncer::SyncChange::ACTION_DELETE, // If there was no TemplateURL in the local model that conflicts with
sync_data)); // |sync_turl|, skip the following preparation steps and just add |sync_turl|
Remove(local_turl); // directly. Otherwise, modify |conflicting_turl| to make room for
// |sync_turl|.
if (conflicting_turl) {
DCHECK(conflicting_turl);
if (IsFromSync(conflicting_turl, sync_data)) {
// |conflicting_turl| is already known to Sync, so we're not allowed to
// remove it. In this case, we want to uniquify the worse one and send an
// update for the changed keyword to sync. We can reuse the logic from
// ResolveSyncKeywordConflict for this.
ResolveSyncKeywordConflict(sync_turl, conflicting_turl, change_list);
} else {
// |conflicting_turl| is not yet known to Sync. If it is better, then we
// want to transfer its values up to sync. Otherwise, we remove it and
// allow the entry from Sync to overtake it in the model.
const std::string guid = conflicting_turl->sync_guid();
if (IsLocalTemplateURLBetter(conflicting_turl, sync_turl)) {
ResetTemplateURLGUID(conflicting_turl, sync_turl->sync_guid());
syncer::SyncData sync_data =
CreateSyncDataFromTemplateURL(*conflicting_turl);
change_list->push_back(syncer::SyncChange(
FROM_HERE, syncer::SyncChange::ACTION_UPDATE, sync_data));
if (conflicting_turl == GetDefaultSearchProvider() &&
!pending_synced_default_search_) {
// If we're not waiting for the Synced default to come in, we should
// override the pref with our new GUID. If we are waiting for the
// arrival of a synced default, setting the pref here would cause us
// to lose the GUID we are waiting on.
PrefService* prefs = GetPrefs();
if (prefs) {
prefs->SetString(prefs::kSyncedDefaultSearchProviderGUID,
conflicting_turl->sync_guid());
}
}
// Note that in this case we do not add the Sync TemplateURL to the
// local model, since we've effectively "merged" it in by updating the
// local conflicting entry with its sync_guid.
should_add_sync_turl = false;
} else {
// We guarantee that this isn't the local search provider. Otherwise,
// local would have won.
DCHECK(conflicting_turl != GetDefaultSearchProvider());
Remove(conflicting_turl);
}
// This TemplateURL was either removed or overwritten in the local model.
// Remove the entry from the local data so it isn't pushed up to Sync.
local_data->erase(guid);
}
}
if (should_add_sync_turl) {
const std::string guid = sync_turl->sync_guid();
// Force the local ID to kInvalidTemplateURLID so we can add it. // Force the local ID to kInvalidTemplateURLID so we can add it.
sync_turl->data_.id = kInvalidTemplateURLID; TemplateURLData data(sync_turl->data());
Add(scoped_sync_turl.release()); data.id = kInvalidTemplateURLID;
if (delete_default) Add(new TemplateURL(profile_, data));
SetDefaultSearchProvider(sync_turl);
} else { // Possibly set the newly added |turl| as the default search provider.
// Change the local TURL's GUID to the server's GUID and push an update to SetDefaultSearchProviderIfNewlySynced(guid);
// Sync. This ensures that the rest of local_url's fields are sync'd up to
// the server, and the next time local_url is synced, it is recognized by
// having the same GUID.
ResetTemplateURLGUID(local_turl, sync_turl->sync_guid());
syncer::SyncData sync_data = CreateSyncDataFromTemplateURL(*local_turl);
change_list->push_back(syncer::SyncChange(FROM_HERE,
syncer::SyncChange::ACTION_UPDATE,
sync_data));
} }
} }
......
...@@ -352,17 +352,11 @@ class TemplateURLService : public WebDataServiceConsumer, ...@@ -352,17 +352,11 @@ class TemplateURLService : public WebDataServiceConsumer,
CreateTemplateURLFromSyncData); CreateTemplateURLFromSyncData);
FRIEND_TEST_ALL_PREFIXES(TemplateURLServiceSyncTest, UniquifyKeyword); FRIEND_TEST_ALL_PREFIXES(TemplateURLServiceSyncTest, UniquifyKeyword);
FRIEND_TEST_ALL_PREFIXES(TemplateURLServiceSyncTest, FRIEND_TEST_ALL_PREFIXES(TemplateURLServiceSyncTest,
SyncKeywordConflictNeitherAutoreplace); ResolveSyncKeywordConflict);
FRIEND_TEST_ALL_PREFIXES(TemplateURLServiceSyncTest, PreSyncDeletes);
FRIEND_TEST_ALL_PREFIXES(TemplateURLServiceSyncTest, FRIEND_TEST_ALL_PREFIXES(TemplateURLServiceSyncTest,
SyncKeywordConflictBothAutoreplace); IsLocalTemplateURLBetter);
FRIEND_TEST_ALL_PREFIXES(TemplateURLServiceSyncTest, FRIEND_TEST_ALL_PREFIXES(TemplateURLServiceSyncTest, MergeInSyncTemplateURL);
SyncKeywordConflictOneAutoreplace);
FRIEND_TEST_ALL_PREFIXES(TemplateURLServiceSyncTest,
FindDuplicateOfSyncTemplateURL);
FRIEND_TEST_ALL_PREFIXES(TemplateURLServiceSyncTest,
MergeSyncAndLocalURLDuplicates);
FRIEND_TEST_ALL_PREFIXES(TemplateURLServiceSyncTest,
PreSyncDeletes);
friend class TemplateURLServiceTestUtil; friend class TemplateURLServiceTestUtil;
...@@ -532,49 +526,46 @@ class TemplateURLService : public WebDataServiceConsumer, ...@@ -532,49 +526,46 @@ class TemplateURLService : public WebDataServiceConsumer,
// execute the special character appending functionality. // execute the special character appending functionality.
string16 UniquifyKeyword(const TemplateURL& turl, bool force); string16 UniquifyKeyword(const TemplateURL& turl, bool force);
// Given a TemplateURL from Sync (cloud) and a local, non-extension // Returns true iff |local_turl| is considered "better" than |sync_turl| for
// TemplateURL with the same keyword, selects "better" and "worse" entries: // the purposes of resolving conflicts. |local_turl| must be a TemplateURL
// * If one of the TemplateURLs is replaceable and the other is not, the // known to the local model (though it may already be synced), and |sync_turl|
// non-replaceable entry is better. // is a new TemplateURL known to Sync but not yet known to the local model.
// * Otherwise, if |local_turl| was created by policy, is the default // The criteria for if |local_turl| is better than |sync_turl| is whether any
// provider, or was modified more recently, it is better. // of the following are true:
// * Otherwise |sync_turl| is better. // * |local_turl|'s last_modified timestamp is newer than sync_turl.
// Then resolves the conflict: // * |local_turl| is created by policy.
// * If the "worse" entry is |sync_turl|, and it is replaceable, add a // * |local_turl| is the local default search provider.
// syncer::SyncChange to delete it, and return false. bool IsLocalTemplateURLBetter(const TemplateURL* local_turl,
// * If the "worse" entry is |local_turl|, and it is replaceable, remove it const TemplateURL* sync_turl);
// from the service and return true.
// * Otherwise, uniquify the keyword of the "worse" entry. If it is // Given two synced TemplateURLs with a conflicting keyword, one of which
// |local_turl|, update it within the service. Add a SyncChange to update // needs to be added to or updated in the local model (|unapplied_sync_turl|)
// things (always for |sync_turl|, sometimes for |local_turl|; see // and one which is already known to the local model (|applied_sync_turl|),
// comments in implementation), and return true. // prepares the local model so that |unapplied_sync_turl| can be added to it,
// When the function returns true, callers can then go ahead and add or update // or applied as an update to an existing TemplateURL.
// |sync_turl| within the service. If it returns false, callers must not add // Since both entries are known to Sync and one of their keywords will change,
// the |sync_turl|, and must Remove() the |sync_turl| if it was being updated. // an ACTION_UPDATE will be appended to |change_list| to reflect this change.
// (Be careful; calling Remove() could add an ACTION_DELETE sync change, which // Note that |applied_sync_turl| must not be an extension keyword.
// this function has already done. Make sure to avoid duplicates.) void ResolveSyncKeywordConflict(TemplateURL* unapplied_sync_turl,
// TemplateURL* applied_sync_turl,
// Note that we never call this for conflicts with extension keywords because
// other code (e.g. AddToMaps()) is responsible for correctly prioritizing
// extension- vs. non-extension-based TemplateURLs with the same keyword.
bool ResolveSyncKeywordConflict(TemplateURL* sync_turl,
TemplateURL* local_turl,
syncer::SyncChangeList* change_list); syncer::SyncChangeList* change_list);
// Returns a TemplateURL from the service that has the same keyword and search // Adds |sync_turl| into the local model, possibly removing or updating a
// URL as |sync_turl|, if it exists. // local TemplateURL to make room for it. This expects |sync_turl| to be a new
TemplateURL* FindDuplicateOfSyncTemplateURL(const TemplateURL& sync_turl); // entry from Sync, not currently known to the local model. |sync_data| should
// be a SyncDataMap where the contents are entries initially known to Sync
// Given a TemplateURL from the cloud and a local matching duplicate found by // during MergeDataAndStartSyncing.
// FindDuplicateOfSyncTemplateURL, merges the two. If |sync_turl| is newer, // Any necessary updates to Sync will be appended to |change_list|. This can
// this replaces |local_turl| with |sync_turl| using the service's Remove and // include updates on local TemplateURLs, if they are found in |sync_data|.
// Add. If |local_turl| is newer, this replaces |sync_turl| with |local_turl| // |initial_data| should be a SyncDataMap of the entries known to the local
// through through adding appropriate SyncChanges to |change_list|. This // model during MergeDataAndStartSyncing. If |sync_turl| replaces a local
// method takes ownership of |sync_turl|, and adds it to the model if it is // entry, that entry is removed from |initial_data| to prevent it from being
// newer, so the caller must release it if need be. // sent up to Sync.
void MergeSyncAndLocalURLDuplicates(TemplateURL* sync_turl, // This should only be called from MergeDataAndStartSyncing.
TemplateURL* local_turl, void MergeInSyncTemplateURL(TemplateURL* sync_turl,
syncer::SyncChangeList* change_list); const SyncDataMap& sync_data,
syncer::SyncChangeList* change_list,
SyncDataMap* local_data);
// Checks a newly added TemplateURL from Sync by its sync_guid and sets it as // Checks a newly added TemplateURL from Sync by its sync_guid and sets it as
// the default search provider if we were waiting for it. // the default search provider if we were waiting for it.
......
...@@ -471,7 +471,45 @@ TEST_F(TemplateURLServiceSyncTest, UniquifyKeyword) { ...@@ -471,7 +471,45 @@ TEST_F(TemplateURLServiceSyncTest, UniquifyKeyword) {
EXPECT_EQ(NULL, model()->GetTemplateURLForKeyword(new_keyword)); EXPECT_EQ(NULL, model()->GetTemplateURLForKeyword(new_keyword));
} }
TEST_F(TemplateURLServiceSyncTest, SyncKeywordConflictNeitherAutoreplace) { TEST_F(TemplateURLServiceSyncTest, IsLocalTemplateURLBetter) {
// Test some edge cases of this function.
const struct {
time_t local_time;
time_t sync_time;
bool local_is_default;
bool local_created_by_policy;
bool expected_result;
} test_cases[] = {
// Sync is better by timestamp but local is Default.
{10, 100, true, false, true},
// Sync is better by timestamp but local is Create by Policy.
{10, 100, false, true, true},
// Tie. Sync wins.
{100, 100, false, false, false},
};
for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_cases); ++i) {
TemplateURL* local_turl = CreateTestTemplateURL(
ASCIIToUTF16("localkey"), "www.local.com", "localguid",
test_cases[i].local_time, true, test_cases[i].local_created_by_policy);
model()->Add(local_turl);
if (test_cases[i].local_is_default)
model()->SetDefaultSearchProvider(local_turl);
scoped_ptr<TemplateURL> sync_turl(CreateTestTemplateURL(
ASCIIToUTF16("synckey"), "www.sync.com", "syncguid",
test_cases[i].sync_time));
EXPECT_EQ(test_cases[i].expected_result,
model()->IsLocalTemplateURLBetter(local_turl, sync_turl.get()));
// Undo the changes.
if (test_cases[i].local_is_default)
model()->SetDefaultSearchProvider(NULL);
model()->Remove(local_turl);
}
}
TEST_F(TemplateURLServiceSyncTest, ResolveSyncKeywordConflict) {
// This tests cases where neither the sync nor the local TemplateURL are // This tests cases where neither the sync nor the local TemplateURL are
// marked safe_for_autoreplace. // marked safe_for_autoreplace.
...@@ -484,8 +522,7 @@ TEST_F(TemplateURLServiceSyncTest, SyncKeywordConflictNeitherAutoreplace) { ...@@ -484,8 +522,7 @@ TEST_F(TemplateURLServiceSyncTest, SyncKeywordConflictNeitherAutoreplace) {
scoped_ptr<TemplateURL> sync_turl(CreateTestTemplateURL(original_turl_keyword, scoped_ptr<TemplateURL> sync_turl(CreateTestTemplateURL(original_turl_keyword,
"http://new.com", "remote", 8999)); "http://new.com", "remote", 8999));
syncer::SyncChangeList changes; syncer::SyncChangeList changes;
EXPECT_TRUE(model()->ResolveSyncKeywordConflict(sync_turl.get(), model()->ResolveSyncKeywordConflict(sync_turl.get(), original_turl, &changes);
original_turl, &changes));
EXPECT_NE(original_turl_keyword, sync_turl->keyword()); EXPECT_NE(original_turl_keyword, sync_turl->keyword());
EXPECT_EQ(original_turl_keyword, original_turl->keyword()); EXPECT_EQ(original_turl_keyword, original_turl->keyword());
ASSERT_EQ(1U, changes.size()); ASSERT_EQ(1U, changes.size());
...@@ -505,8 +542,7 @@ TEST_F(TemplateURLServiceSyncTest, SyncKeywordConflictNeitherAutoreplace) { ...@@ -505,8 +542,7 @@ TEST_F(TemplateURLServiceSyncTest, SyncKeywordConflictNeitherAutoreplace) {
TemplateURLID original_id = original_turl->id(); TemplateURLID original_id = original_turl->id();
sync_turl.reset(CreateTestTemplateURL(original_turl_keyword, "http://new.com", sync_turl.reset(CreateTestTemplateURL(original_turl_keyword, "http://new.com",
std::string(), 9001)); std::string(), 9001));
EXPECT_TRUE(model()->ResolveSyncKeywordConflict(sync_turl.get(), model()->ResolveSyncKeywordConflict(sync_turl.get(), original_turl, &changes);
original_turl, &changes));
EXPECT_EQ(original_turl_keyword, sync_turl->keyword()); EXPECT_EQ(original_turl_keyword, sync_turl->keyword());
EXPECT_NE(original_turl_keyword, original_turl->keyword()); EXPECT_NE(original_turl_keyword, original_turl->keyword());
EXPECT_FALSE(original_turl->safe_for_autoreplace()); EXPECT_FALSE(original_turl->safe_for_autoreplace());
...@@ -525,8 +561,7 @@ TEST_F(TemplateURLServiceSyncTest, SyncKeywordConflictNeitherAutoreplace) { ...@@ -525,8 +561,7 @@ TEST_F(TemplateURLServiceSyncTest, SyncKeywordConflictNeitherAutoreplace) {
model()->Add(original_turl); model()->Add(original_turl);
sync_turl.reset(CreateTestTemplateURL(original_turl_keyword, "http://new.com", sync_turl.reset(CreateTestTemplateURL(original_turl_keyword, "http://new.com",
std::string(), 9000)); std::string(), 9000));
EXPECT_TRUE(model()->ResolveSyncKeywordConflict(sync_turl.get(), model()->ResolveSyncKeywordConflict(sync_turl.get(), original_turl, &changes);
original_turl, &changes));
EXPECT_EQ(original_turl_keyword, sync_turl->keyword()); EXPECT_EQ(original_turl_keyword, sync_turl->keyword());
EXPECT_NE(original_turl_keyword, original_turl->keyword()); EXPECT_NE(original_turl_keyword, original_turl->keyword());
EXPECT_EQ(NULL, model()->GetTemplateURLForKeyword(original_turl_keyword)); EXPECT_EQ(NULL, model()->GetTemplateURLForKeyword(original_turl_keyword));
...@@ -543,8 +578,7 @@ TEST_F(TemplateURLServiceSyncTest, SyncKeywordConflictNeitherAutoreplace) { ...@@ -543,8 +578,7 @@ TEST_F(TemplateURLServiceSyncTest, SyncKeywordConflictNeitherAutoreplace) {
model()->Add(original_turl); model()->Add(original_turl);
sync_turl.reset(CreateTestTemplateURL(original_turl_keyword, "http://new.com", sync_turl.reset(CreateTestTemplateURL(original_turl_keyword, "http://new.com",
"remote2", 9999)); "remote2", 9999));
EXPECT_TRUE(model()->ResolveSyncKeywordConflict(sync_turl.get(), model()->ResolveSyncKeywordConflict(sync_turl.get(), original_turl, &changes);
original_turl, &changes));
EXPECT_NE(original_turl_keyword, sync_turl->keyword()); EXPECT_NE(original_turl_keyword, sync_turl->keyword());
EXPECT_EQ(original_turl_keyword, original_turl->keyword()); EXPECT_EQ(original_turl_keyword, original_turl->keyword());
EXPECT_EQ(NULL, model()->GetTemplateURLForKeyword(sync_turl->keyword())); EXPECT_EQ(NULL, model()->GetTemplateURLForKeyword(sync_turl->keyword()));
...@@ -555,212 +589,6 @@ TEST_F(TemplateURLServiceSyncTest, SyncKeywordConflictNeitherAutoreplace) { ...@@ -555,212 +589,6 @@ TEST_F(TemplateURLServiceSyncTest, SyncKeywordConflictNeitherAutoreplace) {
model()->Remove(original_turl); 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) { TEST_F(TemplateURLServiceSyncTest, StartSyncEmpty) {
model()->MergeDataAndStartSyncing( model()->MergeDataAndStartSyncing(
syncer::SEARCH_ENGINES, syncer::SyncDataList(), syncer::SEARCH_ENGINES, syncer::SyncDataList(),
...@@ -898,8 +726,10 @@ TEST_F(TemplateURLServiceSyncTest, MergeAddFromOlderSyncData) { ...@@ -898,8 +726,10 @@ TEST_F(TemplateURLServiceSyncTest, MergeAddFromOlderSyncData) {
CreateInitialSyncData(), PassProcessor(), CreateInitialSyncData(), PassProcessor(),
CreateAndPassSyncErrorFactory()); CreateAndPassSyncErrorFactory());
// The dupe results in a merge. The other two should be added to the model. // The dupe and conflict results in merges, as local values are always merged
EXPECT_EQ(5U, model()->GetAllSyncData(syncer::SEARCH_ENGINES).size()); // 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 // 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 // copy was not added, and the local copy is pushed upstream to Sync as an
...@@ -909,39 +739,40 @@ TEST_F(TemplateURLServiceSyncTest, MergeAddFromOlderSyncData) { ...@@ -909,39 +739,40 @@ TEST_F(TemplateURLServiceSyncTest, MergeAddFromOlderSyncData) {
ASSERT_TRUE(processor()->contains_guid("key1")); ASSERT_TRUE(processor()->contains_guid("key1"));
syncer::SyncChange key1_change = processor()->change_for_guid("key1"); syncer::SyncChange key1_change = processor()->change_for_guid("key1");
EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, key1_change.change_type()); EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, key1_change.change_type());
// The local sync_guid should no longer be found.
EXPECT_FALSE(model()->GetTemplateURLForGUID("aaa")); EXPECT_FALSE(model()->GetTemplateURLForGUID("aaa"));
// The key2 keyword conflict results in the local copy winning, so ensure it // The key2 keyword conflict results in a merge, with the values of the local
// retains the original keyword, and that an update to the sync copy is pushed // copy winning, so ensure it retains the original URL, and that an update to
// upstream to Sync. Both TemplateURLs should be found locally, however. // the sync guid is pushed upstream to Sync.
const TemplateURL* key2 = model()->GetTemplateURLForGUID("bbb"); const TemplateURL* key2 = model()->GetTemplateURLForGUID("key2");
EXPECT_TRUE(key2); ASSERT_TRUE(key2);
EXPECT_EQ(ASCIIToUTF16("key2"), key2->keyword()); EXPECT_EQ(ASCIIToUTF16("key2"), key2->keyword());
EXPECT_TRUE(model()->GetTemplateURLForGUID("key2")); EXPECT_EQ("http://expected.com", key2->url());
// Check changes for the UPDATE. // Check changes for the UPDATE.
ASSERT_TRUE(processor()->contains_guid("key2")); ASSERT_TRUE(processor()->contains_guid("key2"));
syncer::SyncChange key2_change = processor()->change_for_guid("key2"); syncer::SyncChange key2_change = processor()->change_for_guid("key2");
EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, key2_change.change_type()); 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 // The last TemplateURL should have had no conflicts and was just added. It
// should not have replaced the third local TemplateURL. // should not have replaced the third local TemplateURL.
EXPECT_TRUE(model()->GetTemplateURLForGUID("ccc")); EXPECT_TRUE(model()->GetTemplateURLForGUID("ccc"));
EXPECT_TRUE(model()->GetTemplateURLForGUID("key3")); EXPECT_TRUE(model()->GetTemplateURLForGUID("key3"));
// Two UPDATEs and two ADDs. // Two UPDATEs and one ADD.
EXPECT_EQ(4U, processor()->change_list_size()); EXPECT_EQ(3U, processor()->change_list_size());
// Two ADDs should be pushed up to Sync. // One 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());
ASSERT_TRUE(processor()->contains_guid("ccc")); ASSERT_TRUE(processor()->contains_guid("ccc"));
EXPECT_EQ(syncer::SyncChange::ACTION_ADD, EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
processor()->change_for_guid("ccc").change_type()); processor()->change_for_guid("ccc").change_type());
} }
TEST_F(TemplateURLServiceSyncTest, MergeAddFromNewerSyncData) { 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 // 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). // conflicting keyword, and the last has no conflicts (a clean ADD).
model()->Add(CreateTestTemplateURL(ASCIIToUTF16("key1"), "http://key1.com", model()->Add(CreateTestTemplateURL(ASCIIToUTF16("key1"), "http://key1.com",
...@@ -957,35 +788,33 @@ TEST_F(TemplateURLServiceSyncTest, MergeAddFromNewerSyncData) { ...@@ -957,35 +788,33 @@ TEST_F(TemplateURLServiceSyncTest, MergeAddFromNewerSyncData) {
CreateInitialSyncData(), PassProcessor(), CreateInitialSyncData(), PassProcessor(),
CreateAndPassSyncErrorFactory()); CreateAndPassSyncErrorFactory());
// The dupe results in a merge. The other two should be added to the model. // The dupe and keyword conflict results in merges. The unique keyword be
EXPECT_EQ(5U, model()->GetAllSyncData(syncer::SEARCH_ENGINES).size()); // 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 // The key1 duplicate results in Sync's copy winning. Ensure that Sync's
// copy replaced the local copy. // copy replaced the local copy.
EXPECT_TRUE(model()->GetTemplateURLForGUID("key1")); EXPECT_TRUE(model()->GetTemplateURLForGUID("key1"));
EXPECT_FALSE(model()->GetTemplateURLForGUID("aaa")); EXPECT_FALSE(model()->GetTemplateURLForGUID("aaa"));
EXPECT_FALSE(processor()->contains_guid("key1"));
EXPECT_FALSE(processor()->contains_guid("aaa"));
// The key2 keyword conflict results in Sync's copy winning, so ensure it // The key2 keyword conflict results in Sync's copy winning, so ensure it
// retains the original keyword. The local copy should get a uniquified // retains the original keyword and is added. The local copy should be
// keyword. Both TemplateURLs should be found locally. // removed.
const TemplateURL* key2_sync = model()->GetTemplateURLForGUID("key2"); const TemplateURL* key2_sync = model()->GetTemplateURLForGUID("key2");
ASSERT_TRUE(key2_sync); ASSERT_TRUE(key2_sync);
EXPECT_EQ(ASCIIToUTF16("key2"), key2_sync->keyword()); EXPECT_EQ(ASCIIToUTF16("key2"), key2_sync->keyword());
const TemplateURL* key2_local = model()->GetTemplateURLForGUID("bbb"); EXPECT_FALSE(model()->GetTemplateURLForGUID("bbb"));
ASSERT_TRUE(key2_local);
EXPECT_EQ(ASCIIToUTF16("expected.com"), key2_local->keyword());
// The last TemplateURL should have had no conflicts and was just added. It // The last TemplateURL should have had no conflicts and was just added. It
// should not have replaced the third local TemplateURL. // should not have replaced the third local TemplateURL.
EXPECT_TRUE(model()->GetTemplateURLForGUID("ccc")); EXPECT_TRUE(model()->GetTemplateURLForGUID("ccc"));
EXPECT_TRUE(model()->GetTemplateURLForGUID("key3")); EXPECT_TRUE(model()->GetTemplateURLForGUID("key3"));
// Two ADDs. // One ADD.
EXPECT_EQ(2U, processor()->change_list_size()); EXPECT_EQ(1U, processor()->change_list_size());
// Two ADDs should be pushed up to Sync. // One 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());
ASSERT_TRUE(processor()->contains_guid("ccc")); ASSERT_TRUE(processor()->contains_guid("ccc"));
EXPECT_EQ(syncer::SyncChange::ACTION_ADD, EXPECT_EQ(syncer::SyncChange::ACTION_ADD,
processor()->change_for_guid("ccc").change_type()); processor()->change_for_guid("ccc").change_type());
...@@ -1135,37 +964,6 @@ TEST_F(TemplateURLServiceSyncTest, ProcessChangesWithConflictsLocalWins) { ...@@ -1135,37 +964,6 @@ TEST_F(TemplateURLServiceSyncTest, ProcessChangesWithConflictsLocalWins) {
processor()->change_for_guid("key1").change_type()); 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) { TEST_F(TemplateURLServiceSyncTest, ProcessTemplateURLChange) {
// Ensure that ProcessTemplateURLChange is called and pushes the correct // Ensure that ProcessTemplateURLChange is called and pushes the correct
// changes to Sync whenever local changes are made to TemplateURLs. // changes to Sync whenever local changes are made to TemplateURLs.
...@@ -1291,8 +1089,9 @@ TEST_F(TemplateURLServiceSyncTest, AutogeneratedKeywordConflicts) { ...@@ -1291,8 +1089,9 @@ TEST_F(TemplateURLServiceSyncTest, AutogeneratedKeywordConflicts) {
// try to create conflict with ones in the model. // try to create conflict with ones in the model.
string16 google_keyword(net::StripWWW(ASCIIToUTF16(GURL( string16 google_keyword(net::StripWWW(ASCIIToUTF16(GURL(
UIThreadSearchTermsData(profile_a()).GoogleBaseURLValue()).host()))); UIThreadSearchTermsData(profile_a()).GoogleBaseURLValue()).host())));
TemplateURL* google = CreateTestTemplateURL(google_keyword, const std::string local_google_url =
"{google:baseURL}1/search?q={searchTerms}"); "{google:baseURL}1/search?q={searchTerms}";
TemplateURL* google = CreateTestTemplateURL(google_keyword, local_google_url);
model()->Add(google); model()->Add(google);
TemplateURL* other = TemplateURL* other =
CreateTestTemplateURL(ASCIIToUTF16("other.com"), "http://other.com/foo"); CreateTestTemplateURL(ASCIIToUTF16("other.com"), "http://other.com/foo");
...@@ -1302,34 +1101,46 @@ TEST_F(TemplateURLServiceSyncTest, AutogeneratedKeywordConflicts) { ...@@ -1302,34 +1101,46 @@ TEST_F(TemplateURLServiceSyncTest, AutogeneratedKeywordConflicts) {
"{google:baseURL}2/search?q={searchTerms}", "sync1", 50)); "{google:baseURL}2/search?q={searchTerms}", "sync1", 50));
initial_data.push_back( initial_data.push_back(
CreateCustomSyncData(*turl, true, turl->url(), turl->sync_guid())); 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"), turl.reset(CreateTestTemplateURL(ASCIIToUTF16("sync2"),
"http://other.com/search?q={searchTerms}", "sync2", 150)); synced_other_url, "sync2", 150));
initial_data.push_back( initial_data.push_back(
CreateCustomSyncData(*turl, true, turl->url(), turl->sync_guid())); 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, model()->MergeDataAndStartSyncing(syncer::SEARCH_ENGINES, initial_data,
PassProcessor(), CreateAndPassSyncErrorFactory()); PassProcessor(), CreateAndPassSyncErrorFactory());
// In this case, the conflicts should be handled just like any other keyword // In this case, the conflicts should be handled just like any other keyword
// conflicts -- the later-modified TemplateURL is assumed to be authoritative. // conflicts -- the later-modified TemplateURL is assumed to be authoritative.
EXPECT_EQ(google_keyword, google->keyword()); // Since the initial TemplateURLs were local only, they should be merged with
EXPECT_EQ(google_keyword + ASCIIToUTF16("_"), // the sync TemplateURLs (GUIDs transferred over).
model()->GetTemplateURLForGUID("sync1")->keyword()); EXPECT_FALSE(model()->GetTemplateURLForGUID(local_google_guid));
EXPECT_EQ(ASCIIToUTF16("other.com_"), other->keyword()); 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"), EXPECT_EQ(ASCIIToUTF16("other.com"),
model()->GetTemplateURLForGUID("sync2")->keyword()); model()->GetTemplateURLForGUID("sync2")->keyword());
// Both synced URLs should have associated UPDATEs, since both needed their // Both synced URLs should have associated UPDATEs, since both needed their
// keywords to be generated (and sync1 needed conflict resolution as well). // keywords to be generated.
EXPECT_GE(processor()->change_list_size(), 2U); EXPECT_EQ(processor()->change_list_size(), 2U);
ASSERT_TRUE(processor()->contains_guid("sync1")); ASSERT_TRUE(processor()->contains_guid("sync1"));
syncer::SyncChange sync1_change = processor()->change_for_guid("sync1"); syncer::SyncChange sync1_change = processor()->change_for_guid("sync1");
EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, sync1_change.change_type()); EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, sync1_change.change_type());
EXPECT_EQ(google_keyword + ASCIIToUTF16("_"), EXPECT_EQ(google_keyword, UTF8ToUTF16(GetKeyword(sync1_change.sync_data())));
UTF8ToUTF16(GetKeyword(sync1_change.sync_data()))); EXPECT_EQ(local_google_url, GetURL(sync1_change.sync_data()));
ASSERT_TRUE(processor()->contains_guid("sync2")); ASSERT_TRUE(processor()->contains_guid("sync2"));
syncer::SyncChange sync2_change = processor()->change_for_guid("sync2"); syncer::SyncChange sync2_change = processor()->change_for_guid("sync2");
EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, sync2_change.change_type()); EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, sync2_change.change_type());
EXPECT_EQ("other.com", GetKeyword(sync2_change.sync_data())); EXPECT_EQ("other.com", GetKeyword(sync2_change.sync_data()));
EXPECT_EQ(synced_other_url, GetURL(sync2_change.sync_data()));
} }
TEST_F(TemplateURLServiceSyncTest, TwoAutogeneratedKeywordsUsingGoogleBaseURL) { TEST_F(TemplateURLServiceSyncTest, TwoAutogeneratedKeywordsUsingGoogleBaseURL) {
...@@ -1358,6 +1169,7 @@ TEST_F(TemplateURLServiceSyncTest, TwoAutogeneratedKeywordsUsingGoogleBaseURL) { ...@@ -1358,6 +1169,7 @@ TEST_F(TemplateURLServiceSyncTest, TwoAutogeneratedKeywordsUsingGoogleBaseURL) {
TemplateURL* keyword2 = model()->GetTemplateURLForKeyword(google_keyword); TemplateURL* keyword2 = model()->GetTemplateURLForKeyword(google_keyword);
ASSERT_FALSE(keyword2 == NULL); ASSERT_FALSE(keyword2 == NULL);
EXPECT_EQ("key2", keyword2->sync_guid()); EXPECT_EQ("key2", keyword2->sync_guid());
EXPECT_GE(processor()->change_list_size(), 2U); EXPECT_GE(processor()->change_list_size(), 2U);
ASSERT_TRUE(processor()->contains_guid("key1")); ASSERT_TRUE(processor()->contains_guid("key1"));
syncer::SyncChange key1_change = processor()->change_for_guid("key1"); syncer::SyncChange key1_change = processor()->change_for_guid("key1");
...@@ -1836,8 +1648,11 @@ TEST_F(TemplateURLServiceSyncTest, SyncMergeDeletesDefault) { ...@@ -1836,8 +1648,11 @@ TEST_F(TemplateURLServiceSyncTest, SyncMergeDeletesDefault) {
TEST_F(TemplateURLServiceSyncTest, LocalDefaultWinsConflict) { TEST_F(TemplateURLServiceSyncTest, LocalDefaultWinsConflict) {
// We expect that the local default always wins keyword conflict resolution. // We expect that the local default always wins keyword conflict resolution.
const string16 keyword(ASCIIToUTF16("key1")); const string16 keyword(ASCIIToUTF16("key1"));
const std::string url("http://whatever.com/{searchTerms}");
TemplateURL* default_turl = CreateTestTemplateURL(keyword, TemplateURL* default_turl = CreateTestTemplateURL(keyword,
"http://whatever.com/{searchTerms}", "whateverguid", 10); url,
"whateverguid",
10);
model()->Add(default_turl); model()->Add(default_turl);
model()->SetDefaultSearchProvider(default_turl); model()->SetDefaultSearchProvider(default_turl);
...@@ -1851,16 +1666,24 @@ TEST_F(TemplateURLServiceSyncTest, LocalDefaultWinsConflict) { ...@@ -1851,16 +1666,24 @@ TEST_F(TemplateURLServiceSyncTest, LocalDefaultWinsConflict) {
model()->MergeDataAndStartSyncing(syncer::SEARCH_ENGINES, initial_data, model()->MergeDataAndStartSyncing(syncer::SEARCH_ENGINES, initial_data,
PassProcessor(), CreateAndPassSyncErrorFactory()); PassProcessor(), CreateAndPassSyncErrorFactory());
// The conflicting TemplateURL should be added, but it should have lost // Since the local default was not yet synced, it should be merged with the
// conflict resolution against the default. // conflicting TemplateURL. However, its values should have been preserved
EXPECT_EQ(4U, model()->GetAllSyncData(syncer::SEARCH_ENGINES).size()); // since it would have won conflict resolution due to being the default.
const TemplateURL* winner = model()->GetTemplateURLForGUID("whateverguid"); EXPECT_EQ(3U, model()->GetAllSyncData(syncer::SEARCH_ENGINES).size());
const TemplateURL* winner = model()->GetTemplateURLForGUID("key1");
ASSERT_TRUE(winner); ASSERT_TRUE(winner);
EXPECT_EQ(model()->GetDefaultSearchProvider(), winner); EXPECT_EQ(model()->GetDefaultSearchProvider(), winner);
EXPECT_EQ(keyword, winner->keyword()); EXPECT_EQ(keyword, winner->keyword());
const TemplateURL* loser = model()->GetTemplateURLForGUID("key1"); EXPECT_EQ(url, winner->url());
ASSERT_TRUE(loser); ASSERT_TRUE(processor()->contains_guid("key1"));
EXPECT_EQ(ASCIIToUTF16("key1.com"), loser->keyword()); EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE,
processor()->change_for_guid("key1").change_type());
EXPECT_EQ(url, GetURL(processor()->change_for_guid("key1").sync_data()));
// 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) { TEST_F(TemplateURLServiceSyncTest, DeleteBogusData) {
...@@ -2012,3 +1835,150 @@ TEST_F(TemplateURLServiceSyncTest, SyncBaseURLs) { ...@@ -2012,3 +1835,150 @@ TEST_F(TemplateURLServiceSyncTest, SyncBaseURLs) {
EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, change.change_type()); EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE, change.change_type());
EXPECT_EQ("google.co.in", GetKeyword(change.sync_data())); EXPECT_EQ("google.co.in", GetKeyword(change.sync_data()));
} }
TEST_F(TemplateURLServiceSyncTest, MergeInSyncTemplateURL) {
// An enumeration used to indicate which TemplateURL test value is expected
// for a particular test result.
enum ExpectedTemplateURL {
LOCAL,
SYNC,
BOTH,
NEITHER,
};
// Sets up and executes a MergeInSyncTemplateURL test given a number of
// expected start and end states:
// * |conflict_winner| denotes which TemplateURL should win the
// conflict.
// * |synced_at_start| denotes which of the TemplateURLs should known
// to Sync.
// * |update_sent| denotes which TemplateURL should have an
// ACTION_UPDATE sent to the server after the merge.
// * |turl_uniquified| denotes which TemplateURL should have its
// keyword updated after the merge.
// * |present_in_model| denotes which TemplateURL should be found in
// the model after the merge.
// * If |keywords_conflict| is true, the TemplateURLs are set up with
// the same keyword.
const struct {
ExpectedTemplateURL conflict_winner;
ExpectedTemplateURL synced_at_start;
ExpectedTemplateURL update_sent;
ExpectedTemplateURL turl_uniquified;
ExpectedTemplateURL present_in_model;
bool keywords_conflict;
} test_cases[] = {
// Both are synced and the new sync entry is better: Local is uniquified and
// UPDATE sent. Sync is added.
{SYNC, BOTH, LOCAL, LOCAL, BOTH, true},
// Both are synced and the local entry is better: Sync is uniquified and
// added to the model. An UPDATE is sent for it.
{LOCAL, BOTH, SYNC, SYNC, BOTH, true},
// Local was not known to Sync and the new sync entry is better: Sync is
// added. Local is removed. No updates.
{SYNC, SYNC, NEITHER, NEITHER, SYNC, true},
// Local was not known to sync and the local entry is better: Local is
// updated with sync GUID, Sync is not added. UPDATE sent for Sync.
{LOCAL, SYNC, SYNC, NEITHER, SYNC, true},
// No conflicting keyword. Both should be added with their original
// keywords, with no updates sent. Note that MergeDataAndStartSyncing is
// responsible for creating the ACTION_ADD for the local TemplateURL.
{NEITHER, SYNC, NEITHER, NEITHER, BOTH, false},
};
for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_cases); ++i) {
// Assert all the valid states of ExpectedTemplateURLs.
ASSERT_FALSE(test_cases[i].conflict_winner == BOTH);
ASSERT_FALSE(test_cases[i].synced_at_start == NEITHER);
ASSERT_FALSE(test_cases[i].synced_at_start == LOCAL);
ASSERT_FALSE(test_cases[i].update_sent == BOTH);
ASSERT_FALSE(test_cases[i].turl_uniquified == BOTH);
ASSERT_FALSE(test_cases[i].present_in_model == NEITHER);
const string16 local_keyword = ASCIIToUTF16("localkeyword");
const string16 sync_keyword = test_cases[i].keywords_conflict ?
local_keyword : ASCIIToUTF16("synckeyword");
const std::string local_url = "www.localurl.com";
const std::string sync_url = "www.syncurl.com";
const time_t local_last_modified = 100;
const time_t sync_last_modified =
test_cases[i].conflict_winner == SYNC ? 110 : 90;
const std::string local_guid = "local_guid";
const std::string sync_guid = "sync_guid";
// Initialize expectations.
string16 expected_local_keyword = local_keyword;
string16 expected_sync_keyword = sync_keyword;
// Create the data and run the actual test.
TemplateURL* local_turl = CreateTestTemplateURL(
local_keyword, local_url, local_guid, local_last_modified);
model()->Add(local_turl);
TemplateURL* sync_turl = CreateTestTemplateURL(
sync_keyword, sync_url, sync_guid, sync_last_modified);
SyncDataMap sync_data;
if (test_cases[i].synced_at_start == SYNC ||
test_cases[i].synced_at_start == BOTH) {
sync_data[sync_turl->sync_guid()] =
TemplateURLService::CreateSyncDataFromTemplateURL(*sync_turl);
}
if (test_cases[i].synced_at_start == BOTH) {
sync_data[local_turl->sync_guid()] =
TemplateURLService::CreateSyncDataFromTemplateURL(*local_turl);
}
SyncDataMap initial_data;
initial_data[local_turl->sync_guid()] =
TemplateURLService::CreateSyncDataFromTemplateURL(*local_turl);
syncer::SyncChangeList change_list;
model()->MergeInSyncTemplateURL(sync_turl, sync_data, &change_list,
&initial_data);
// Check for expected updates, if any.
std::string expected_update_guid;
if (test_cases[i].update_sent == LOCAL)
expected_update_guid = local_guid;
else if (test_cases[i].update_sent == SYNC)
expected_update_guid = sync_guid;
if (!expected_update_guid.empty()) {
ASSERT_EQ(1U, change_list.size());
EXPECT_EQ(expected_update_guid, GetGUID(change_list[0].sync_data()));
EXPECT_EQ(syncer::SyncChange::ACTION_UPDATE,
change_list[0].change_type());
} else {
EXPECT_EQ(0U, change_list.size());
}
// Adjust the expectations based on the expectation enums.
if (test_cases[i].turl_uniquified == LOCAL) {
DCHECK(test_cases[i].keywords_conflict);
expected_local_keyword = ASCIIToUTF16("localkeyword_");
}
if (test_cases[i].turl_uniquified == SYNC) {
DCHECK(test_cases[i].keywords_conflict);
expected_sync_keyword = ASCIIToUTF16("localkeyword_");
}
// Check for TemplateURLs expected in the model. Note that this is checked
// by GUID rather than the initial pointer, as a merge could occur (the
// Sync TemplateURL overtakes the local one). Also remove the present
// TemplateURL when done so the next test case starts with a clean slate.
if (test_cases[i].present_in_model == LOCAL ||
test_cases[i].present_in_model == BOTH) {
ASSERT_TRUE(model()->GetTemplateURLForGUID(local_guid));
EXPECT_EQ(expected_local_keyword, local_turl->keyword());
EXPECT_EQ(local_url, local_turl->url());
EXPECT_EQ(local_last_modified, local_turl->last_modified().ToTimeT());
model()->Remove(model()->GetTemplateURLForGUID(local_guid));
}
if (test_cases[i].present_in_model == SYNC ||
test_cases[i].present_in_model == BOTH) {
ASSERT_TRUE(model()->GetTemplateURLForGUID(sync_guid));
EXPECT_EQ(expected_sync_keyword, sync_turl->keyword());
EXPECT_EQ(sync_url, sync_turl->url());
EXPECT_EQ(sync_last_modified, sync_turl->last_modified().ToTimeT());
model()->Remove(model()->GetTemplateURLForGUID(sync_guid));
}
} // for
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment