[Sync] Support commit only types.

components/sync/engine_impl/model_type_worker_unittest.cc

Index: components/sync/engine_impl/model_type_worker_unittest.cc
diff --git a/components/sync/engine_impl/model_type_worker_unittest.cc b/components/sync/engine_impl/model_type_worker_unittest.cc
index cc4fd1bd2ff812e13ede75e9229d9a52bf6ddd15..fa5b2e9e41c328f600a6962812ff3739a06cecf7 100644
--- a/components/sync/engine_impl/model_type_worker_unittest.cc
+++ b/components/sync/engine_impl/model_type_worker_unittest.cc
@@ -131,9 +131,11 @@ class ModelTypeWorkerTest : public ::testing::Test {
       : foreign_encryption_key_index_(0),
         update_encryption_filter_index_(0),
         mock_type_processor_(nullptr),
-        mock_server_(kModelType),
+        mock_server_(base::MakeUnique<SingleTypeMockServer>(kModelType)),
         is_processor_disconnected_(false),
-        preferences_emitter_(kModelType, &type_observers_) {}
+        emitter_(base::MakeUnique<NonBlockingTypeDebugInfoEmitter>(
+            kModelType,
+            &type_observers_)) {}
 
   ~ModelTypeWorkerTest() override {}
 
@@ -148,7 +150,7 @@ class ModelTypeWorkerTest : public ::testing::Test {
     initial_state.mutable_progress_marker()->set_data_type_id(
         GetSpecificsFieldNumberFromModelType(kModelType));
 
-    InitializeWithState(initial_state, UpdateResponseDataList());
+    InitializeWithState(kModelType, initial_state, UpdateResponseDataList());
   }
 
   // Initializes with some existing data type state. Allows us to start
@@ -168,13 +170,26 @@ class ModelTypeWorkerTest : public ::testing::Test {
 
     initial_state.set_initial_sync_done(true);
 
-    InitializeWithState(initial_state, initial_pending_updates);
+    InitializeWithState(kModelType, initial_state, initial_pending_updates);
 
     mock_nudge_handler_.ClearCounters();
   }
 
+  void InitializeCommitOnly() {
+    mock_server_ = base::MakeUnique<SingleTypeMockServer>(USER_EVENTS);
+    emitter_ = base::MakeUnique<NonBlockingTypeDebugInfoEmitter>(
+        USER_EVENTS, &type_observers_);
+
+    // Don't set progress marker, commit only types don't use them.
+    sync_pb::ModelTypeState initial_state;
+    initial_state.set_initial_sync_done(true);
+
+    InitializeWithState(USER_EVENTS, initial_state, UpdateResponseDataList());
+  }
+
   // Initialize with a custom initial ModelTypeState and pending updates.
   void InitializeWithState(
+      const ModelType type,
       const sync_pb::ModelTypeState& state,
       const UpdateResponseDataList& initial_pending_updates) {
     DCHECK(!worker_);
@@ -192,9 +207,8 @@ class ModelTypeWorkerTest : public ::testing::Test {
 
     // TODO(maxbogue): crbug.com/529498: Inject pending updates somehow.
     worker_ = base::MakeUnique<ModelTypeWorker>(
-        kModelType, state, !state.initial_sync_done(),
-        std::move(cryptographer_copy), &mock_nudge_handler_,
-        std::move(processor), &preferences_emitter_);
+        type, state, !state.initial_sync_done(), std::move(cryptographer_copy),
+        &mock_nudge_handler_, std::move(processor), emitter_.get());
   }
 
   // Introduce a new key that the local cryptographer can't decrypt.
@@ -268,10 +282,13 @@ class ModelTypeWorkerTest : public ::testing::Test {
   // Modifications on the model thread that get sent to the worker under test.
 
   void CommitRequest(const std::string& name, const std::string& value) {
-    const std::string tag_hash = GenerateTagHash(name);
-    CommitRequestData data = mock_type_processor_->CommitRequest(
-        tag_hash, GenerateSpecifics(name, value));
-    worker_->EnqueueForCommit({data});
+    CommitRequest(GenerateTagHash(name), GenerateSpecifics(name, value));
+  }
+
+  void CommitRequest(const std::string& tag_hash,
+                     const sync_pb::EntitySpecifics& specifics) {
+    worker_->EnqueueForCommit(
+        {mock_type_processor_->CommitRequest(tag_hash, specifics)});
   }
 
   void DeleteRequest(const std::string& tag) {
@@ -283,9 +300,9 @@ class ModelTypeWorkerTest : public ::testing::Test {
   // Pretend to receive update messages from the server.
 
   void TriggerTypeRootUpdateFromServer() {
-    sync_pb::SyncEntity entity = mock_server_.TypeRootUpdate();
-    worker_->ProcessGetUpdatesResponse(mock_server_.GetProgress(),
-                                       mock_server_.GetContext(), {&entity},
+    sync_pb::SyncEntity entity = mock_server_->TypeRootUpdate();
+    worker_->ProcessGetUpdatesResponse(mock_server_->GetProgress(),
+                                       mock_server_->GetContext(), {&entity},
                                        nullptr);
     worker_->PassiveApplyUpdates(nullptr);
   }
@@ -293,7 +310,7 @@ class ModelTypeWorkerTest : public ::testing::Test {
   void TriggerPartialUpdateFromServer(int64_t version_offset,
                                       const std::string& tag,
                                       const std::string& value) {
-    sync_pb::SyncEntity entity = mock_server_.UpdateFromServer(
+    sync_pb::SyncEntity entity = mock_server_->UpdateFromServer(
         version_offset, GenerateTagHash(tag), GenerateSpecifics(tag, value));
 
     if (update_encryption_filter_index_ != 0) {
@@ -301,8 +318,8 @@ class ModelTypeWorkerTest : public ::testing::Test {
                     entity.mutable_specifics());
     }
 
-    worker_->ProcessGetUpdatesResponse(mock_server_.GetProgress(),
-                                       mock_server_.GetContext(), {&entity},
+    worker_->ProcessGetUpdatesResponse(mock_server_->GetProgress(),
+                                       mock_server_->GetContext(), {&entity},
                                        nullptr);
   }
 
@@ -316,15 +333,15 @@ class ModelTypeWorkerTest : public ::testing::Test {
   void TriggerTombstoneFromServer(int64_t version_offset,
                                   const std::string& tag) {
     sync_pb::SyncEntity entity =
-        mock_server_.TombstoneFromServer(version_offset, GenerateTagHash(tag));
+        mock_server_->TombstoneFromServer(version_offset, GenerateTagHash(tag));
 
     if (update_encryption_filter_index_ != 0) {
       EncryptUpdate(GetNthKeyParams(update_encryption_filter_index_),
                     entity.mutable_specifics());
     }
 
-    worker_->ProcessGetUpdatesResponse(mock_server_.GetProgress(),
-                                       mock_server_.GetContext(), {&entity},
+    worker_->ProcessGetUpdatesResponse(mock_server_->GetProgress(),
+                                       mock_server_->GetContext(), {&entity},
                                        nullptr);
     worker_->ApplyUpdates(nullptr);
   }
@@ -340,7 +357,7 @@ class ModelTypeWorkerTest : public ::testing::Test {
   // protocol. Try to use the other, higher level methods if possible.
   void DeliverRawUpdates(const SyncEntityList& list) {
     worker_->ProcessGetUpdatesResponse(
-        mock_server_.GetProgress(), mock_server_.GetContext(), list, nullptr);
+        mock_server_->GetProgress(), mock_server_->GetContext(), list, nullptr);
     worker_->ApplyUpdates(nullptr);
   }
 
@@ -384,7 +401,7 @@ class ModelTypeWorkerTest : public ::testing::Test {
     contribution->AddToCommitMessage(&message);
 
     sync_pb::ClientToServerResponse response =
-        mock_server_.DoSuccessfulCommit(message);
+        mock_server_->DoSuccessfulCommit(message);
 
     contribution->ProcessCommitResponse(response, nullptr);
     contribution->CleanUp();
@@ -411,7 +428,7 @@ class ModelTypeWorkerTest : public ::testing::Test {
   }
 
   // Returns the name of the encryption key in the cryptographer last passed to
-  // the CommitQueue. Returns an empty string if no crypgorapher is
+  // the CommitQueue. Returns an empty string if no cryptographer is
   // in use. See also: DecryptPendingKey().
   std::string GetLocalCryptographerKeyName() const {
     if (!cryptographer_) {
@@ -422,8 +439,8 @@ class ModelTypeWorkerTest : public ::testing::Test {
 
   MockModelTypeProcessor* processor() { return mock_type_processor_; }
   ModelTypeWorker* worker() { return worker_.get(); }
-  SingleTypeMockServer* server() { return &mock_server_; }
-  NonBlockingTypeDebugInfoEmitter* emitter() { return &preferences_emitter_; }
+  SingleTypeMockServer* server() { return mock_server_.get(); }
+  NonBlockingTypeDebugInfoEmitter* emitter() { return emitter_.get(); }
 
  private:
   // An encryptor for our cryptographer.
@@ -450,7 +467,7 @@ class ModelTypeWorkerTest : public ::testing::Test {
   // A mock that emulates enough of the sync server that it can be used
   // a single UpdateHandler and CommitContributor pair. In this test
   // harness, the |worker_| is both of them.
-  SingleTypeMockServer mock_server_;
+  std::unique_ptr<SingleTypeMockServer> mock_server_;
 
   // A mock to track the number of times the CommitQueue requests to
   // sync.
@@ -460,7 +477,7 @@ class ModelTypeWorkerTest : public ::testing::Test {
 
   base::ObserverList<TypeDebugInfoObserver> type_observers_;
 
-  NonBlockingTypeDebugInfoEmitter preferences_emitter_;
+  std::unique_ptr<NonBlockingTypeDebugInfoEmitter> emitter_;
 };
 
 // Requests a commit and verifies the messages sent to the client and server as
@@ -1177,4 +1194,43 @@ TEST_F(ModelTypeWorkerTest, RecreateDeletedEntity) {
   }
 }
 
+TEST_F(ModelTypeWorkerTest, CommitOnly) {
+  InitializeCommitOnly();
+
+  sync_pb::EntitySpecifics specifics;
+  specifics.mutable_user_event();

[Patrick Noland, 2017/06/01 21:34:40]

Do you not want to put any data into this?

[skym, 2017/06/01 22:43:20]

Done.

+  CommitRequest(kHash1, specifics);
+
+  EXPECT_EQ(1, GetNumCommitNudges());
+
+  ASSERT_TRUE(WillCommit());
+  DoSuccessfulCommit();
+
+  ASSERT_EQ(1U, server()->GetNumCommitMessages());
+  EXPECT_EQ(1, server()->GetNthCommitMessage(0).commit().entries_size());
+  const sync_pb::SyncEntity entity =
+      server()->GetNthCommitMessage(0).commit().entries(0);
+
+  EXPECT_EQ(0, entity.attachment_id_size());
+  EXPECT_FALSE(entity.has_ctime());
+  EXPECT_FALSE(entity.has_deleted());
+  EXPECT_FALSE(entity.has_folder());
+  EXPECT_FALSE(entity.has_id_string());
+  EXPECT_FALSE(entity.has_mtime());
+  EXPECT_FALSE(entity.has_version());
+  EXPECT_FALSE(entity.has_name());
+  EXPECT_TRUE(entity.specifics().has_user_event());
+
+  EXPECT_EQ(1, emitter()->GetCommitCounters().num_commits_attempted);

[Patrick Noland, 2017/06/01 21:34:39]

This process(verifying the commit counters and responses) seems kind of
repetitive, is there a good way to streamline it?

[skym, 2017/06/01 22:43:20]

Done.

+  EXPECT_EQ(1, emitter()->GetCommitCounters().num_commits_success);
+
+  ASSERT_EQ(1U, processor()->GetNumCommitResponses());
+  EXPECT_EQ(1U, processor()->GetNthCommitResponse(0).size());
+  ASSERT_TRUE(processor()->HasCommitResponse(kHash1));
+  const CommitResponseData& commit_response =
+      processor()->GetCommitResponse(kHash1);
+  EXPECT_EQ(kHash1, commit_response.client_tag_hash);
+  EXPECT_FALSE(commit_response.specifics_hash.empty());
+}
+
 }  // namespace syncer