Skip to content

Commit f005c59

Browse files
AlexeyBarabashAlexeyBarabash
authored
Merge pull request #1188 from brave/sync_frequent_initial_update
Update devices each 1 second until chain is not created
2 parents 9c45f2f + ca1da86 commit f005c59

File tree

4 files changed

+121
-13
lines changed

4 files changed

+121
-13
lines changed

‎components/brave_sync/brave_sync_service_impl.cc

+24-11
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,9 @@ void BraveSyncServiceImpl::BackgroundSyncStarted(bool startup) {
290290
if (startup)
291291
bookmark_change_processor_->Start();
292292

293-
StartLoop();
293+
const bool waiting_for_second_device =
294+
!sync_prefs_->GetSyncDevices()->has_second_device();
295+
StartLoop(waiting_for_second_device);
294296
}
295297

296298
void BraveSyncServiceImpl::BackgroundSyncStopped(bool shutdown) {
@@ -466,9 +468,10 @@ BraveSyncServiceImpl::PrepareResolvedPreferences(const RecordsList& records) {
466468
void BraveSyncServiceImpl::OnResolvedPreferences(const RecordsList& records) {
467469
const std::string this_device_id = sync_prefs_->GetThisDeviceId();
468470
bool this_device_deleted = false;
469-
bool contains_only_one_device = false;
471+
bool at_least_one_deleted = false;
470472

471473
auto sync_devices = sync_prefs_->GetSyncDevices();
474+
const bool waiting_for_second_device = !sync_devices->has_second_device();
472475
for (const auto &record : records) {
473476
DCHECK(record->has_device() || record->has_sitesetting());
474477
if (record->has_device()) {
@@ -484,18 +487,20 @@ void BraveSyncServiceImpl::OnResolvedPreferences(const RecordsList& records) {
484487
(record->deviceId == this_device_id &&
485488
record->action == jslib::SyncRecord::Action::A_DELETE &&
486489
actually_merged);
487-
contains_only_one_device = sync_devices->size() < 2 &&
488-
record->action == jslib::SyncRecord::Action::A_DELETE &&
489-
actually_merged;
490+
at_least_one_deleted = at_least_one_deleted ||
491+
(record->action == jslib::SyncRecord::Action::A_DELETE &&
492+
actually_merged);
490493
}
491494
} // for each device
492495

493496
sync_prefs_->SetSyncDevices(*sync_devices);
494-
495-
if (this_device_deleted)
497+
if (this_device_deleted ||
498+
(at_least_one_deleted && !sync_devices->has_second_device())) {
496499
ResetSyncInternal();
497-
if (contains_only_one_device)
498-
OnResetSync();
500+
} else if (waiting_for_second_device && sync_devices->has_second_device()) {
501+
// Restart loop with 30 sec interval
502+
StartLoop(false);
503+
}
499504
}
500505

501506
void BraveSyncServiceImpl::OnSyncPrefsChanged(const std::string& pref) {
@@ -610,10 +615,14 @@ void BraveSyncServiceImpl::SendDeviceSyncRecord(
610615
}
611616

612617
static const int64_t kCheckUpdatesIntervalSec = 60;
618+
static const int64_t kCheckInitialUpdatesIntervalSec = 1;
613619

614-
void BraveSyncServiceImpl::StartLoop() {
620+
void BraveSyncServiceImpl::StartLoop(const bool use_initial_update_interval) {
615621
timer_->Start(FROM_HERE,
616-
base::TimeDelta::FromSeconds(kCheckUpdatesIntervalSec),
622+
base::TimeDelta::FromSeconds(
623+
use_initial_update_interval ?
624+
kCheckInitialUpdatesIntervalSec :
625+
kCheckUpdatesIntervalSec),
617626
this,
618627
&BraveSyncServiceImpl::LoopProc);
619628
}
@@ -640,6 +649,10 @@ void BraveSyncServiceImpl::LoopProcThreadAligned() {
640649
RequestSyncData();
641650
}
642651

652+
base::TimeDelta BraveSyncServiceImpl::GetLoopDelay() const {
653+
return timer_->GetCurrentDelay();
654+
}
655+
643656
void BraveSyncServiceImpl::NotifyLogMessage(const std::string& message) {
644657
DLOG(INFO) << message;
645658
}

‎components/brave_sync/brave_sync_service_impl.h

+4-1
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnSyncReadyNewToSync);
3333
FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnGetExistingObjects);
3434
FORWARD_DECLARE_TEST(BraveSyncServiceTest, BackgroundSyncStarted);
3535
FORWARD_DECLARE_TEST(BraveSyncServiceTest, BackgroundSyncStopped);
36+
FORWARD_DECLARE_TEST(BraveSyncServiceTest, LoopDelayVaries);
3637

3738
class BraveSyncServiceTest;
3839

@@ -110,6 +111,7 @@ class BraveSyncServiceImpl
110111
FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, OnGetExistingObjects);
111112
FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, BackgroundSyncStarted);
112113
FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, BackgroundSyncStopped);
114+
FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, LoopDelayVaries);
113115
friend class ::BraveSyncServiceTest;
114116

115117
// SyncMessageHandler overrides
@@ -157,10 +159,11 @@ class BraveSyncServiceImpl
157159
const std::string& deviceId,
158160
const std::string& objectId);
159161

160-
void StartLoop();
162+
void StartLoop(const bool use_initial_update_interval);
161163
void StopLoop();
162164
void LoopProc();
163165
void LoopProcThreadAligned();
166+
base::TimeDelta GetLoopDelay() const; // For tests only
164167

165168
void GetExistingHistoryObjects(
166169
const RecordsList &records,

‎components/brave_sync/brave_sync_service_unittest.cc

+92-1
Original file line numberDiff line numberDiff line change
@@ -526,7 +526,7 @@ TEST_F(BraveSyncServiceTest, OnDeleteDeviceWhenSelfDeleted) {
526526
auto resolved_record = SyncRecord::Clone(*records.at(0));
527527
resolved_record->action = jslib::SyncRecord::Action::A_DELETE;
528528
resolved_records.push_back(std::move(resolved_record));
529-
EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(5);
529+
EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(AtLeast(3));
530530
sync_service()->OnResolvedPreferences(resolved_records);
531531

532532
auto devices_final = sync_service()->sync_prefs_->GetSyncDevices();
@@ -698,3 +698,94 @@ TEST_F(BraveSyncServiceTest, BackgroundSyncStopped) {
698698
sync_service()->BackgroundSyncStopped(false);
699699
EXPECT_FALSE(sync_service()->timer_->IsRunning());
700700
}
701+
702+
TEST_F(BraveSyncServiceTest, LoopDelayVaries) {
703+
// This test is almost the same as BraveSyncServiceTest::OnDeleteDevice
704+
// Loop delay should be:
705+
// 1 sec when there is no sync chain yet
706+
// 60 sec when 2 or more devices
707+
// after sync chain destroy the loop should not be running
708+
709+
sync_service()->BackgroundSyncStarted(false);
710+
711+
EXPECT_TRUE(sync_service()->timer_->IsRunning());
712+
EXPECT_EQ(sync_service()->GetLoopDelay().InSeconds(), 1u);
713+
714+
RecordsList records;
715+
records.push_back(SimpleDeviceRecord(
716+
jslib::SyncRecord::Action::A_CREATE,
717+
"1", "device1"));
718+
records.push_back(SimpleDeviceRecord(
719+
jslib::SyncRecord::Action::A_CREATE,
720+
"2", "device2"));
721+
records.push_back(SimpleDeviceRecord(
722+
jslib::SyncRecord::Action::A_CREATE,
723+
"3", "device3"));
724+
EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(1);
725+
sync_service()->OnResolvedPreferences(records);
726+
727+
sync_service()->sync_prefs_->SetThisDeviceId("1");
728+
auto devices = sync_service()->sync_prefs_->GetSyncDevices();
729+
730+
// Have 3 devices now
731+
EXPECT_EQ(sync_service()->GetLoopDelay().InSeconds(), 60u);
732+
733+
EXPECT_TRUE(DevicesContains(devices.get(), "1", "device1"));
734+
EXPECT_TRUE(DevicesContains(devices.get(), "2", "device2"));
735+
EXPECT_TRUE(DevicesContains(devices.get(), "3", "device3"));
736+
737+
using brave_sync::jslib::SyncRecord;
738+
EXPECT_CALL(*sync_client(), SendSyncRecords("PREFERENCES",
739+
ContainsDeviceRecord(SyncRecord::Action::A_DELETE, "device3"))).Times(1);
740+
sync_service()->OnDeleteDevice("3");
741+
742+
RecordsList resolved_records;
743+
auto resolved_record = SyncRecord::Clone(*records.at(2));
744+
resolved_record->action = jslib::SyncRecord::Action::A_DELETE;
745+
resolved_records.push_back(std::move(resolved_record));{
746+
EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(1);
747+
sync_service()->OnResolvedPreferences(resolved_records);}
748+
749+
auto devices_final = sync_service()->sync_prefs_->GetSyncDevices();
750+
EXPECT_TRUE(DevicesContains(devices_final.get(), "1", "device1"));
751+
EXPECT_TRUE(DevicesContains(devices_final.get(), "2", "device2"));
752+
EXPECT_FALSE(DevicesContains(devices_final.get(), "3", "device3"));
753+
754+
// Have 2 devices now, still expecting 60 sec delay
755+
EXPECT_EQ(sync_service()->GetLoopDelay().InSeconds(), 60u);
756+
757+
// Delete all remaining devices
758+
resolved_records.clear();
759+
760+
auto resolved_record_0 = SyncRecord::Clone(*records.at(0));
761+
resolved_record_0->action = jslib::SyncRecord::Action::A_DELETE;
762+
resolved_records.push_back(std::move(resolved_record_0));
763+
764+
auto resolved_record_1 = SyncRecord::Clone(*records.at(1));
765+
resolved_record_1->action = jslib::SyncRecord::Action::A_DELETE;
766+
resolved_records.push_back(std::move(resolved_record_1));
767+
768+
{
769+
// Expecting call of OnSyncStateChanged at least 3 times:
770+
// delete "device1"
771+
// delete "device2"
772+
// brave_sync.enabled to false, several times
773+
EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(AtLeast(3));
774+
EXPECT_CALL(*sync_client(), OnSyncEnabledChanged).Times(AtLeast(1));
775+
sync_service()->OnResolvedPreferences(resolved_records);
776+
}
777+
778+
devices_final = sync_service()->sync_prefs_->GetSyncDevices();
779+
EXPECT_FALSE(DevicesContains(devices_final.get(), "1", "device1"));
780+
EXPECT_FALSE(DevicesContains(devices_final.get(), "2", "device2"));
781+
EXPECT_FALSE(DevicesContains(devices_final.get(), "3", "device3"));
782+
783+
// We have mock client, so emulate these steps when all devices removed
784+
// 1) brave_sync.enabled goes to false
785+
// 2) BraveSyncClientImpl::OnSyncEnabledChanged
786+
// 3) BraveSyncClientImpl::LoadOrUnloadExtension(false)
787+
// 4) BraveSyncClientImpl::OnExtensionUnloaded
788+
// 5) BraveSyncServiceImpl::BackgroundSyncStopped
789+
sync_service()->BackgroundSyncStopped(true);
790+
EXPECT_FALSE(sync_service()->timer_->IsRunning());
791+
}

‎components/brave_sync/sync_devices.h

+1
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ class SyncDevices {
4343
std::unique_ptr<base::Value> ToValueArrOnly() const;
4444
std::string ToJson() const;
4545
size_t size() const { return devices_.size(); }
46+
bool has_second_device() const { return size() >= 2; }
4647
void FromJson(const std::string &str_json);
4748
void Merge(const SyncDevice& device, int action, bool* actually_merged);
4849

0 commit comments

Comments
 (0)