[Sync:Sharing] Pass failed commit items to bridge.

We want to propagate failed commit item errors all the way from server
to bridge. This is second step. Propagate failed commit items from
ClientTagBasedModelTypeProcessor to the bridge.

Bug: 1034923
Change-Id: I4fe5d4448ed337b6ab14e04a749765053b62b249
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2010774
Commit-Queue: vitaliii <vitaliii@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733572}

components/sync/model/model_type_sync_bridge.cc

@@ -54,6 +54,11 @@ void ModelTypeSyncBridge::ApplyStopSyncChanges(
   }
 }
 
+void ModelTypeSyncBridge::OnCommitAttemptErrors(
+    const syncer::FailedCommitResponseDataList& error_response_list) {
+  // By default the bridge just ignores failed commit items.
+}
+
 size_t ModelTypeSyncBridge::EstimateSyncOverheadMemoryUsage() const {
   return 0U;
 }

components/sync/model/model_type_sync_bridge.h

@@ -11,6 +11,7 @@
 
 #include "base/callback.h"
 #include "base/optional.h"
+#include "components/sync/engine/non_blocking_sync_common.h"
 #include "components/sync/model/entity_change.h"
 #include "components/sync/model/model_type_change_processor.h"
 
@@ -162,6 +163,11 @@ class ModelTypeSyncBridge {
   virtual void ApplyStopSyncChanges(
       std::unique_ptr<MetadataChangeList> delete_metadata_change_list);
 
+  // Called only when some items in a commit haven't been committed due to an
+  // error.
+  virtual void OnCommitAttemptErrors(
+      const syncer::FailedCommitResponseDataList& error_response_list);
+
   // Returns an estimate of memory usage attributed to sync (that is, excludes
   // the actual model). Because the resulting UMA metrics are often used to
   // compare with the non-USS equivalent implementations (SyncableService), it's

components/sync/model_impl/client_tag_based_model_type_processor.cc

@@ -635,9 +635,13 @@ void ClientTagBasedModelTypeProcessor::OnCommitCompleted(
     entity_kv.second->ClearTransientSyncState();
   }
 
-  // TODO(crbug.com/1034923): Propagate failed items to bridge.
   base::Optional<ModelError> error = bridge_->ApplySyncChanges(
       std::move(metadata_change_list), std::move(entity_change_list));
+
+  if (!error_response_list.empty()) {
+    bridge_->OnCommitAttemptErrors(error_response_list);
+  }
+
   if (error) {
     ReportError(*error);
   }

components/sync/model_impl/client_tag_based_model_type_processor_unittest.cc

@@ -11,6 +11,7 @@
 #include <vector>
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/run_loop.h"
 #include "base/test/metrics/histogram_tester.h"
@@ -21,6 +22,7 @@
 #include "components/sync/base/sync_mode.h"
 #include "components/sync/base/time.h"
 #include "components/sync/engine/data_type_activation_response.h"
+#include "components/sync/engine/non_blocking_sync_common.h"
 #include "components/sync/model/conflict_resolution.h"
 #include "components/sync/model/data_type_activation_request.h"
 #include "components/sync/model/fake_model_type_sync_bridge.h"
@@ -165,6 +167,18 @@ class TestModelTypeSyncBridge : public FakeModelTypeSyncBridge {
     }
   }
 
+  void OnCommitAttemptErrors(
+      const FailedCommitResponseDataList& error_response_list) override {
+    if (!on_commit_attempt_errors_callback_.is_null()) {
+      std::move(on_commit_attempt_errors_callback_).Run(error_response_list);
+    }
+  }
+
+  void SetOnCommitAttemptErrorsCallback(
+      base::OnceCallback<void(const FailedCommitResponseDataList&)> callback) {
+    on_commit_attempt_errors_callback_ = std::move(callback);
+  }
+
  private:
   void CaptureDataCallback(DataCallback callback,
                            std::unique_ptr<DataBatch> data) {
@@ -186,6 +200,9 @@ class TestModelTypeSyncBridge : public FakeModelTypeSyncBridge {
   // Whether to return GetData results synchronously. Overrides the default
   // callback capture behavior if set to true.
   bool synchronous_data_callback_ = false;
+
+  base::OnceCallback<void(const FailedCommitResponseDataList&)>
+      on_commit_attempt_errors_callback_;
 };
 
 }  // namespace
@@ -2429,6 +2446,57 @@ TEST_F(ClientTagBasedModelTypeProcessorTest,
   EXPECT_TRUE(type_processor()->IsTrackingEntityForTest(kStorageKey2));
 }
 
+TEST_F(ClientTagBasedModelTypeProcessorTest,
+       ShouldPropagateFailedCommitItemsToBridgeWhenCommitCompleted) {
+  FailedCommitResponseData response_data;
+  response_data.client_tag_hash = GetHash("dummy tag");
+  response_data.response_type = sync_pb::CommitResponse::TRANSIENT_ERROR;
+
+  FailedCommitResponseDataList failed_list;
+  failed_list.push_back(response_data);
+
+  FailedCommitResponseDataList actual_error_response_list;
+
+  auto on_commit_attempt_errors_callback = base::BindOnce(
+      [](FailedCommitResponseDataList* actual_error_response_list,
+         const FailedCommitResponseDataList& error_response_list) {
+        // We put expectations outside of the callback, so that they fail if
+        // callback is not ran.
+        *actual_error_response_list = error_response_list;
+      },
+      &actual_error_response_list);
+
+  bridge()->SetOnCommitAttemptErrorsCallback(
+      std::move(on_commit_attempt_errors_callback));
+
+  type_processor()->OnCommitCompleted(
+      model_type_state(),
+      /*committed_response_list=*/CommitResponseDataList(), failed_list);
+
+  ASSERT_EQ(1u, actual_error_response_list.size());
+  EXPECT_EQ(response_data.client_tag_hash,
+            actual_error_response_list[0].client_tag_hash);
+  EXPECT_EQ(response_data.response_type,
+            actual_error_response_list[0].response_type);
+}
+
+TEST_F(ClientTagBasedModelTypeProcessorTest,
+       ShouldNotPropagateFailedCommitAttemptToBridgeWhenNoFailedItems) {
+  auto on_commit_attempt_errors_callback = base::BindOnce(
+      [](const FailedCommitResponseDataList& error_response_list) {
+        ADD_FAILURE()
+            << "OnCommitAttemptErrors is called when no failed items.";
+      });
+
+  bridge()->SetOnCommitAttemptErrorsCallback(
+      std::move(on_commit_attempt_errors_callback));
+
+  type_processor()->OnCommitCompleted(
+      model_type_state(),
+      /*committed_response_list=*/CommitResponseDataList(),
+      /*rror_response_list=*/FailedCommitResponseDataList());
+}
+
 class CommitOnlyClientTagBasedModelTypeProcessorTest
     : public ClientTagBasedModelTypeProcessorTest {
  protected: