[Sync::USS] Implement NigoriModelTypeProcessor::NudgeForCommitIfNeeded

Bug: 922900
Change-Id: Idde575a3f4ba559811ab98373f6fb3e30bef0ea7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1572406
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652205}

components/sync/nigori/nigori_model_type_processor.cc

@@ -31,8 +31,7 @@ void NigoriModelTypeProcessor::ConnectSync(
   DVLOG(1) << "Successfully connected Encryption Keys";
 
   worker_ = std::move(worker);
-  // TODO(mamir): NudgeForCommitIfNeeded();
-  NOTIMPLEMENTED();
+  NudgeForCommitIfNeeded();
 }
 
 void NigoriModelTypeProcessor::DisconnectSync() {
@@ -180,8 +179,7 @@ void NigoriModelTypeProcessor::Put(std::unique_ptr<EntityData> entity_data) {
   }
 
   entity_->MakeLocalChange(std::move(entity_data));
-  // TODO(mamir): NudgeForCommitIfNeeded();
-  NOTIMPLEMENTED();
+  NudgeForCommitIfNeeded();
 }
 
 NigoriMetadataBatch NigoriModelTypeProcessor::GetMetadata() {
@@ -205,4 +203,22 @@ bool NigoriModelTypeProcessor::IsConnected() const {
   return worker_ != nullptr;
 }
 
+void NigoriModelTypeProcessor::NudgeForCommitIfNeeded() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  // Don't bother sending anything if there's no one to send to.
+  if (!IsConnected()) {
+    return;
+  }
+
+  // Don't send anything if the type is not ready to handle commits.
+  if (!model_type_state_.initial_sync_done()) {
+    return;
+  }
+
+  // Nudge worker if there are any entities with local changes.
+  if (entity_->RequiresCommitRequest()) {
+    worker_->NudgeForCommit();
+  }
+}
+
 }  // namespace syncer

components/sync/nigori/nigori_model_type_processor.h

@@ -57,6 +57,9 @@ class NigoriModelTypeProcessor : public ModelTypeProcessor,
   // Returns true if the handshake with sync thread is complete.
   bool IsConnected() const;
 
+  // Nudges worker if there are any local changes to be committed.
+  void NudgeForCommitIfNeeded() const;
+
   // The bridge owns this processor instance so the pointer should never become
   // invalid.
   NigoriSyncBridge* bridge_;

components/sync/nigori/nigori_model_type_processor_unittest.cc

@@ -9,6 +9,7 @@
 
 #include "base/bind.h"
 #include "components/sync/base/time.h"
+#include "components/sync/engine/commit_queue.h"
 #include "components/sync/nigori/nigori_sync_bridge.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -44,9 +45,20 @@ class MockNigoriSyncBridge : public NigoriSyncBridge {
   MOCK_METHOD0(ApplyDisableSyncChanges, void());
 };
 
+class MockCommitQueue : public CommitQueue {
+ public:
+  MockCommitQueue() = default;
+  ~MockCommitQueue() = default;
+
+  MOCK_METHOD0(NudgeForCommit, void());
+};
+
 class NigoriModelTypeProcessorTest : public testing::Test {
  public:
-  NigoriModelTypeProcessorTest() = default;
+  NigoriModelTypeProcessorTest() {
+    mock_commit_queue_ = std::make_unique<testing::NiceMock<MockCommitQueue>>();
+    mock_commit_queue_ptr_ = mock_commit_queue_.get();
+  }
 
   void SimulateModelReadyToSyncWithInitialSyncDone() {
     NigoriMetadataBatch nigori_metadata_batch;
@@ -60,14 +72,22 @@ class NigoriModelTypeProcessorTest : public testing::Test {
                                 std::move(nigori_metadata_batch));
   }
 
+  void SimulateConnectSync() {
+    processor_.ConnectSync(std::move(mock_commit_queue_));
+  }
+
   MockNigoriSyncBridge* mock_nigori_sync_bridge() {
     return &mock_nigori_sync_bridge_;
   }
 
+  MockCommitQueue* mock_commit_queue() { return mock_commit_queue_ptr_; }
+
   NigoriModelTypeProcessor* processor() { return &processor_; }
 
  private:
   testing::NiceMock<MockNigoriSyncBridge> mock_nigori_sync_bridge_;
+  std::unique_ptr<testing::NiceMock<MockCommitQueue>> mock_commit_queue_;
+  MockCommitQueue* mock_commit_queue_ptr_;
   NigoriModelTypeProcessor processor_;
 };
 
@@ -147,6 +167,32 @@ TEST_F(NigoriModelTypeProcessorTest, ShouldGetLocalChangesWhenPut) {
   EXPECT_EQ(kNigoriNonUniqueName, commit_request[0]->entity->non_unique_name);
 }
 
+TEST_F(NigoriModelTypeProcessorTest,
+       ShouldNudgeForCommitUponConnectSyncIfReadyToSyncAndLocalChanges) {
+  SimulateModelReadyToSyncWithInitialSyncDone();
+
+  auto entity_data = std::make_unique<syncer::EntityData>();
+  entity_data->specifics.mutable_nigori();
+  entity_data->non_unique_name = kNigoriNonUniqueName;
+
+  processor()->Put(std::move(entity_data));
+
+  EXPECT_CALL(*mock_commit_queue(), NudgeForCommit());
+  SimulateConnectSync();
+}
+
+TEST_F(NigoriModelTypeProcessorTest, ShouldNudgeForCommitUponPutIfReadyToSync) {
+  SimulateModelReadyToSyncWithInitialSyncDone();
+  SimulateConnectSync();
+
+  auto entity_data = std::make_unique<syncer::EntityData>();
+  entity_data->specifics.mutable_nigori();
+  entity_data->non_unique_name = kNigoriNonUniqueName;
+
+  EXPECT_CALL(*mock_commit_queue(), NudgeForCommit());
+  processor()->Put(std::move(entity_data));
+}
+
 }  // namespace
 
 }  // namespace syncer