diff --git a/components/brave_sync/brave_sync_service_impl.cc b/components/brave_sync/brave_sync_service_impl.cc index cb76609e0f92..6f45b1fd9227 100644 --- a/components/brave_sync/brave_sync_service_impl.cc +++ b/components/brave_sync/brave_sync_service_impl.cc @@ -290,7 +290,9 @@ void BraveSyncServiceImpl::BackgroundSyncStarted(bool startup) { if (startup) bookmark_change_processor_->Start(); - StartLoop(); + const bool waiting_for_second_device = + !sync_prefs_->GetSyncDevices()->has_second_device(); + StartLoop(waiting_for_second_device); } void BraveSyncServiceImpl::BackgroundSyncStopped(bool shutdown) { @@ -466,9 +468,10 @@ BraveSyncServiceImpl::PrepareResolvedPreferences(const RecordsList& records) { void BraveSyncServiceImpl::OnResolvedPreferences(const RecordsList& records) { const std::string this_device_id = sync_prefs_->GetThisDeviceId(); bool this_device_deleted = false; - bool contains_only_one_device = false; + bool at_least_one_deleted = false; auto sync_devices = sync_prefs_->GetSyncDevices(); + const bool waiting_for_second_device = !sync_devices->has_second_device(); for (const auto &record : records) { DCHECK(record->has_device() || record->has_sitesetting()); if (record->has_device()) { @@ -484,18 +487,20 @@ void BraveSyncServiceImpl::OnResolvedPreferences(const RecordsList& records) { (record->deviceId == this_device_id && record->action == jslib::SyncRecord::Action::A_DELETE && actually_merged); - contains_only_one_device = sync_devices->size() < 2 && - record->action == jslib::SyncRecord::Action::A_DELETE && - actually_merged; + at_least_one_deleted = at_least_one_deleted || + (record->action == jslib::SyncRecord::Action::A_DELETE && + actually_merged); } } // for each device sync_prefs_->SetSyncDevices(*sync_devices); - - if (this_device_deleted) + if (this_device_deleted || + (at_least_one_deleted && !sync_devices->has_second_device())) { ResetSyncInternal(); - if (contains_only_one_device) - OnResetSync(); + } else if (waiting_for_second_device && sync_devices->has_second_device()) { + // Restart loop with 30 sec interval + StartLoop(false); + } } void BraveSyncServiceImpl::OnSyncPrefsChanged(const std::string& pref) { @@ -610,10 +615,14 @@ void BraveSyncServiceImpl::SendDeviceSyncRecord( } static const int64_t kCheckUpdatesIntervalSec = 60; +static const int64_t kCheckInitialUpdatesIntervalSec = 1; -void BraveSyncServiceImpl::StartLoop() { +void BraveSyncServiceImpl::StartLoop(const bool use_initial_update_interval) { timer_->Start(FROM_HERE, - base::TimeDelta::FromSeconds(kCheckUpdatesIntervalSec), + base::TimeDelta::FromSeconds( + use_initial_update_interval ? + kCheckInitialUpdatesIntervalSec : + kCheckUpdatesIntervalSec), this, &BraveSyncServiceImpl::LoopProc); } @@ -640,6 +649,10 @@ void BraveSyncServiceImpl::LoopProcThreadAligned() { RequestSyncData(); } +base::TimeDelta BraveSyncServiceImpl::GetLoopDelay() const { + return timer_->GetCurrentDelay(); +} + void BraveSyncServiceImpl::NotifyLogMessage(const std::string& message) { DLOG(INFO) << message; } diff --git a/components/brave_sync/brave_sync_service_impl.h b/components/brave_sync/brave_sync_service_impl.h index 587667b49faf..5dbffd580759 100644 --- a/components/brave_sync/brave_sync_service_impl.h +++ b/components/brave_sync/brave_sync_service_impl.h @@ -33,6 +33,7 @@ FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnSyncReadyNewToSync); FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnGetExistingObjects); FORWARD_DECLARE_TEST(BraveSyncServiceTest, BackgroundSyncStarted); FORWARD_DECLARE_TEST(BraveSyncServiceTest, BackgroundSyncStopped); +FORWARD_DECLARE_TEST(BraveSyncServiceTest, LoopDelayVaries); class BraveSyncServiceTest; @@ -110,6 +111,7 @@ class BraveSyncServiceImpl FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, OnGetExistingObjects); FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, BackgroundSyncStarted); FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, BackgroundSyncStopped); + FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, LoopDelayVaries); friend class ::BraveSyncServiceTest; // SyncMessageHandler overrides @@ -157,10 +159,11 @@ class BraveSyncServiceImpl const std::string& deviceId, const std::string& objectId); - void StartLoop(); + void StartLoop(const bool use_initial_update_interval); void StopLoop(); void LoopProc(); void LoopProcThreadAligned(); + base::TimeDelta GetLoopDelay() const; // For tests only void GetExistingHistoryObjects( const RecordsList &records, diff --git a/components/brave_sync/brave_sync_service_unittest.cc b/components/brave_sync/brave_sync_service_unittest.cc index 37468429520b..bf0c2e7ea6a9 100644 --- a/components/brave_sync/brave_sync_service_unittest.cc +++ b/components/brave_sync/brave_sync_service_unittest.cc @@ -526,7 +526,7 @@ TEST_F(BraveSyncServiceTest, OnDeleteDeviceWhenSelfDeleted) { auto resolved_record = SyncRecord::Clone(*records.at(0)); resolved_record->action = jslib::SyncRecord::Action::A_DELETE; resolved_records.push_back(std::move(resolved_record)); - EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(5); + EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(AtLeast(3)); sync_service()->OnResolvedPreferences(resolved_records); auto devices_final = sync_service()->sync_prefs_->GetSyncDevices(); @@ -698,3 +698,94 @@ TEST_F(BraveSyncServiceTest, BackgroundSyncStopped) { sync_service()->BackgroundSyncStopped(false); EXPECT_FALSE(sync_service()->timer_->IsRunning()); } + +TEST_F(BraveSyncServiceTest, LoopDelayVaries) { + // This test is almost the same as BraveSyncServiceTest::OnDeleteDevice + // Loop delay should be: + // 1 sec when there is no sync chain yet + // 60 sec when 2 or more devices + // after sync chain destroy the loop should not be running + + sync_service()->BackgroundSyncStarted(false); + + EXPECT_TRUE(sync_service()->timer_->IsRunning()); + EXPECT_EQ(sync_service()->GetLoopDelay().InSeconds(), 1u); + + RecordsList records; + records.push_back(SimpleDeviceRecord( + jslib::SyncRecord::Action::A_CREATE, + "1", "device1")); + records.push_back(SimpleDeviceRecord( + jslib::SyncRecord::Action::A_CREATE, + "2", "device2")); + records.push_back(SimpleDeviceRecord( + jslib::SyncRecord::Action::A_CREATE, + "3", "device3")); + EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(1); + sync_service()->OnResolvedPreferences(records); + + sync_service()->sync_prefs_->SetThisDeviceId("1"); + auto devices = sync_service()->sync_prefs_->GetSyncDevices(); + + // Have 3 devices now + EXPECT_EQ(sync_service()->GetLoopDelay().InSeconds(), 60u); + + EXPECT_TRUE(DevicesContains(devices.get(), "1", "device1")); + EXPECT_TRUE(DevicesContains(devices.get(), "2", "device2")); + EXPECT_TRUE(DevicesContains(devices.get(), "3", "device3")); + + using brave_sync::jslib::SyncRecord; + EXPECT_CALL(*sync_client(), SendSyncRecords("PREFERENCES", + ContainsDeviceRecord(SyncRecord::Action::A_DELETE, "device3"))).Times(1); + sync_service()->OnDeleteDevice("3"); + + RecordsList resolved_records; + auto resolved_record = SyncRecord::Clone(*records.at(2)); + resolved_record->action = jslib::SyncRecord::Action::A_DELETE; + resolved_records.push_back(std::move(resolved_record));{ + EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(1); + sync_service()->OnResolvedPreferences(resolved_records);} + + auto devices_final = sync_service()->sync_prefs_->GetSyncDevices(); + EXPECT_TRUE(DevicesContains(devices_final.get(), "1", "device1")); + EXPECT_TRUE(DevicesContains(devices_final.get(), "2", "device2")); + EXPECT_FALSE(DevicesContains(devices_final.get(), "3", "device3")); + + // Have 2 devices now, still expecting 60 sec delay + EXPECT_EQ(sync_service()->GetLoopDelay().InSeconds(), 60u); + + // Delete all remaining devices + resolved_records.clear(); + + auto resolved_record_0 = SyncRecord::Clone(*records.at(0)); + resolved_record_0->action = jslib::SyncRecord::Action::A_DELETE; + resolved_records.push_back(std::move(resolved_record_0)); + + auto resolved_record_1 = SyncRecord::Clone(*records.at(1)); + resolved_record_1->action = jslib::SyncRecord::Action::A_DELETE; + resolved_records.push_back(std::move(resolved_record_1)); + + { + // Expecting call of OnSyncStateChanged at least 3 times: + // delete "device1" + // delete "device2" + // brave_sync.enabled to false, several times + EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(AtLeast(3)); + EXPECT_CALL(*sync_client(), OnSyncEnabledChanged).Times(AtLeast(1)); + sync_service()->OnResolvedPreferences(resolved_records); + } + + devices_final = sync_service()->sync_prefs_->GetSyncDevices(); + EXPECT_FALSE(DevicesContains(devices_final.get(), "1", "device1")); + EXPECT_FALSE(DevicesContains(devices_final.get(), "2", "device2")); + EXPECT_FALSE(DevicesContains(devices_final.get(), "3", "device3")); + + // We have mock client, so emulate these steps when all devices removed + // 1) brave_sync.enabled goes to false + // 2) BraveSyncClientImpl::OnSyncEnabledChanged + // 3) BraveSyncClientImpl::LoadOrUnloadExtension(false) + // 4) BraveSyncClientImpl::OnExtensionUnloaded + // 5) BraveSyncServiceImpl::BackgroundSyncStopped + sync_service()->BackgroundSyncStopped(true); + EXPECT_FALSE(sync_service()->timer_->IsRunning()); +} diff --git a/components/brave_sync/sync_devices.h b/components/brave_sync/sync_devices.h index 10c35135c970..ab4c48aa4738 100644 --- a/components/brave_sync/sync_devices.h +++ b/components/brave_sync/sync_devices.h @@ -43,6 +43,7 @@ class SyncDevices { std::unique_ptr ToValueArrOnly() const; std::string ToJson() const; size_t size() const { return devices_.size(); } + bool has_second_device() const { return size() >= 2; } void FromJson(const std::string &str_json); void Merge(const SyncDevice& device, int action, bool* actually_merged);