[Sync] Test that MergeSyncData is not called on subsequent starts.

BUG=

Review-Url: https://codereview.chromium.org/2363853002
Cr-Commit-Position: refs/heads/master@{#420723}

components/sync/core/shared_model_type_processor_unittest.cc

@@ -170,25 +170,6 @@ class SharedModelTypeProcessorTest : public ::testing::Test,
     worker_ = nullptr;
   }
 
-  // Simulates an initial GetUpdates response from the worker with |updates|.
-  void OnInitialSyncDone(UpdateResponseDataList updates) {
-    DataTypeState data_type_state(db_.data_type_state());
-    data_type_state.set_initial_sync_done(true);
-    type_processor()->OnUpdateReceived(data_type_state, updates);
-  }
-
-  // Overloaded form with no updates.
-  void OnInitialSyncDone() { OnInitialSyncDone(UpdateResponseDataList()); }
-
-  // Overloaded form that constructs an update for a single entity.
-  void OnInitialSyncDone(const std::string& key, const std::string& value) {
-    UpdateResponseDataList updates;
-    UpdateResponseData update;
-    update.entity = GenerateEntityData(key, value)->PassToPtr();
-    updates.push_back(update);
-    OnInitialSyncDone(updates);
-  }
-
   // Return the number of entities the processor has metadata for.
   size_t ProcessorEntityCount() const {
     DCHECK(type_processor());
@@ -219,6 +200,8 @@ class SharedModelTypeProcessorTest : public ::testing::Test,
     return error_handler_;
   }
 
+  int merge_call_count() const { return merge_call_count_; }
+
  private:
   void CheckPostConditions() override {
     FakeModelTypeService::CheckPostConditions();
@@ -248,6 +231,12 @@ class SharedModelTypeProcessorTest : public ::testing::Test,
 
   // FakeModelTypeService overrides.
 
+  syncer::SyncError MergeSyncData(std::unique_ptr<MetadataChangeList> mcl,
+                                  EntityDataMap entity_data_map) override {
+    merge_call_count_++;
+    return FakeModelTypeService::MergeSyncData(std::move(mcl), entity_data_map);
+  }
+
   void CaptureDataCallback(DataCallback callback,
                            syncer::SyncError error,
                            std::unique_ptr<DataBatch> data) {
@@ -284,6 +273,9 @@ class SharedModelTypeProcessorTest : public ::testing::Test,
 
   // The error to expect in OnReadyToConnect().
   syncer::SyncError::ErrorType expected_start_error_ = syncer::SyncError::UNSET;
+
+  // The number of times MergeSyncData has been called.
+  int merge_call_count_ = 0;
 };
 
 // Test that an initial sync handles local and remote items properly.
@@ -301,8 +293,10 @@ TEST_F(SharedModelTypeProcessorTest, InitialSync) {
   EXPECT_EQ(0U, ProcessorEntityCount());
   EXPECT_EQ(0U, worker()->GetNumPendingCommits());
 
+  EXPECT_EQ(0, merge_call_count());
   // Initial sync with one server item.
-  OnInitialSyncDone(kKey2, kValue2);
+  worker()->UpdateFromServer(kHash2, GenerateSpecifics(kKey2, kValue2));
+  EXPECT_EQ(1, merge_call_count());
 
   // Now have data and metadata for both items, as well as a commit request for
   // the local item.
@@ -314,6 +308,24 @@ TEST_F(SharedModelTypeProcessorTest, InitialSync) {
   worker()->ExpectPendingCommits({kHash1});
 }
 
+// Test that subsequent starts don't call MergeSyncData.
+TEST_F(SharedModelTypeProcessorTest, NonInitialSync) {
+  // This sets initial_sync_done to true.
+  InitializeToMetadataLoaded();
+
+  // Write an item before sync connects.
+  WriteItem(kKey1, kValue1);
+  EXPECT_EQ(1U, db().data_count());
+  EXPECT_EQ(1U, db().metadata_count());
+
+  // Check that data coming from sync is treated as a normal GetUpdates.
+  OnSyncStarting();
+  worker()->UpdateFromServer(kHash2, GenerateSpecifics(kKey2, kValue2));
+  EXPECT_EQ(0, merge_call_count());
+  EXPECT_EQ(2U, db().data_count());
+  EXPECT_EQ(2U, db().metadata_count());
+}
+
 // Test that an error during the merge is propagated to the error handler.
 TEST_F(SharedModelTypeProcessorTest, InitialSyncError) {
   CreateChangeProcessor();
@@ -322,7 +334,7 @@ TEST_F(SharedModelTypeProcessorTest, InitialSyncError) {
 
   SetServiceError(syncer::SyncError::DATATYPE_ERROR);
   error_handler()->ExpectError(syncer::SyncError::DATATYPE_ERROR);
-  OnInitialSyncDone();
+  worker()->UpdateFromServer();
 }
 
 // Test that errors before it's called are passed to |start_callback| correctly.
@@ -1078,7 +1090,7 @@ TEST_F(SharedModelTypeProcessorTest, Disable) {
   CreateChangeProcessor();
   OnMetadataLoaded();
   OnSyncStarting();
-  OnInitialSyncDone();
+  worker()->UpdateFromServer();
 
   // Once we're ready to commit, all three local items should consider
   // themselves uncommitted and pending for commit.

components/sync/test/engine/mock_model_type_worker.cc

@@ -21,7 +21,9 @@ namespace {
 MockModelTypeWorker::MockModelTypeWorker(
     const sync_pb::DataTypeState& data_type_state,
     ModelTypeProcessor* processor)
-    : data_type_state_(data_type_state), processor_(processor) {}
+    : data_type_state_(data_type_state), processor_(processor) {
+  data_type_state_.set_initial_sync_done(true);
+}
 
 MockModelTypeWorker::~MockModelTypeWorker() {}
 
@@ -88,6 +90,10 @@ void MockModelTypeWorker::ExpectPendingCommits(
   }
 }
 
+void MockModelTypeWorker::UpdateFromServer() {
+  processor_->OnUpdateReceived(data_type_state_, UpdateResponseDataList());
+}
+
 void MockModelTypeWorker::UpdateFromServer(
     const std::string& tag_hash,
     const sync_pb::EntitySpecifics& specifics) {
@@ -107,10 +113,10 @@ void MockModelTypeWorker::UpdateFromServer(
     const sync_pb::EntitySpecifics& specifics,
     int64_t version_offset,
     const std::string& ekn) {
-  UpdateResponseDataList update;
-  update.push_back(
+  UpdateResponseDataList updates;
+  updates.push_back(
       GenerateUpdateData(tag_hash, specifics, version_offset, ekn));
-  processor_->OnUpdateReceived(data_type_state_, update);
+  processor_->OnUpdateReceived(data_type_state_, updates);
 }
 
 UpdateResponseData MockModelTypeWorker::GenerateUpdateData(

components/sync/test/engine/mock_model_type_worker.h

@@ -53,9 +53,10 @@ class MockModelTypeWorker : public CommitQueue {
   // length one.
   void ExpectPendingCommits(const std::vector<std::string>& tag_hashes);
 
-  // Trigger an update from the server containing a single entity. See
-  // GenerateUpdateData for parameter descriptions. |version_offset| defaults to
-  // 1 and |ekn| defaults to the current encryption key name the worker has.
+  // Trigger an update from the server. See GenerateUpdateData for parameter
+  // descriptions. |version_offset| defaults to 1 and |ekn| defaults to the
+  // current encryption key name the worker has.
+  void UpdateFromServer();
   void UpdateFromServer(const std::string& tag_hash,
                         const sync_pb::EntitySpecifics& specifics);
   void UpdateFromServer(const std::string& tag_hash,