Sync reset reenable (#3038)

brave · Aug 19, 2019 · 91e5d05 · 91e5d05
1 parent 97d0eca
commit 91e5d05
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 28 deletions.
diff --git a/components/brave_sync/brave_profile_sync_service_impl.cc b/components/brave_sync/brave_profile_sync_service_impl.cc
@@ -260,11 +260,14 @@ void BraveProfileSyncServiceImpl::OnSetupSyncHaveCode(
     return;
   }
 
-  if (brave_sync_configured_) {
+  if (IsBraveSyncConfigured()) {
     NotifyLogMessage("already configured");
     return;
   }
 
+  ForceCompleteReset();
+  DCHECK(!brave_sync_prefs_->GetSyncEnabled());
+
   if (device_name.empty())
     brave_sync_prefs_->SetThisDeviceName(GetDeviceName());
   else
@@ -284,11 +287,14 @@ void BraveProfileSyncServiceImpl::OnSetupSyncNewToSync(
     return;
   }
 
-  if (brave_sync_configured_) {
+  if (IsBraveSyncConfigured()) {
     NotifyLogMessage("already configured");
     return;
   }
 
+  ForceCompleteReset();
+  DCHECK(!brave_sync_prefs_->GetSyncEnabled());
+
   // If the previous attempt was connect to sync chain
   // and failed to receive save-init-data
   brave_sync_words_.clear();
@@ -329,6 +335,7 @@ void BraveProfileSyncServiceImpl::OnResetSync() {
     // we can reset it by ResetInternal()
     const std::string device_id = brave_sync_prefs_->GetThisDeviceId();
     OnDeleteDevice(device_id);
+    reseting_ = true;
   }
 }
 
@@ -636,6 +643,14 @@ void BraveProfileSyncServiceImpl::ResetSyncInternal() {
   brave_sync_initialized_ = false;
 
   brave_sync_prefs_->SetSyncEnabled(false);
+
+  reseting_ = false;
+}
+
+void BraveProfileSyncServiceImpl::ForceCompleteReset() {
+  if (reseting_) {
+    ResetSyncInternal();
+  }
 }
 
 void BraveProfileSyncServiceImpl::SetPermanentNodesOrder(
@@ -817,7 +832,11 @@ bool BraveProfileSyncServiceImpl::IsBraveSyncInitialized() const {
 }
 
 bool BraveProfileSyncServiceImpl::IsBraveSyncConfigured() const {
-  return brave_sync_configured_;
+  return brave_sync_configured_ &&
+      // When there is 0 or 1 device, it means chain is not completely created,
+      // so we should give a chance to make force reset in |OnSetupSyncHaveCode|
+      // or in |OnSetupSyncNewToSync|
+      (brave_sync_prefs_->GetSyncDevices()->size() >= 2);
 }
 
 void BraveProfileSyncServiceImpl::OnPollSyncCycle(GetRecordsCallback cb,

diff --git a/components/brave_sync/brave_profile_sync_service_impl.h b/components/brave_sync/brave_profile_sync_service_impl.h
@@ -34,6 +34,8 @@ FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnSyncDebug);
 FORWARD_DECLARE_TEST(BraveSyncServiceTest, StartSyncNonDeviceRecords);
 FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnSyncReadyNewToSync);
 FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnGetExistingObjects);
+FORWARD_DECLARE_TEST(BraveSyncServiceTest,
+                     OnSetupSyncHaveCode_Reset_SetupAgain);
 
 class BraveSyncServiceTest;
 
@@ -131,6 +133,8 @@ class BraveProfileSyncServiceImpl : public BraveProfileSyncService,
   FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, StartSyncNonDeviceRecords);
   FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, OnSyncReadyNewToSync);
   FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, OnGetExistingObjects);
+  FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest,
+                           OnSetupSyncHaveCode_Reset_SetupAgain);
   friend class ::BraveSyncServiceTest;
 
   void SignalWaitableEvent();
@@ -148,6 +152,10 @@ class BraveProfileSyncServiceImpl : public BraveProfileSyncService,
   void NotifyHaveSyncWords(const std::string& sync_words);
 
   void ResetSyncInternal();
+  void ForceCompleteReset();
+  bool GetResettingForTest() const {
+    return reseting_;
+  }
 
   void SetPermanentNodesOrder(const std::string& base_order);
 
@@ -172,6 +180,8 @@ class BraveProfileSyncServiceImpl : public BraveProfileSyncService,
   // while being initializing
   bool brave_sync_initializing_ = false;
 
+  bool reseting_ = false;
+
   std::string brave_sync_words_;
 
   brave_sync::GetRecordsCallback get_record_cb_;

diff --git a/components/brave_sync/brave_sync_service_unittest.cc b/components/brave_sync/brave_sync_service_unittest.cc
@@ -690,6 +690,27 @@ TEST_F(BraveSyncServiceTest, OnGetExistingObjects) {
       false);
 }
 
+TEST_F(BraveSyncServiceTest, GetPreferredDataTypes) {
+  // Make sure GetPreferredDataTypes contains DEVICE_INFO which is needed for
+  // brave device record polling
+  EXPECT_TRUE(sync_service()->GetPreferredDataTypes().Has(syncer::DEVICE_INFO));
+}
+
+TEST_F(BraveSyncServiceTest, GetDisableReasons) {
+  sync_prefs()->SetManagedForTest(true);
+  EXPECT_EQ(sync_service()->GetDisableReasons(),
+            syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY);
+  sync_service()->OnSetSyncEnabled(true);
+  EXPECT_EQ(sync_service()->GetDisableReasons(),
+            syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY);
+  brave_sync_prefs()->SetMigratedBookmarksVersion(2);
+  EXPECT_EQ(sync_service()->GetDisableReasons(),
+            syncer::SyncService::DISABLE_REASON_NONE);
+  sync_service()->OnSetSyncEnabled(false);
+  EXPECT_TRUE(sync_service()->HasDisableReason(
+              syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY));
+}
+
 TEST_F(BraveSyncServiceTest, OnSetupSyncHaveCode_Reset_SetupAgain) {
   EXPECT_FALSE(sync_service()->GetResettingForTest());
   EXPECT_CALL(*sync_client(), OnSyncEnabledChanged).Times(1);
@@ -702,9 +723,6 @@ TEST_F(BraveSyncServiceTest, OnSetupSyncHaveCode_Reset_SetupAgain) {
   brave_sync::Uint8Array seed
                       = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16};
   brave_sync::Uint8Array device_id = {0};
-  // These settings are changed:
-  //    bookmarks_enabled, site_settings_enabled, history_enabled
-  EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(3);
   sync_service()->OnSaveInitData(seed, device_id);
 
   RecordsList records;
@@ -714,7 +732,7 @@ TEST_F(BraveSyncServiceTest, OnSetupSyncHaveCode_Reset_SetupAgain) {
   EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(1);
   sync_service()->OnResolvedPreferences(records);
 
-  auto devices = sync_service()->sync_prefs_->GetSyncDevices();
+  auto devices = brave_sync_prefs()->GetSyncDevices();
 
   ASSERT_EQ(devices->size(), 1u);
   EXPECT_TRUE(DevicesContains(devices.get(), "0", "this_device"));
@@ -756,24 +774,3 @@ TEST_F(BraveSyncServiceTest, OnSetupSyncHaveCode_Reset_SetupAgain) {
   EXPECT_TRUE(profile()->GetPrefs()->GetBoolean(
        brave_sync::prefs::kSyncEnabled));
 }
-
-TEST_F(BraveSyncServiceTest, GetPreferredDataTypes) {
-  // Make sure GetPreferredDataTypes contains DEVICE_INFO which is needed for
-  // brave device record polling
-  EXPECT_TRUE(sync_service()->GetPreferredDataTypes().Has(syncer::DEVICE_INFO));
-}
-
-TEST_F(BraveSyncServiceTest, GetDisableReasons) {
-  sync_prefs()->SetManagedForTest(true);
-  EXPECT_EQ(sync_service()->GetDisableReasons(),
-            syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY);
-  sync_service()->OnSetSyncEnabled(true);
-  EXPECT_EQ(sync_service()->GetDisableReasons(),
-            syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY);
-  brave_sync_prefs()->SetMigratedBookmarksVersion(2);
-  EXPECT_EQ(sync_service()->GetDisableReasons(),
-            syncer::SyncService::DISABLE_REASON_NONE);
-  sync_service()->OnSetSyncEnabled(false);
-  EXPECT_TRUE(sync_service()->HasDisableReason(
-              syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY));
-}