[Sync] Do not turn off Sharing message data type when backed off

The data type might stop with clearing all data on any server-side error
(even transient). This might happen because the data type will be backed
off on any server-side error. In some cases it's not desired behaviour,
e.g. if there is a transient AUTH_ERROR which is transient and the next
commit request most probably will succeed.

Originally it was introduced to detect internet connection problems or
other error during commit. However there is already a mechanism to
detect if there is no internet connection. Additionally, there are more
cases when the client gets a transient error and it would be better to
try to send the message again.

Bug: 1097054
Change-Id: Ibfe4fe2d5d2614965a8c105017bb943447316ba4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2360076Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Rushan Suleymanov <rushans@google.com>
Cr-Commit-Position: refs/heads/master@{#798606}

chrome/browser/sharing/sharing_message_model_type_controller.cc

@@ -36,9 +36,6 @@ SharingMessageModelTypeController::GetPreconditionState() const {
   if (syncer::IsWebSignout(sync_service_->GetAuthError())) {
     return PreconditionState::kMustStopAndClearData;
   }
-  if (sync_service_->GetBackedOffDataTypes().Has(syncer::SHARING_MESSAGE)) {
-    return PreconditionState::kMustStopAndClearData;
-  }
   return PreconditionState::kPreconditionsMet;
 }
 

chrome/browser/sync/test/integration/single_client_sharing_message_sync_test.cc

@@ -43,16 +43,6 @@ class NextCycleIterationChecker : public SingleClientStatusChangeChecker {
   base::Time last_synced_time_;
 };
 
-class BackedOffSharingMessageChecker : public SingleClientStatusChangeChecker {
- public:
-  explicit BackedOffSharingMessageChecker(syncer::ProfileSyncService* service)
-      : SingleClientStatusChangeChecker(service) {}
-
-  bool IsExitConditionSatisfied(std::ostream* os) override {
-    return service()->GetBackedOffDataTypes().Has(syncer::SHARING_MESSAGE);
-  }
-};
-
 class SharingMessageEqualityChecker : public SingleClientStatusChangeChecker {
  public:
   SharingMessageEqualityChecker(
@@ -192,39 +182,6 @@ IN_PROC_BROWSER_TEST_F(SingleClientSharingMessageSyncTest,
   ASSERT_TRUE(NextCycleIterationChecker(GetSyncService(0)).Wait());
 }
 
-IN_PROC_BROWSER_TEST_F(SingleClientSharingMessageSyncTest,
-                       ShouldStopDataTypeWhenBackedOff) {
-  base::MockRepeatingCallback<void(const sync_pb::SharingMessageCommitError&)>
-      callback;
-
-  ASSERT_TRUE(SetupSync());
-  SharingMessageBridge* sharing_message_bridge =
-      SharingMessageBridgeFactory::GetForBrowserContext(GetProfile(0));
-  SharingMessageSpecifics specifics;
-  specifics.set_payload("payload");
-  sharing_message_bridge->SendSharingMessage(
-      std::make_unique<SharingMessageSpecifics>(specifics), callback.Get());
-
-  EXPECT_CALL(
-      callback,
-      Run(HasErrorCode(sync_pb::SharingMessageCommitError::SYNC_TURNED_OFF)));
-  ASSERT_FALSE(
-      GetSyncService(0)->GetBackedOffDataTypes().Has(syncer::SHARING_MESSAGE));
-
-  // Run the data type into backed off state before the message gets sent.
-  fake_server::FakeServerHttpPostProvider::DisableNetwork();
-
-  ASSERT_TRUE(BackedOffSharingMessageChecker(GetSyncService(0)).Wait());
-
-  EXPECT_TRUE(
-      GetSyncService(0)->GetBackedOffDataTypes().Has(syncer::SHARING_MESSAGE));
-  EXPECT_CALL(
-      callback,
-      Run(HasErrorCode(sync_pb::SharingMessageCommitError::SYNC_TURNED_OFF)));
-  sharing_message_bridge->SendSharingMessage(
-      std::make_unique<SharingMessageSpecifics>(specifics), callback.Get());
-}
-
 IN_PROC_BROWSER_TEST_F(SingleClientSharingMessageSyncTest,
                        ShouldCleanPendingMessagesAfterSyncPaused) {
   base::MockOnceCallback<void(const sync_pb::SharingMessageCommitError&)>