[Sync] Services can now always assume processor exists.

components/sync/core/shared_model_type_processor_unittest.cc

Index: components/sync/core/shared_model_type_processor_unittest.cc
diff --git a/components/sync/core/shared_model_type_processor_unittest.cc b/components/sync/core/shared_model_type_processor_unittest.cc
index 853f6b0642bec3075364b3204fd0fa42a71d60c0..8a49713ffb930164df29370ba0dee40b3510b49b 100644
--- a/components/sync/core/shared_model_type_processor_unittest.cc
+++ b/components/sync/core/shared_model_type_processor_unittest.cc
@@ -54,6 +54,67 @@ SyncError CreateSyncError(SyncError::ErrorType error_type) {
   return SyncError(FROM_HERE, error_type, "TestError", PREFERENCES);
 }
 
+class TestModelTypeService : public FakeModelTypeService {
+ public:
+  TestModelTypeService()
+      : FakeModelTypeService(
+            base::Bind(&SharedModelTypeProcessor::CreateAsChangeProcessor)) {}
+
+  explicit TestModelTypeService(std::unique_ptr<TestModelTypeService> other)
+      : TestModelTypeService() {
+    std::swap(db_, other->db_);
+  }
+
+  void OnPendingCommitDataLoaded() {
+    DCHECK(!data_callback_.is_null());
+    data_callback_.Run();
+    data_callback_.Reset();
+  }
+
+  void InitializeToReadyState() {
+    if (!data_callback_.is_null()) {
+      OnPendingCommitDataLoaded();
+    }
+  }
+
+  void SetInitialSyncDone(bool is_done) {
+    ModelTypeState model_type_state(db()->model_type_state());
+    model_type_state.set_initial_sync_done(is_done);
+    db()->set_model_type_state(model_type_state);
+  }
+
+  int merge_call_count() const { return merge_call_count_; }
+
+  // FakeModelTypeService overrides.
+  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 GetData(StorageKeyList keys, DataCallback callback) override {
+    FakeModelTypeService::GetData(
+        keys, base::Bind(&TestModelTypeService::CaptureDataCallback,
+                         base::Unretained(this), callback));
+  }
+  void CheckPostConditions() override {
+    FakeModelTypeService::CheckPostConditions();
+    DCHECK(data_callback_.is_null());
+  }
+
+ private:
+  void CaptureDataCallback(DataCallback callback,
+                           SyncError error,
+                           std::unique_ptr<DataBatch> data) {
+    data_callback_ = base::Bind(callback, error, base::Passed(std::move(data)));
+  }
+
+  // The number of times MergeSyncData has been called.
+  int merge_call_count_ = 0;
+
+  // Stores the data callback between GetData() and OnPendingCommitDataLoaded().
+  base::Closure data_callback_;
+};
+
 }  // namespace
 
 // Tests the various functionality of SharedModelTypeProcessor.
@@ -71,39 +132,30 @@ SyncError CreateSyncError(SyncError::ErrorType error_type) {
 //   storage and the correct commit requests on the worker side.
 // - Updates and commit responses from the worker correctly affect data and
 //   metadata in storage on the service side.
-class SharedModelTypeProcessorTest : public ::testing::Test,
-                                     public FakeModelTypeService {
+class SharedModelTypeProcessorTest : public ::testing::Test {
  public:
   SharedModelTypeProcessorTest()
-      : FakeModelTypeService(
-            base::Bind(&SharedModelTypeProcessor::CreateAsChangeProcessor)) {}
+      : service_(base::MakeUnique<TestModelTypeService>()) {}
+
+  ~SharedModelTypeProcessorTest() override { CheckPostConditions(); }
 
   void InitializeToMetadataLoaded() {
-    CreateChangeProcessor();
-    ModelTypeState model_type_state(db_.model_type_state());
-    model_type_state.set_initial_sync_done(true);
-    db_.set_model_type_state(model_type_state);
+    service()->SetInitialSyncDone(true);
     OnMetadataLoaded();
   }
 
   // Initialize to a "ready-to-commit" state.
   void InitializeToReadyState() {
     InitializeToMetadataLoaded();
-    if (!data_callback_.is_null()) {
-      OnPendingCommitDataLoaded();
-    }
+    service()->InitializeToReadyState();
     OnSyncStarting();
   }
 
   void OnMetadataLoaded() {
-    type_processor()->OnMetadataLoaded(SyncError(), db_.CreateMetadataBatch());
+    type_processor()->OnMetadataLoaded(SyncError(), db().CreateMetadataBatch());
   }
 
-  void OnPendingCommitDataLoaded() {
-    DCHECK(!data_callback_.is_null());
-    data_callback_.Run();
-    data_callback_.Reset();
-  }
+  void OnPendingCommitDataLoaded() { service()->OnPendingCommitDataLoaded(); }
 
   void OnSyncStarting() {
     std::unique_ptr<DataTypeErrorHandlerMock> error_handler =
@@ -123,16 +175,17 @@ class SharedModelTypeProcessorTest : public ::testing::Test,
   // Writes data for |key| and simulates a commit response for it.
   EntitySpecifics WriteItemAndAck(const std::string& key,
                                   const std::string& value) {
-    EntitySpecifics specifics = WriteItem(key, value);
-    worker()->ExpectPendingCommits({TagHashFromKey(key)});
+    EntitySpecifics specifics = service()->WriteItem(key, value);
+    worker()->ExpectPendingCommits({FakeModelTypeService::TagHashFromKey(key)});
     worker()->AckOnePendingCommit();
     EXPECT_EQ(0U, worker()->GetNumPendingCommits());
     return specifics;
   }
 
-  void ResetState() {
-    clear_change_processor();
-    db_.Reset();
+  void ResetState(bool keep_db) {
+    service_ = keep_db
+                   ? base::MakeUnique<TestModelTypeService>(std::move(service_))
+                   : base::MakeUnique<TestModelTypeService>();
     worker_ = nullptr;
     CheckPostConditions();
   }
@@ -140,33 +193,30 @@ class SharedModelTypeProcessorTest : public ::testing::Test,
   // Wipes existing DB and simulates a pending update of a server-known item.
   EntitySpecifics ResetStateWriteItem(const std::string& name,
                                       const std::string& value) {
-    ResetState();
+    ResetState(false);
     InitializeToReadyState();
     EXPECT_EQ(0U, ProcessorEntityCount());
     WriteItemAndAck(name, "acked-value");
-    EntitySpecifics specifics = WriteItem(name, value);
+    EntitySpecifics specifics = service()->WriteItem(name, value);
     EXPECT_EQ(1U, ProcessorEntityCount());
-    clear_change_processor();
-    worker_ = nullptr;
+    ResetState(true);
     return specifics;
   }
 
   // Wipes existing DB and simulates a pending deletion of a server-known item.
   void ResetStateDeleteItem(const std::string& name, const std::string& value) {
-    ResetState();
+    ResetState(false);
     InitializeToReadyState();
     EXPECT_EQ(0U, ProcessorEntityCount());
     WriteItemAndAck(name, value);
     EXPECT_EQ(1U, ProcessorEntityCount());
-    DeleteItem(name);
+    service()->DeleteItem(name);
     EXPECT_EQ(1U, ProcessorEntityCount());
-    clear_change_processor();
-    worker_ = nullptr;
+    ResetState(true);
   }
 
   // Return the number of entities the processor has metadata for.
   size_t ProcessorEntityCount() const {
-    DCHECK(type_processor());
     return type_processor()->entities_.size();
   }
 
@@ -177,23 +227,24 @@ class SharedModelTypeProcessorTest : public ::testing::Test,
     expected_start_error_ = error_type;
   }
 
-  MockModelTypeWorker* worker() { return worker_; }
+  TestModelTypeService* service() const { return service_.get(); }
+
+  const FakeModelTypeService::Store& db() const { return *service()->db(); }
+
+  MockModelTypeWorker* worker() const { return worker_; }
 
   SharedModelTypeProcessor* type_processor() const {
-    return static_cast<SharedModelTypeProcessor*>(change_processor());
+    return static_cast<SharedModelTypeProcessor*>(
+        service()->change_processor());
   }
 
-  DataTypeErrorHandlerMock* error_handler() {
+  DataTypeErrorHandlerMock* error_handler() const {
     DCHECK(error_handler_);
     return error_handler_;
   }
 
-  int merge_call_count() const { return merge_call_count_; }
-
  private:
-  void CheckPostConditions() override {
-    FakeModelTypeService::CheckPostConditions();
-    DCHECK(data_callback_.is_null());
+  void CheckPostConditions() {
     DCHECK_EQ(SyncError::UNSET, expected_start_error_);
   }
 
@@ -217,25 +268,7 @@ class SharedModelTypeProcessorTest : public ::testing::Test,
     type_processor()->ConnectSync(std::move(worker));
   }
 
-  // FakeModelTypeService overrides.
-
-  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,
-                           SyncError error,
-                           std::unique_ptr<DataBatch> data) {
-    data_callback_ = base::Bind(callback, error, base::Passed(std::move(data)));
-  }
-
-  void GetData(StorageKeyList keys, DataCallback callback) override {
-    FakeModelTypeService::GetData(
-        keys, base::Bind(&SharedModelTypeProcessorTest::CaptureDataCallback,
-                         base::Unretained(this), callback));
-  }
+  std::unique_ptr<TestModelTypeService> service_;
 
   // This sets ThreadTaskRunnerHandle on the current thread, which the type
   // processor will pick up as the sync task runner.
@@ -244,27 +277,20 @@ class SharedModelTypeProcessorTest : public ::testing::Test,
   // The current mock queue, which is owned by |type_processor()|.
   MockModelTypeWorker* worker_;
 
-  // Stores the data callback between GetData() and OnPendingCommitDataLoaded().
-  base::Closure data_callback_;
-
   // The processor's error handler.
   DataTypeErrorHandlerMock* error_handler_;
 
   // The error to expect in OnReadyToConnect().
   SyncError::ErrorType expected_start_error_ = 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.
 TEST_F(SharedModelTypeProcessorTest, InitialSync) {
-  CreateChangeProcessor();
   OnMetadataLoaded();
   OnSyncStarting();
 
   // Local write before initial sync.
-  WriteItem(kKey1, kValue1);
+  service()->WriteItem(kKey1, kValue1);
 
   // Has data, but no metadata, entity in the processor, or commit request.
   EXPECT_EQ(1U, db().data_count());
@@ -272,10 +298,11 @@ TEST_F(SharedModelTypeProcessorTest, InitialSync) {
   EXPECT_EQ(0U, ProcessorEntityCount());
   EXPECT_EQ(0U, worker()->GetNumPendingCommits());
 
-  EXPECT_EQ(0, merge_call_count());
+  EXPECT_EQ(0, service()->merge_call_count());
   // Initial sync with one server item.
-  worker()->UpdateFromServer(kHash2, GenerateSpecifics(kKey2, kValue2));
-  EXPECT_EQ(1, merge_call_count());
+  worker()->UpdateFromServer(kHash2,
+                             service()->GenerateSpecifics(kKey2, kValue2));
+  EXPECT_EQ(1, service()->merge_call_count());
 
   // Now have data and metadata for both items, as well as a commit request for
   // the local item.
@@ -293,40 +320,38 @@ TEST_F(SharedModelTypeProcessorTest, NonInitialSync) {
   InitializeToMetadataLoaded();
 
   // Write an item before sync connects.
-  WriteItem(kKey1, kValue1);
+  service()->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());
+  worker()->UpdateFromServer(kHash2,
+                             service()->GenerateSpecifics(kKey2, kValue2));
+  EXPECT_EQ(0, service()->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();
   OnMetadataLoaded();
   OnSyncStarting();
 
-  SetServiceError(SyncError::DATATYPE_ERROR);
+  service()->SetServiceError(SyncError::DATATYPE_ERROR);
   error_handler()->ExpectError(SyncError::DATATYPE_ERROR);
   worker()->UpdateFromServer();
 }
 
 // Test that errors before it's called are passed to |start_callback| correctly.
 TEST_F(SharedModelTypeProcessorTest, StartErrors) {
-  CreateChangeProcessor();
   type_processor()->OnMetadataLoaded(CreateSyncError(SyncError::DATATYPE_ERROR),
                                      nullptr);
   ExpectStartError(SyncError::DATATYPE_ERROR);
   OnSyncStarting();
 
   // Test OnSyncStarting happening first.
-  ResetState();
-  CreateChangeProcessor();
+  ResetState(false);
   OnSyncStarting();
   ExpectStartError(SyncError::DATATYPE_ERROR);
   type_processor()->OnMetadataLoaded(CreateSyncError(SyncError::DATATYPE_ERROR),
@@ -334,7 +359,7 @@ TEST_F(SharedModelTypeProcessorTest, StartErrors) {
 
   // Test an error loading pending data.
   ResetStateWriteItem(kKey1, kValue1);
-  SetServiceError(SyncError::DATATYPE_ERROR);
+  service()->SetServiceError(SyncError::DATATYPE_ERROR);
   InitializeToMetadataLoaded();
   OnPendingCommitDataLoaded();
   ExpectStartError(SyncError::DATATYPE_ERROR);
@@ -374,7 +399,7 @@ TEST_F(SharedModelTypeProcessorTest, LoadPendingCommit) {
   InitializeToMetadataLoaded();
   OnPendingCommitDataLoaded();
   OnSyncStarting();
-  EntitySpecifics specifics4 = WriteItem(kKey1, kValue2);
+  EntitySpecifics specifics4 = service()->WriteItem(kKey1, kValue2);
   EXPECT_EQ(2U, worker()->GetNumPendingCommits());
   worker()->ExpectNthPendingCommit(0, kHash1, specifics3);
   worker()->ExpectNthPendingCommit(1, kHash1, specifics4);
@@ -383,7 +408,7 @@ TEST_F(SharedModelTypeProcessorTest, LoadPendingCommit) {
   ResetStateWriteItem(kKey1, kValue1);
   InitializeToMetadataLoaded();
   OnPendingCommitDataLoaded();
-  EntitySpecifics specifics5 = WriteItem(kKey1, kValue2);
+  EntitySpecifics specifics5 = service()->WriteItem(kKey1, kValue2);
   OnSyncStarting();
   EXPECT_EQ(1U, worker()->GetNumPendingCommits());
   worker()->ExpectNthPendingCommit(0, kHash1, specifics5);
@@ -393,7 +418,7 @@ TEST_F(SharedModelTypeProcessorTest, LoadPendingCommit) {
   InitializeToMetadataLoaded();
   OnSyncStarting();
   OnPendingCommitDataLoaded();
-  EntitySpecifics specifics7 = WriteItem(kKey1, kValue2);
+  EntitySpecifics specifics7 = service()->WriteItem(kKey1, kValue2);
   EXPECT_EQ(2U, worker()->GetNumPendingCommits());
   worker()->ExpectNthPendingCommit(0, kHash1, specifics6);
   worker()->ExpectNthPendingCommit(1, kHash1, specifics7);
@@ -402,7 +427,7 @@ TEST_F(SharedModelTypeProcessorTest, LoadPendingCommit) {
   ResetStateWriteItem(kKey1, kValue1);
   InitializeToMetadataLoaded();
   OnSyncStarting();
-  EntitySpecifics specifics8 = WriteItem(kKey1, kValue2);
+  EntitySpecifics specifics8 = service()->WriteItem(kKey1, kValue2);
   EXPECT_EQ(nullptr, worker());
   OnPendingCommitDataLoaded();
   EXPECT_EQ(1U, worker()->GetNumPendingCommits());
@@ -411,7 +436,7 @@ TEST_F(SharedModelTypeProcessorTest, LoadPendingCommit) {
   // Put, data, connect.
   ResetStateWriteItem(kKey1, kValue1);
   InitializeToMetadataLoaded();
-  EntitySpecifics specifics9 = WriteItem(kKey1, kValue2);
+  EntitySpecifics specifics9 = service()->WriteItem(kKey1, kValue2);
   OnPendingCommitDataLoaded();
   OnSyncStarting();
   EXPECT_EQ(1U, worker()->GetNumPendingCommits());
@@ -420,7 +445,7 @@ TEST_F(SharedModelTypeProcessorTest, LoadPendingCommit) {
   // Put, connect, data.
   ResetStateWriteItem(kKey1, kValue1);
   InitializeToMetadataLoaded();
-  EntitySpecifics specifics10 = WriteItem(kKey1, kValue2);
+  EntitySpecifics specifics10 = service()->WriteItem(kKey1, kValue2);
   OnSyncStarting();
   EXPECT_EQ(nullptr, worker());
   OnPendingCommitDataLoaded();
@@ -432,7 +457,7 @@ TEST_F(SharedModelTypeProcessorTest, LoadPendingCommit) {
   InitializeToMetadataLoaded();
   OnPendingCommitDataLoaded();
   OnSyncStarting();
-  DeleteItem(kKey1);
+  service()->DeleteItem(kKey1);
   EXPECT_EQ(2U, worker()->GetNumPendingCommits());
   worker()->ExpectNthPendingCommit(0, kHash1, specifics11);
   worker()->ExpectNthPendingCommit(1, kHash1, kEmptySpecifics);
@@ -441,7 +466,7 @@ TEST_F(SharedModelTypeProcessorTest, LoadPendingCommit) {
   ResetStateWriteItem(kKey1, kValue1);
   InitializeToMetadataLoaded();
   OnPendingCommitDataLoaded();
-  DeleteItem(kKey1);
+  service()->DeleteItem(kKey1);
   OnSyncStarting();
   EXPECT_EQ(1U, worker()->GetNumPendingCommits());
   worker()->ExpectNthPendingCommit(0, kHash1, kEmptySpecifics);
@@ -451,7 +476,7 @@ TEST_F(SharedModelTypeProcessorTest, LoadPendingCommit) {
   InitializeToMetadataLoaded();
   OnSyncStarting();
   OnPendingCommitDataLoaded();
-  DeleteItem(kKey1);
+  service()->DeleteItem(kKey1);
   EXPECT_EQ(2U, worker()->GetNumPendingCommits());
   worker()->ExpectNthPendingCommit(0, kHash1, specifics12);
   worker()->ExpectNthPendingCommit(1, kHash1, kEmptySpecifics);
@@ -460,7 +485,7 @@ TEST_F(SharedModelTypeProcessorTest, LoadPendingCommit) {
   ResetStateWriteItem(kKey1, kValue1);
   InitializeToMetadataLoaded();
   OnSyncStarting();
-  DeleteItem(kKey1);
+  service()->DeleteItem(kKey1);
   EXPECT_EQ(nullptr, worker());
   OnPendingCommitDataLoaded();
   EXPECT_EQ(1U, worker()->GetNumPendingCommits());
@@ -469,7 +494,7 @@ TEST_F(SharedModelTypeProcessorTest, LoadPendingCommit) {
   // Delete, data, connect.
   ResetStateWriteItem(kKey1, kValue1);
   InitializeToMetadataLoaded();
-  DeleteItem(kKey1);
+  service()->DeleteItem(kKey1);
   OnPendingCommitDataLoaded();
   OnSyncStarting();
   EXPECT_EQ(1U, worker()->GetNumPendingCommits());
@@ -478,7 +503,7 @@ TEST_F(SharedModelTypeProcessorTest, LoadPendingCommit) {
   // Delete, connect, data.
   ResetStateWriteItem(kKey1, kValue1);
   InitializeToMetadataLoaded();
-  DeleteItem(kKey1);
+  service()->DeleteItem(kKey1);
   OnSyncStarting();
   EXPECT_EQ(nullptr, worker());
   OnPendingCommitDataLoaded();
@@ -509,7 +534,7 @@ TEST_F(SharedModelTypeProcessorTest, LoadPendingDelete) {
   InitializeToMetadataLoaded();
   OnSyncStarting();
   EXPECT_EQ(1U, worker()->GetNumPendingCommits());
-  EntitySpecifics specifics1 = WriteItem(kKey1, kValue2);
+  EntitySpecifics specifics1 = service()->WriteItem(kKey1, kValue2);
   EXPECT_EQ(2U, worker()->GetNumPendingCommits());
   worker()->ExpectNthPendingCommit(0, kHash1, kEmptySpecifics);
   worker()->ExpectNthPendingCommit(1, kHash1, specifics1);
@@ -517,7 +542,7 @@ TEST_F(SharedModelTypeProcessorTest, LoadPendingDelete) {
   // Put, connect.
   ResetStateDeleteItem(kKey1, kValue1);
   InitializeToMetadataLoaded();
-  EntitySpecifics specifics2 = WriteItem(kKey1, kValue2);
+  EntitySpecifics specifics2 = service()->WriteItem(kKey1, kValue2);
   OnSyncStarting();
   EXPECT_EQ(1U, worker()->GetNumPendingCommits());
   worker()->ExpectNthPendingCommit(0, kHash1, specifics2);
@@ -527,7 +552,7 @@ TEST_F(SharedModelTypeProcessorTest, LoadPendingDelete) {
   InitializeToMetadataLoaded();
   OnSyncStarting();
   EXPECT_EQ(1U, worker()->GetNumPendingCommits());
-  DeleteItem(kKey1);
+  service()->DeleteItem(kKey1);
   EXPECT_EQ(2U, worker()->GetNumPendingCommits());
   worker()->ExpectNthPendingCommit(0, kHash1, kEmptySpecifics);
   worker()->ExpectNthPendingCommit(1, kHash1, kEmptySpecifics);
@@ -535,7 +560,7 @@ TEST_F(SharedModelTypeProcessorTest, LoadPendingDelete) {
   // Delete, connect.
   ResetStateDeleteItem(kKey1, kValue1);
   InitializeToMetadataLoaded();
-  DeleteItem(kKey1);
+  service()->DeleteItem(kKey1);
   OnSyncStarting();
   EXPECT_EQ(1U, worker()->GetNumPendingCommits());
   worker()->ExpectNthPendingCommit(0, kHash1, kEmptySpecifics);
@@ -545,7 +570,7 @@ TEST_F(SharedModelTypeProcessorTest, LoadPendingDelete) {
 TEST_F(SharedModelTypeProcessorTest, LoadCommited) {
   InitializeToReadyState();
   WriteItemAndAck(kKey1, kValue1);
-  clear_change_processor();
+  ResetState(true);
 
   // Test that a new processor loads the metadata without committing.
   InitializeToReadyState();
@@ -559,7 +584,7 @@ TEST_F(SharedModelTypeProcessorTest, LocalCreateItem) {
   InitializeToReadyState();
   EXPECT_EQ(0U, worker()->GetNumPendingCommits());
 
-  WriteItem(kKey1, kValue1);
+  service()->WriteItem(kKey1, kValue1);
 
   // Verify the commit request this operation has triggered.
   worker()->ExpectPendingCommits({kHash1});
@@ -601,8 +626,8 @@ TEST_F(SharedModelTypeProcessorTest, LocalCreateItem) {
 // propagated to the error handler.
 TEST_F(SharedModelTypeProcessorTest, ErrorApplyingAck) {
   InitializeToReadyState();
-  WriteItem(kKey1, kValue1);
-  SetServiceError(SyncError::DATATYPE_ERROR);
+  service()->WriteItem(kKey1, kValue1);
+  service()->SetServiceError(SyncError::DATATYPE_ERROR);
   error_handler()->ExpectError(SyncError::DATATYPE_ERROR);
   worker()->AckOnePendingCommit();
 }
@@ -623,7 +648,7 @@ TEST_F(SharedModelTypeProcessorTest, LocalUpdateItemWithOverrides) {
   entity_data->non_unique_name = kKey1;
   entity_data->client_tag_hash = kHash3;
   entity_data->id = kId1;
-  WriteItem(kKey1, std::move(entity_data));
+  service()->WriteItem(kKey1, std::move(entity_data));
 
   EXPECT_EQ(1U, worker()->GetNumPendingCommits());
   ASSERT_FALSE(worker()->HasPendingCommitForHash(kHash3));
@@ -648,7 +673,7 @@ TEST_F(SharedModelTypeProcessorTest, LocalUpdateItemWithOverrides) {
   entity_data->client_tag_hash = kHash3;
   // Make sure ID isn't overwritten either.
   entity_data->id = kId2;
-  WriteItem(kKey1, std::move(entity_data));
+  service()->WriteItem(kKey1, std::move(entity_data));
 
   EXPECT_EQ(2U, worker()->GetNumPendingCommits());
   ASSERT_FALSE(worker()->HasPendingCommitForHash(kHash3));
@@ -673,7 +698,7 @@ TEST_F(SharedModelTypeProcessorTest, LocalUpdateItemWithOverrides) {
 TEST_F(SharedModelTypeProcessorTest, LocalUpdateItem) {
   InitializeToReadyState();
 
-  WriteItem(kKey1, kValue1);
+  service()->WriteItem(kKey1, kValue1);
   EXPECT_EQ(1U, db().metadata_count());
   worker()->ExpectPendingCommits({kHash1});
 
@@ -682,7 +707,7 @@ TEST_F(SharedModelTypeProcessorTest, LocalUpdateItem) {
   const EntityData& data_v1 = request_data_v1.entity.value();
   const EntityMetadata metadata_v1 = db().GetMetadata(kKey1);
 
-  WriteItem(kKey1, kValue2);
+  service()->WriteItem(kKey1, kValue2);
   EXPECT_EQ(1U, db().metadata_count());
   worker()->ExpectPendingCommits({kHash1, kHash1});
 
@@ -725,18 +750,19 @@ TEST_F(SharedModelTypeProcessorTest, LocalUpdateItem) {
 // commit request.
 TEST_F(SharedModelTypeProcessorTest, LocalUpdateItemRedundant) {
   InitializeToReadyState();
-  WriteItem(kKey1, kValue1);
+  service()->WriteItem(kKey1, kValue1);
   EXPECT_EQ(1U, db().metadata_count());
   worker()->ExpectPendingCommits({kHash1});
 
-  WriteItem(kKey1, kValue1);
+  service()->WriteItem(kKey1, kValue1);
   worker()->ExpectPendingCommits({kHash1});
 }
 
 // Thoroughly tests the data generated by a server item creation.
 TEST_F(SharedModelTypeProcessorTest, ServerCreateItem) {
   InitializeToReadyState();
-  worker()->UpdateFromServer(kHash1, GenerateSpecifics(kKey1, kValue1));
+  worker()->UpdateFromServer(kHash1,
+                             service()->GenerateSpecifics(kKey1, kValue1));

[maxbogue, 2016/10/12 23:40:35]

Huh. This one is not obviously related to the service; it's just a static
function on FakeModelTypeService... I'd make a helper function for it, or even
an anonymous one. Or can you just do "using
FakeModelTypeService::GenerateSpecifics;"? I have no idea if that works.

[skym, 2016/10/13 19:21:38]

Oooh, you're right, just calling the static function is way better.
Unfortunately using doesn't work with static functions as far as I can tell.
Will create wrapping functions to keep these small.

   EXPECT_EQ(1U, db().data_count());
   EXPECT_EQ(1U, db().metadata_count());
   EXPECT_EQ(1U, ProcessorEntityCount());
@@ -767,9 +793,10 @@ TEST_F(SharedModelTypeProcessorTest, ServerCreateItem) {
 // propagated to the error handler.
 TEST_F(SharedModelTypeProcessorTest, ErrorApplyingUpdate) {
   InitializeToReadyState();
-  SetServiceError(SyncError::DATATYPE_ERROR);
+  service()->SetServiceError(SyncError::DATATYPE_ERROR);
   error_handler()->ExpectError(SyncError::DATATYPE_ERROR);
-  worker()->UpdateFromServer(kHash1, GenerateSpecifics(kKey1, kValue1));
+  worker()->UpdateFromServer(kHash1,
+                             service()->GenerateSpecifics(kKey1, kValue1));
 }
 
 // Thoroughly tests the data generated by a server item creation.
@@ -782,12 +809,14 @@ TEST_F(SharedModelTypeProcessorTest, ServerUpdateItem) {
   EXPECT_EQ(2U, db().metadata_change_count());
 
   // Redundant update from server doesn't write data but updates metadata.
-  worker()->UpdateFromServer(kHash1, GenerateSpecifics(kKey1, kValue1));
+  worker()->UpdateFromServer(kHash1,
+                             service()->GenerateSpecifics(kKey1, kValue1));
   EXPECT_EQ(1U, db().data_change_count());
   EXPECT_EQ(3U, db().metadata_change_count());
 
   // A reflection (update already received) is ignored completely.
-  worker()->UpdateFromServer(kHash1, GenerateSpecifics(kKey1, kValue1),
+  worker()->UpdateFromServer(kHash1,
+                             service()->GenerateSpecifics(kKey1, kValue1),
                              0 /* version_offset */);
   EXPECT_EQ(1U, db().data_change_count());
   EXPECT_EQ(3U, db().metadata_change_count());
@@ -805,7 +834,7 @@ TEST_F(SharedModelTypeProcessorTest, LocalDeleteItem) {
   EXPECT_EQ(1, metadata_v1.acked_sequence_number());
   EXPECT_EQ(1, metadata_v1.server_version());
 
-  DeleteItem(kKey1);
+  service()->DeleteItem(kKey1);
   EXPECT_EQ(0U, db().data_count());
   // Metadata is not removed until the commit response comes back.
   EXPECT_EQ(1U, db().metadata_count());
@@ -828,7 +857,7 @@ TEST_F(SharedModelTypeProcessorTest, LocalDeleteItem) {
 // response, then getting the commit responses.
 TEST_F(SharedModelTypeProcessorTest, LocalDeleteItemInterleaved) {
   InitializeToReadyState();
-  WriteItem(kKey1, kValue1);
+  service()->WriteItem(kKey1, kValue1);
   worker()->ExpectPendingCommits({kHash1});
   const CommitRequestData& data_v1 =
       worker()->GetLatestPendingCommitForHash(kHash1);
@@ -839,7 +868,7 @@ TEST_F(SharedModelTypeProcessorTest, LocalDeleteItemInterleaved) {
   EXPECT_EQ(0, metadata_v1.acked_sequence_number());
   EXPECT_EQ(kUncommittedVersion, metadata_v1.server_version());
 
-  DeleteItem(kKey1);
+  service()->DeleteItem(kKey1);
   EXPECT_EQ(0U, db().data_count());
   EXPECT_EQ(1U, db().metadata_count());
   EXPECT_EQ(1U, ProcessorEntityCount());
@@ -896,7 +925,7 @@ TEST_F(SharedModelTypeProcessorTest, ServerDeleteItem) {
 // Should have no effect and not crash.
 TEST_F(SharedModelTypeProcessorTest, LocalDeleteUnknown) {
   InitializeToReadyState();
-  DeleteItem(kKey1);
+  service()->DeleteItem(kKey1);
   EXPECT_EQ(0U, db().data_count());
   EXPECT_EQ(0U, db().metadata_count());
   EXPECT_EQ(0U, ProcessorEntityCount());
@@ -920,7 +949,7 @@ TEST_F(SharedModelTypeProcessorTest, TwoIndependentItems) {
   InitializeToReadyState();
   EXPECT_EQ(0U, worker()->GetNumPendingCommits());
 
-  WriteItem(kKey1, kValue1);
+  service()->WriteItem(kKey1, kValue1);
   EXPECT_EQ(1U, db().data_count());
   EXPECT_EQ(1U, db().metadata_count());
   const EntityMetadata metadata1 = db().GetMetadata(kKey1);
@@ -928,7 +957,7 @@ TEST_F(SharedModelTypeProcessorTest, TwoIndependentItems) {
   // There should be one commit request for this item only.
   worker()->ExpectPendingCommits({kHash1});
 
-  WriteItem(kKey2, kValue2);
+  service()->WriteItem(kKey2, kValue2);
   EXPECT_EQ(2U, db().data_count());
   EXPECT_EQ(2U, db().metadata_count());
   const EntityMetadata metadata2 = db().GetMetadata(kKey2);
@@ -949,7 +978,7 @@ TEST_F(SharedModelTypeProcessorTest, TwoIndependentItems) {
 
 TEST_F(SharedModelTypeProcessorTest, ConflictResolutionChangesMatch) {
   InitializeToReadyState();
-  EntitySpecifics specifics = WriteItem(kKey1, kValue1);
+  EntitySpecifics specifics = service()->WriteItem(kKey1, kValue1);
   EXPECT_EQ(1U, db().data_change_count());
   EXPECT_EQ(kValue1, db().GetValue(kKey1));
   EXPECT_EQ(1U, db().metadata_change_count());
@@ -968,10 +997,11 @@ TEST_F(SharedModelTypeProcessorTest, ConflictResolutionChangesMatch) {
 
 TEST_F(SharedModelTypeProcessorTest, ConflictResolutionUseLocal) {
   InitializeToReadyState();
-  EntitySpecifics specifics = WriteItem(kKey1, kValue1);
-  SetConflictResolution(ConflictResolution::UseLocal());
+  EntitySpecifics specifics = service()->WriteItem(kKey1, kValue1);
+  service()->SetConflictResolution(ConflictResolution::UseLocal());
 
-  worker()->UpdateFromServer(kHash1, GenerateSpecifics(kKey1, kValue2));
+  worker()->UpdateFromServer(kHash1,
+                             service()->GenerateSpecifics(kKey1, kValue2));
 
   // Updated metadata but not data; new commit request.
   EXPECT_EQ(1U, db().data_change_count());
@@ -983,9 +1013,10 @@ TEST_F(SharedModelTypeProcessorTest, ConflictResolutionUseLocal) {
 
 TEST_F(SharedModelTypeProcessorTest, ConflictResolutionUseRemote) {
   InitializeToReadyState();
-  WriteItem(kKey1, kValue1);
-  SetConflictResolution(ConflictResolution::UseRemote());
-  worker()->UpdateFromServer(kHash1, GenerateSpecifics(kKey1, kValue2));
+  service()->WriteItem(kKey1, kValue1);
+  service()->SetConflictResolution(ConflictResolution::UseRemote());
+  worker()->UpdateFromServer(kHash1,
+                             service()->GenerateSpecifics(kKey1, kValue2));
 
   // Updated client data and metadata; no new commit request.
   EXPECT_EQ(2U, db().data_change_count());
@@ -997,18 +1028,19 @@ TEST_F(SharedModelTypeProcessorTest, ConflictResolutionUseRemote) {
 
 TEST_F(SharedModelTypeProcessorTest, ConflictResolutionUseNew) {
   InitializeToReadyState();
-  WriteItem(kKey1, kValue1);
-  SetConflictResolution(
-      ConflictResolution::UseNew(GenerateEntityData(kKey1, kValue3)));
+  service()->WriteItem(kKey1, kValue1);
+  service()->SetConflictResolution(ConflictResolution::UseNew(
+      service()->GenerateEntityData(kKey1, kValue3)));

[maxbogue, 2016/10/12 23:40:35]

Same with GenerateEntityData.

[skym, 2016/10/13 19:21:38]

Done.

 
-  worker()->UpdateFromServer(kHash1, GenerateSpecifics(kKey1, kValue2));
+  worker()->UpdateFromServer(kHash1,
+                             service()->GenerateSpecifics(kKey1, kValue2));
   EXPECT_EQ(2U, db().data_change_count());
   EXPECT_EQ(kValue3, db().GetValue(kKey1));
   EXPECT_EQ(2U, db().metadata_change_count());
   EXPECT_EQ(1, db().GetMetadata(kKey1).server_version());
   worker()->ExpectPendingCommits({kHash1, kHash1});
-  worker()->ExpectNthPendingCommit(1, kHash1,
-                                   GenerateSpecifics(kKey1, kValue3));
+  worker()->ExpectNthPendingCommit(
+      1, kHash1, service()->GenerateSpecifics(kKey1, kValue3));
 }
 
 // Test proper handling of disconnect and reconnect.
@@ -1022,13 +1054,13 @@ TEST_F(SharedModelTypeProcessorTest, Disconnect) {
   WriteItemAndAck(kKey1, kValue1);
 
   // The second item has a commit request in progress.
-  WriteItem(kKey2, kValue2);
+  service()->WriteItem(kKey2, kValue2);
   EXPECT_TRUE(worker()->HasPendingCommitForHash(kHash2));
 
   DisconnectSync();
 
   // The third item is added after stopping.
-  WriteItem(kKey3, kValue3);
+  service()->WriteItem(kKey3, kValue3);
 
   // Reconnect.
   OnSyncStarting();
@@ -1057,19 +1089,19 @@ TEST_F(SharedModelTypeProcessorTest, Disable) {
   WriteItemAndAck(kKey1, kValue1);
 
   // The second item has a commit request in progress.
-  WriteItem(kKey2, kValue2);
+  service()->WriteItem(kKey2, kValue2);
   EXPECT_TRUE(worker()->HasPendingCommitForHash(kHash2));
 
-  DisableSync();
+  service()->DisableSync();
+  EXPECT_FALSE(type_processor()->IsTrackingMetadata());
 
   // The third item is added after disable.
-  WriteItem(kKey3, kValue3);
+  service()->WriteItem(kKey3, kValue3);
 
   // Now we re-enable.
-  CreateChangeProcessor();
-  OnMetadataLoaded();
   OnSyncStarting();
   worker()->UpdateFromServer();
+  EXPECT_TRUE(type_processor()->IsTrackingMetadata());
 
   // Once we're ready to commit, all three local items should consider
   // themselves uncommitted and pending for commit.
@@ -1083,7 +1115,7 @@ TEST_F(SharedModelTypeProcessorTest, ReEncryptCommitsWithNewKey) {
   // Commit an item.
   EntitySpecifics specifics1 = WriteItemAndAck(kKey1, kValue1);
   // Create another item and don't wait for its commit response.
-  EntitySpecifics specifics2 = WriteItem(kKey2, kValue2);
+  EntitySpecifics specifics2 = service()->WriteItem(kKey2, kValue2);
   worker()->ExpectPendingCommits({kHash2});
   EXPECT_EQ(1U, db().GetMetadata(kKey1).sequence_number());
   EXPECT_EQ(1U, db().GetMetadata(kKey2).sequence_number());
@@ -1108,7 +1140,7 @@ TEST_F(SharedModelTypeProcessorTest, ReEncryptCommitsWithNewKey) {
 TEST_F(SharedModelTypeProcessorTest, ReEncryptErrorLoadingData) {
   InitializeToReadyState();
   WriteItemAndAck(kKey1, kValue1);
-  SetServiceError(SyncError::DATATYPE_ERROR);
+  service()->SetServiceError(SyncError::DATATYPE_ERROR);
   worker()->UpdateWithEncryptionKey("k1");
   error_handler()->ExpectError(SyncError::DATATYPE_ERROR);
   OnPendingCommitDataLoaded();
@@ -1119,16 +1151,17 @@ TEST_F(SharedModelTypeProcessorTest, ReEncryptUpdatesWithNewKey) {
   InitializeToReadyState();
 
   // Receive an unencrypted update.
-  worker()->UpdateFromServer(kHash1, GenerateSpecifics(kKey1, kValue1));
+  worker()->UpdateFromServer(kHash1,
+                             service()->GenerateSpecifics(kKey1, kValue1));
   ASSERT_EQ(0U, worker()->GetNumPendingCommits());
 
   UpdateResponseDataList update;
   // Receive an entity with old encryption as part of the update.
   update.push_back(worker()->GenerateUpdateData(
-      kHash2, GenerateSpecifics(kKey2, kValue2), 1, "k1"));
+      kHash2, service()->GenerateSpecifics(kKey2, kValue2), 1, "k1"));
   // Receive an entity with up-to-date encryption as part of the update.
   update.push_back(worker()->GenerateUpdateData(
-      kHash3, GenerateSpecifics(kKey3, kValue3), 1, "k2"));
+      kHash3, service()->GenerateSpecifics(kKey3, kValue3), 1, "k2"));
   // Set desired encryption key to k2 to force updates to some items.
   worker()->UpdateWithEncryptionKey("k2", update);
 
@@ -1139,14 +1172,14 @@ TEST_F(SharedModelTypeProcessorTest, ReEncryptUpdatesWithNewKey) {
   worker()->ExpectPendingCommits({kHash2, kHash1});
 
   // Receive a separate update that was encrypted with key k1.
-  worker()->UpdateFromServer(kHash4, GenerateSpecifics(kKey4, kValue1), 1,
-                             "k1");
+  worker()->UpdateFromServer(
+      kHash4, service()->GenerateSpecifics(kKey4, kValue1), 1, "k1");
   // Receipt of updates encrypted with old key also forces a re-encrypt commit.
   worker()->ExpectPendingCommits({kHash2, kHash1, kHash4});
 
   // Receive an update that was encrypted with key k2.
-  worker()->UpdateFromServer(kHash5, GenerateSpecifics(kKey5, kValue1), 1,
-                             "k2");
+  worker()->UpdateFromServer(
+      kHash5, service()->GenerateSpecifics(kKey5, kValue1), 1, "k2");
   // That was the correct key, so no re-encryption is required.
   worker()->ExpectPendingCommits({kHash2, kHash1, kHash4});
 }
@@ -1155,12 +1188,13 @@ TEST_F(SharedModelTypeProcessorTest, ReEncryptUpdatesWithNewKey) {
 TEST_F(SharedModelTypeProcessorTest, ReEncryptConflictResolutionUseLocal) {
   InitializeToReadyState();
   worker()->UpdateWithEncryptionKey("k1");
-  EntitySpecifics specifics = WriteItem(kKey1, kValue1);
+  EntitySpecifics specifics = service()->WriteItem(kKey1, kValue1);
   worker()->ExpectPendingCommits({kHash1});
 
-  SetConflictResolution(ConflictResolution::UseLocal());
+  service()->SetConflictResolution(ConflictResolution::UseLocal());
   // Unencrypted update needs to be re-commited with key k1.
-  worker()->UpdateFromServer(kHash1, GenerateSpecifics(kKey1, kValue2), 1, "");
+  worker()->UpdateFromServer(
+      kHash1, service()->GenerateSpecifics(kKey1, kValue2), 1, "");
 
   // Ensure the re-commit has the correct value.
   EXPECT_EQ(2U, worker()->GetNumPendingCommits());
@@ -1172,11 +1206,11 @@ TEST_F(SharedModelTypeProcessorTest, ReEncryptConflictResolutionUseLocal) {
 TEST_F(SharedModelTypeProcessorTest, ReEncryptConflictResolutionUseRemote) {
   InitializeToReadyState();
   worker()->UpdateWithEncryptionKey("k1");
-  WriteItem(kKey1, kValue1);
+  service()->WriteItem(kKey1, kValue1);
 
-  SetConflictResolution(ConflictResolution::UseRemote());
+  service()->SetConflictResolution(ConflictResolution::UseRemote());
   // Unencrypted update needs to be re-commited with key k1.
-  EntitySpecifics specifics = GenerateSpecifics(kKey1, kValue2);
+  EntitySpecifics specifics = service()->GenerateSpecifics(kKey1, kValue2);
   worker()->UpdateFromServer(kHash1, specifics, 1, "");
 
   // Ensure the re-commit has the correct value.
@@ -1189,17 +1223,18 @@ TEST_F(SharedModelTypeProcessorTest, ReEncryptConflictResolutionUseRemote) {
 TEST_F(SharedModelTypeProcessorTest, ReEncryptConflictResolutionUseNew) {
   InitializeToReadyState();
   worker()->UpdateWithEncryptionKey("k1");
-  WriteItem(kKey1, kValue1);
+  service()->WriteItem(kKey1, kValue1);
 
-  SetConflictResolution(
-      ConflictResolution::UseNew(GenerateEntityData(kKey1, kValue3)));
+  service()->SetConflictResolution(ConflictResolution::UseNew(
+      service()->GenerateEntityData(kKey1, kValue3)));
   // Unencrypted update needs to be re-commited with key k1.
-  worker()->UpdateFromServer(kHash1, GenerateSpecifics(kKey1, kValue2), 1, "");
+  worker()->UpdateFromServer(
+      kHash1, service()->GenerateSpecifics(kKey1, kValue2), 1, "");
 
   // Ensure the re-commit has the correct value.
   EXPECT_EQ(2U, worker()->GetNumPendingCommits());
-  worker()->ExpectNthPendingCommit(1, kHash1,
-                                   GenerateSpecifics(kKey1, kValue3));
+  worker()->ExpectNthPendingCommit(
+      1, kHash1, service()->GenerateSpecifics(kKey1, kValue3));
   EXPECT_EQ(kValue3, db().GetValue(kKey1));
 }
 
@@ -1212,7 +1247,7 @@ TEST_F(SharedModelTypeProcessorTest, ReEncryptConflictWhileLoading) {
   EXPECT_EQ(0U, worker()->GetNumPendingCommits());
 
   // Unencrypted update needs to be re-commited with key k1.
-  EntitySpecifics specifics = GenerateSpecifics(kKey1, kValue2);
+  EntitySpecifics specifics = service()->GenerateSpecifics(kKey1, kValue2);
   worker()->UpdateFromServer(kHash1, specifics, 1, "");
 
   // Ensure the re-commit has the correct value.
@@ -1234,7 +1269,8 @@ TEST_F(SharedModelTypeProcessorTest, IgnoreLocalEncryption) {
   EXPECT_EQ(1U, worker()->GetNumPendingCommits());
   worker()->ExpectNthPendingCommit(0, kHash1, specifics);
 
-  worker()->UpdateFromServer(kHash1, GenerateSpecifics(kKey1, kValue2));
+  worker()->UpdateFromServer(kHash1,
+                             service()->GenerateSpecifics(kKey1, kValue2));
   EXPECT_EQ(1U, worker()->GetNumPendingCommits());
 }
 
@@ -1243,7 +1279,7 @@ TEST_F(SharedModelTypeProcessorTest, IgnoreRemoteEncryption) {
   InitializeToReadyState();
   EntitySpecifics specifics1 = WriteItemAndAck(kKey1, kValue1);
 
-  EntitySpecifics specifics2 = WriteItem(kKey1, kValue2);
+  EntitySpecifics specifics2 = service()->WriteItem(kKey1, kValue2);
   UpdateResponseDataList update;
   update.push_back(worker()->GenerateUpdateData(kHash1, specifics1, 1, "k1"));
   worker()->UpdateWithEncryptionKey("k1", update);
@@ -1255,8 +1291,8 @@ TEST_F(SharedModelTypeProcessorTest, IgnoreRemoteEncryption) {
 // Same as above but with two commit requests before one ack.
 TEST_F(SharedModelTypeProcessorTest, IgnoreRemoteEncryptionInterleaved) {
   InitializeToReadyState();
-  EntitySpecifics specifics1 = WriteItem(kKey1, kValue1);
-  EntitySpecifics specifics2 = WriteItem(kKey1, kValue2);
+  EntitySpecifics specifics1 = service()->WriteItem(kKey1, kValue1);
+  EntitySpecifics specifics2 = service()->WriteItem(kKey1, kValue2);
   worker()->AckOnePendingCommit();
   // kValue1 is now the base value.
   EXPECT_EQ(1U, worker()->GetNumPendingCommits());