chrome/browser/search_engines/template_url_service_sync_unittest.cc - Issue 9749012: [Sync] Have SyncableService's take ownership of their SyncChangeProcessor.

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

Issue 9749012: [Sync] Have SyncableService's take ownership of their SyncChangeProcessor. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src

Patch Set: Initial comments Created 8 years, 9 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/prefs/pref_model_associator.cc ('K') | « chrome/browser/search_engines/template_url_service.cc ('k') | chrome/browser/search_engines/template_url_service_test_util.h » ('j') | chrome/browser/sync/glue/ui_data_type_controller.h » ('J')
Expand Comments ('e') | Collapse Comments ('c') | 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 9f9133bd07ba57e5581fbc682994a0d2912b4817..8d261315927cfb03f7c6c4202de16b52922c5004 100644

--- a/chrome/browser/search_engines/template_url_service_sync_unittest.cc

+++ b/chrome/browser/search_engines/template_url_service_sync_unittest.cc

@@ -127,11 +127,34 @@ class TestChangeProcessor : public SyncChangeProcessor {

DISALLOW_COPY_AND_ASSIGN(TestChangeProcessor);

};

+class SyncChangeProcessorDelegate : public SyncChangeProcessor {

akalin 2012/03/22 20:15:08 since you're defining this in multiple places, mak

Nicolas Zea 2012/03/22 21:22:18 Done.

+ public:

+ explicit SyncChangeProcessorDelegate(SyncChangeProcessor* recipient)

+ : recipient_(recipient) {

+ DCHECK(recipient_);

+ }

+ virtual ~SyncChangeProcessorDelegate() {}

+ // SyncChangeProcessor implementation.

+ virtual SyncError ProcessSyncChanges(

+ const tracked_objects::Location& from_here,

+ const SyncChangeList& change_list) OVERRIDE {

+ return recipient_->ProcessSyncChanges(from_here, change_list);

+ }

+ private:

+ // The recipient of all sync changes.

+ SyncChangeProcessor* recipient_;

+};

class TemplateURLServiceSyncTest : public testing::Test {

public:

typedef TemplateURLService::SyncDataMap SyncDataMap;

- TemplateURLServiceSyncTest() {}

+ TemplateURLServiceSyncTest()

+ : sync_processor_(new TestChangeProcessor),

+ sync_processor_delegate_(new SyncChangeProcessorDelegate(

+ sync_processor_.get())) {}

virtual void SetUp() {

profile_a_.reset(new TestingProfile);

@@ -153,7 +176,10 @@ class TemplateURLServiceSyncTest : public testing::Test {

// involve syncing two models.

TemplateURLService* model_a() { return model_a_.get(); }

TemplateURLService* model_b() { return model_b_.get(); }

- TestChangeProcessor* processor() { return &processor_; }

+ TestChangeProcessor* processor() { return sync_processor_.get(); }

+ scoped_ptr<SyncChangeProcessor> PassProcessor() {

+ return sync_processor_delegate_.PassAs<SyncChangeProcessor>();

+ }

// Create a TemplateURL with some test values. The caller owns the returned

// TemplateURL*.

@@ -275,7 +301,8 @@ class TemplateURLServiceSyncTest : public testing::Test {

scoped_ptr<TemplateURLService> model_b_;

// Our dummy ChangeProcessor used to inspect changes pushed to Sync.

- TestChangeProcessor processor_;

+ scoped_ptr<TestChangeProcessor> sync_processor_;

+ scoped_ptr<SyncChangeProcessorDelegate> sync_processor_delegate_;

DISALLOW_COPY_AND_ASSIGN(TemplateURLServiceSyncTest);

};

@@ -521,7 +548,7 @@ TEST_F(TemplateURLServiceSyncTest, MergeSyncAndLocalURLDuplicates) {

TEST_F(TemplateURLServiceSyncTest, StartSyncEmpty) {

model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, SyncDataList(),

- processor());

+ PassProcessor());

EXPECT_EQ(0U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());

EXPECT_EQ(0, processor()->change_list_size());

@@ -531,7 +558,7 @@ TEST_F(TemplateURLServiceSyncTest, MergeIntoEmpty) {

SyncDataList initial_data = CreateInitialSyncData();

model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, initial_data,

- processor());

+ PassProcessor());

EXPECT_EQ(3U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());

// We expect the model to have accepted all of the initial sync data. Search

@@ -555,7 +582,7 @@ TEST_F(TemplateURLServiceSyncTest, MergeInAllNewData) {

SyncDataList initial_data = CreateInitialSyncData();

model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, initial_data,

- processor());

+ PassProcessor());

EXPECT_EQ(6U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());

// We expect the model to have accepted all of the initial sync data. Search

@@ -588,7 +615,7 @@ TEST_F(TemplateURLServiceSyncTest, MergeSyncIsTheSame) {

}

model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, initial_data,

- processor());

+ PassProcessor());

EXPECT_EQ(3U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());

for (SyncDataList::const_iterator iter = initial_data.begin();

@@ -621,7 +648,7 @@ TEST_F(TemplateURLServiceSyncTest, MergeUpdateFromSync) {

TemplateURLService::CreateSyncDataFromTemplateURL(*turl2_older));

model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, initial_data,

- processor());

+ PassProcessor());

// Both were local updates, so we expect the same count.

EXPECT_EQ(2U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());

@@ -654,7 +681,7 @@ TEST_F(TemplateURLServiceSyncTest, MergeAddFromOlderSyncData) {

"http://unique.com", "ccc")); // add

model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, initial_data,

- processor());

+ PassProcessor());

// The dupe results in a merge. The other two should be added to the model.

EXPECT_EQ(5U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());

@@ -714,7 +741,7 @@ TEST_F(TemplateURLServiceSyncTest, MergeAddFromNewerSyncData) {

"http://unique.com", "ccc", 10)); // add

model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, initial_data,

- processor());

+ PassProcessor());

// The dupe results in a merge. The other two should be added to the model.

EXPECT_EQ(5U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());

@@ -753,7 +780,7 @@ TEST_F(TemplateURLServiceSyncTest, MergeAddFromNewerSyncData) {

TEST_F(TemplateURLServiceSyncTest, ProcessChangesEmptyModel) {

// We initially have no data.

model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, SyncDataList(),

- processor());

+ PassProcessor());

// Set up a bunch of ADDs.

SyncChangeList changes;

@@ -775,7 +802,7 @@ TEST_F(TemplateURLServiceSyncTest, ProcessChangesEmptyModel) {

TEST_F(TemplateURLServiceSyncTest, ProcessChangesNoConflicts) {

model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,

- CreateInitialSyncData(), processor());

+ CreateInitialSyncData(), PassProcessor());

// Process different types of changes, without conflicts.

SyncChangeList changes;

@@ -804,7 +831,7 @@ TEST_F(TemplateURLServiceSyncTest, ProcessChangesNoConflicts) {

TEST_F(TemplateURLServiceSyncTest, ProcessChangesWithConflictsSyncWins) {

model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,

- CreateInitialSyncData(), processor());

+ CreateInitialSyncData(), PassProcessor());

// Process different types of changes, with conflicts. Note that all this data

// has a newer timestamp, so Sync will win in these scenarios.

@@ -840,7 +867,7 @@ TEST_F(TemplateURLServiceSyncTest, ProcessChangesWithConflictsSyncWins) {

TEST_F(TemplateURLServiceSyncTest, ProcessChangesWithConflictsLocalWins) {

model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,

- CreateInitialSyncData(), processor());

+ CreateInitialSyncData(), PassProcessor());

// Process different types of changes, with conflicts. Note that all this data

// has an older timestamp, so the local data will win in these scenarios.

@@ -887,7 +914,7 @@ TEST_F(TemplateURLServiceSyncTest, ProcessTemplateURLChange) {

// Ensure that ProcessTemplateURLChange is called and pushes the correct

// changes to Sync whenever local changes are made to TemplateURLs.

model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,

- CreateInitialSyncData(), processor());

+ CreateInitialSyncData(), PassProcessor());

// Add a new search engine.

TemplateURL* new_turl =

@@ -922,12 +949,15 @@ TEST_F(TemplateURLServiceSyncTest, ProcessTemplateURLChange) {

TEST_F(TemplateURLServiceSyncTest, MergeTwoClientsBasic) {

// Start off B with some empty data.

model_b()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,

- CreateInitialSyncData(), processor());

+ CreateInitialSyncData(), PassProcessor());

// Merge A and B. All of B's data should transfer over to A, which initially

// has no data.

+ scoped_ptr<SyncChangeProcessorDelegate> delegate_b(

+ new SyncChangeProcessorDelegate(model_b()));

model_a()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,

- model_b()->GetAllSyncData(syncable::SEARCH_ENGINES), model_b());

+ model_b()->GetAllSyncData(syncable::SEARCH_ENGINES),

+ delegate_b.PassAs<SyncChangeProcessor>());

// They should be consistent.

AssertEquals(model_a()->GetAllSyncData(syncable::SEARCH_ENGINES),

@@ -937,7 +967,7 @@ TEST_F(TemplateURLServiceSyncTest, MergeTwoClientsBasic) {

TEST_F(TemplateURLServiceSyncTest, MergeTwoClientsDupesAndConflicts) {

// Start off B with some empty data.

model_b()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,

- CreateInitialSyncData(), processor());

+ CreateInitialSyncData(), PassProcessor());

// Set up A so we have some interesting duplicates and conflicts.

model_a()->Add(CreateTestTemplateURL(ASCIIToUTF16("key4"), "http://key4.com",

@@ -950,8 +980,11 @@ TEST_F(TemplateURLServiceSyncTest, MergeTwoClientsDupesAndConflicts) {

"key6", 10)); // Conflict with key1

// Merge A and B.

+ scoped_ptr<SyncChangeProcessorDelegate> delegate_b(

+ new SyncChangeProcessorDelegate(model_b()));

model_a()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,

- model_b()->GetAllSyncData(syncable::SEARCH_ENGINES), model_b());

+ model_b()->GetAllSyncData(syncable::SEARCH_ENGINES),

+ delegate_b.PassAs<SyncChangeProcessor>());

// They should be consistent.

AssertEquals(model_a()->GetAllSyncData(syncable::SEARCH_ENGINES),

@@ -960,7 +993,7 @@ TEST_F(TemplateURLServiceSyncTest, MergeTwoClientsDupesAndConflicts) {

TEST_F(TemplateURLServiceSyncTest, StopSyncing) {

SyncError error = model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,

- CreateInitialSyncData(), processor());

+ CreateInitialSyncData(), PassProcessor());

ASSERT_FALSE(error.IsSet());

model()->StopSyncing(syncable::SEARCH_ENGINES);

@@ -979,7 +1012,7 @@ TEST_F(TemplateURLServiceSyncTest, StopSyncing) {

TEST_F(TemplateURLServiceSyncTest, SyncErrorOnInitialSync) {

processor()->set_erroneous(true);

SyncError error = model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,

- CreateInitialSyncData(), processor());

+ CreateInitialSyncData(), PassProcessor());

EXPECT_TRUE(error.IsSet());

// Ensure that if the initial merge was erroneous, then subsequence attempts

@@ -1002,7 +1035,7 @@ TEST_F(TemplateURLServiceSyncTest, SyncErrorOnLaterSync) {

// Ensure that if the SyncProcessor succeeds in the initial merge, but fails

// in future ProcessSyncChanges, we still return an error.

SyncError error = model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,

- CreateInitialSyncData(), processor());

+ CreateInitialSyncData(), PassProcessor());

ASSERT_FALSE(error.IsSet());

SyncChangeList changes;

@@ -1024,7 +1057,7 @@ TEST_F(TemplateURLServiceSyncTest, MergeTwiceWithSameSyncData) {

"key1", 10)); // earlier

SyncError error = model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,

- initial_data, processor());

+ initial_data, PassProcessor());

ASSERT_FALSE(error.IsSet());

// We should have updated the original TemplateURL with Sync's version.

@@ -1044,8 +1077,10 @@ TEST_F(TemplateURLServiceSyncTest, MergeTwiceWithSameSyncData) {

// Remerge the data again. This simulates shutting down and syncing again

// at a different time, but the cloud data has not changed.

model()->StopSyncing(syncable::SEARCH_ENGINES);

+ sync_processor_delegate_.reset(new SyncChangeProcessorDelegate(

+ sync_processor_.get()));

error = model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,

- initial_data, processor());

+ initial_data, PassProcessor());

ASSERT_FALSE(error.IsSet());

// Check that the TemplateURL was not modified.

@@ -1057,7 +1092,7 @@ TEST_F(TemplateURLServiceSyncTest, MergeTwiceWithSameSyncData) {

TEST_F(TemplateURLServiceSyncTest, SyncedDefaultGUIDArrivesFirst) {

SyncDataList initial_data = CreateInitialSyncData();

model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, initial_data,

- processor());

+ PassProcessor());

model()->SetDefaultSearchProvider(model()->GetTemplateURLForGUID("key2"));

EXPECT_EQ(3U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());

@@ -1118,7 +1153,7 @@ TEST_F(TemplateURLServiceSyncTest, SyncedDefaultArrivesAfterStartup) {

// destined to become the new default.

SyncDataList initial_data = CreateInitialSyncData();

model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, initial_data,

- processor());

+ PassProcessor());

// Ensure that the new default has been set.

EXPECT_EQ(4U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());

@@ -1146,7 +1181,7 @@ TEST_F(TemplateURLServiceSyncTest, SyncedDefaultAlreadySetOnStartup) {

// Now sync the initial data.

SyncDataList initial_data = CreateInitialSyncData();

model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, initial_data,

- processor());

+ PassProcessor());

// Ensure that the new entries were added and the default has not changed.

EXPECT_EQ(4U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());

@@ -1158,7 +1193,7 @@ TEST_F(TemplateURLServiceSyncTest, SyncWithManagedDefaultSearch) {

// default search provider.

SyncDataList initial_data = CreateInitialSyncData();

model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES, initial_data,

- processor());

+ PassProcessor());

model()->SetDefaultSearchProvider(model()->GetTemplateURLForGUID("key2"));

EXPECT_EQ(3U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());

@@ -1214,7 +1249,7 @@ TEST_F(TemplateURLServiceSyncTest, SyncMergeDeletesDefault) {

// The key1 entry should be a duplicate of the default.

model()->MergeDataAndStartSyncing(syncable::SEARCH_ENGINES,

- CreateInitialSyncData(), processor());

+ CreateInitialSyncData(), PassProcessor());

EXPECT_EQ(3U, model()->GetAllSyncData(syncable::SEARCH_ENGINES).size());

EXPECT_FALSE(model()->GetTemplateURLForGUID("whateverguid"));