Leave sync chain after 5 seconds without ack response

fixes brave/brave-browser#18054

chromium_src/components/sync_device_info/device_info_sync_bridge.cc

@@ -25,6 +25,8 @@ namespace syncer {
 
 namespace {
 
+const int kFailedAttemtpsToAckDeviceDelete = 5;
+
 std::unique_ptr<BraveDeviceInfo> BraveSpecificsToModel(
     const DeviceInfoSpecifics& specifics) {
   ModelTypeSet data_types;
@@ -67,19 +69,21 @@ void DeviceInfoSyncBridge::DeleteDeviceInfo(const std::string& client_id,
   base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
       FROM_HERE,
       base::BindOnce(&DeviceInfoSyncBridge::OnDeviceInfoDeleted,
-                     weak_ptr_factory_.GetWeakPtr(), client_id,
+                     weak_ptr_factory_.GetWeakPtr(), client_id, 1,
                      std::move(callback)),
       base::TimeDelta::FromSeconds(1));
 }
 
 void DeviceInfoSyncBridge::OnDeviceInfoDeleted(const std::string& client_id,
+                                               const int attempt,
                                                base::OnceClosure callback) {
   // Make sure the deleted device info is sent
-  if (change_processor()->IsEntityUnsynced(client_id)) {
+  if (change_processor()->IsEntityUnsynced(client_id) &&
+      attempt < kFailedAttemtpsToAckDeviceDelete) {
     base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
         FROM_HERE,
         base::BindOnce(&DeviceInfoSyncBridge::OnDeviceInfoDeleted,
-                       weak_ptr_factory_.GetWeakPtr(), client_id,
+                       weak_ptr_factory_.GetWeakPtr(), client_id, attempt + 1,
                        std::move(callback)),
         base::TimeDelta::FromSeconds(1));
   } else {

chromium_src/components/sync_device_info/device_info_sync_bridge.h

@@ -12,18 +12,18 @@
 #define ForcePulseForTest                                                    \
   DeleteDeviceInfo(const std::string& client_id, base::OnceClosure callback) \
       override;                                                              \
-  std::vector<std::unique_ptr<BraveDeviceInfo>>                              \
-  GetAllBraveDeviceInfo() const override;                                    \
+  std::vector<std::unique_ptr<BraveDeviceInfo>> GetAllBraveDeviceInfo()      \
+      const override;                                                        \
   void ForcePulseForTest
 
 #define RefreshLocalDeviceInfoIfNeeded           \
   RefreshLocalDeviceInfoIfNeeded_ChromiumImpl(); \
   void RefreshLocalDeviceInfoIfNeeded
 
 // private:
-#define StoreSpecifics                              \
-  OnDeviceInfoDeleted(const std::string& client_id, \
-                      base::OnceClosure callback);  \
+#define StoreSpecifics                                                 \
+  OnDeviceInfoDeleted(const std::string& client_id, const int attempt, \
+                      base::OnceClosure callback);                     \
   void StoreSpecifics
 
 #include "../../../../components/sync_device_info/device_info_sync_bridge.h"

chromium_src/components/sync_device_info/device_info_sync_bridge_unittest.cc

@@ -4,9 +4,39 @@
 // you can obtain one at http://mozilla.org/MPL/2.0/.
 
 #include "base/system/sys_info.h"
+#include "base/test/task_environment.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace base {
+namespace test {
+namespace {
+
+// This trick substitutes TaskEnvironment to make the test
+// LocalDeleteWhenOffline avoid wait 6 sec for real
+class TaskEnvironmentOptionalMockTime : public TaskEnvironment {
+ public:
+  TaskEnvironmentOptionalMockTime()
+      : TaskEnvironment(
+            IsMockTimedTest(
+                testing::UnitTest::GetInstance()->current_test_info()->name())
+                ? TimeSource::MOCK_TIME
+                : TimeSource::DEFAULT) {}
+
+  static bool IsMockTimedTest(base::StringPiece test_name) {
+    return test_name == "LocalDeleteWhenOffline";
+  }
+};
+
+}  // namespace
+}  // namespace test
+}  // namespace base
+
+#define TaskEnvironment TaskEnvironmentOptionalMockTime
 
 #include "../../../../components/sync_device_info/device_info_sync_bridge_unittest.cc"
 
+#undef TaskEnvironment
+
 namespace syncer {
 namespace {
 
@@ -84,5 +114,46 @@ TEST_F(DeviceInfoSyncBridgeTest, RemoteDelete) {
   EXPECT_THAT(ReadAllFromStore(), UnorderedElementsAre(Pair(kLocalGuid, _)));
 }
 
+TEST_F(DeviceInfoSyncBridgeTest, LocalDeleteWhenOffline) {
+  InitializeAndMergeInitialData(SyncMode::kFull);
+  ASSERT_EQ(1u, bridge()->GetAllDeviceInfo().size());
+  ASSERT_EQ(1, change_count());
+  ASSERT_FALSE(ReadAllFromStore().empty());
+
+  const DeviceInfoSpecifics specifics = CreateSpecifics(1, base::Time::Now());
+  auto error = bridge()->ApplySyncChanges(bridge()->CreateMetadataChangeList(),
+                                          EntityAddList({specifics}));
+
+  ASSERT_FALSE(error);
+  ASSERT_EQ(2u, bridge()->GetAllDeviceInfo().size());
+  ASSERT_EQ(2, change_count());
+
+  const std::string kLocalGuid = CacheGuidForSuffix(kLocalSuffix);
+  ON_CALL(*processor(), IsEntityUnsynced(kLocalGuid))
+      .WillByDefault(Return(true));
+  // The statement below means that DeviceInfoSyncBridge::OnDeviceInfoDeleted
+  // 5 times did check of IsEntityUnsynced for the entity being deleted
+  EXPECT_CALL(*processor(), IsEntityUnsynced).Times(5);
+  EXPECT_CALL(*processor(), Delete(kLocalGuid, _)).Times(1);
+
+  bool deleted_device_info_sent = false;
+  base::RunLoop loop;
+  bridge()->DeleteDeviceInfo(
+      kLocalGuid, base::BindOnce(
+                      [](base::RunLoop* loop, bool* deleted_device_info_sent) {
+                        *deleted_device_info_sent = true;
+                        loop->Quit();
+                      },
+                      &loop, &deleted_device_info_sent));
+
+  loop.Run();
+
+  EXPECT_TRUE(deleted_device_info_sent);
+  EXPECT_EQ(1u, bridge()->GetAllDeviceInfo().size());
+  ASSERT_EQ(3, change_count());
+  EXPECT_THAT(ReadAllFromStore(),
+              UnorderedElementsAre(Pair(specifics.cache_guid(), _)));
+}
+
 }  // namespace
 }  // namespace syncer