Skip to content

Commit 7a4d18c

Browse files
Redo Brave sync prefs
Allow one device to be in chain. Remove unnecessary states.
1 parent e24df69 commit 7a4d18c

File tree

2 files changed

+53
-129
lines changed

2 files changed

+53
-129
lines changed

components/brave_sync/brave_profile_sync_service_impl.cc

+38-115
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
#include "components/bookmarks/browser/bookmark_model.h"
3131
#include "components/signin/public/identity_manager/account_info.h"
3232
#include "components/sync/engine_impl/syncer.h"
33-
#include "content/public/browser/browser_thread.h"
3433
#include "net/base/network_interfaces.h"
3534
#include "ui/base/models/tree_node_iterator.h"
3635

@@ -78,22 +77,21 @@ std::string GetDeviceName() {
7877
return hostname;
7978
}
8079

81-
RecordsListPtr CreateDeviceCreationRecordExtension(
82-
const std::string& deviceName,
83-
const std::string& objectId,
84-
const SyncRecord::Action& action,
85-
const std::string& deviceId) {
80+
RecordsListPtr CreateDeviceRecord(const std::string& device_name,
81+
const std::string& object_id,
82+
const SyncRecord::Action& action,
83+
const std::string& device_id) {
8684
RecordsListPtr records = std::make_unique<RecordsList>();
8785

8886
SyncRecordPtr record = std::make_unique<SyncRecord>();
8987

9088
record->action = action;
91-
record->deviceId = deviceId;
92-
record->objectId = objectId;
89+
record->deviceId = device_id;
90+
record->objectId = object_id;
9391
record->objectData = SyncObjectData_DEVICE; // "device"
9492

9593
std::unique_ptr<Device> device = std::make_unique<Device>();
96-
device->name = deviceName;
94+
device->name = device_name;
9795
record->SetDevice(std::move(device));
9896

9997
records->emplace_back(std::move(record));
@@ -167,11 +165,10 @@ BraveProfileSyncServiceImpl::BraveProfileSyncServiceImpl(Profile* profile,
167165
InitParams init_params)
168166
: BraveProfileSyncService(std::move(init_params)),
169167
brave_sync_client_(BraveSyncClient::Create(this, profile)) {
170-
brave_sync_words_ = std::string();
171168
brave_sync_prefs_ =
172169
std::make_unique<prefs::Prefs>(sync_client_->GetPrefService());
173170

174-
// Moniter syncs prefs required in GetSettingsAndDevices
171+
// Monitor syncs prefs required in GetSettingsAndDevices
175172
brave_pref_change_registrar_.Init(sync_client_->GetPrefService());
176173
brave_pref_change_registrar_.Add(
177174
prefs::kSyncEnabled,
@@ -200,30 +197,18 @@ BraveProfileSyncServiceImpl::BraveProfileSyncServiceImpl(Profile* profile,
200197

201198
model_ = BookmarkModelFactory::GetForBrowserContext(profile);
202199

203-
if (!brave_sync_prefs_->GetSeed().empty() &&
204-
!brave_sync_prefs_->GetThisDeviceName().empty()) {
205-
brave_sync_configured_ = true;
206-
}
207200
network_connection_tracker_->AddNetworkConnectionObserver(this);
208201
RecordSyncStateP3A();
209202
}
210203

211204
void BraveProfileSyncServiceImpl::OnNudgeSyncCycle(RecordsListPtr records) {
212-
if (!IsBraveSyncEnabled())
205+
if (!brave_sync_prefs_->GetSyncEnabled())
213206
return;
214207

215208
for (auto& record : *records) {
216209
record->deviceId = brave_sync_prefs_->GetThisDeviceId();
217210
}
218211
if (!records->empty()) {
219-
if (((!brave_sync::tools::IsTimeEmpty(chain_created_time_) &&
220-
(base::Time::Now() - chain_created_time_).InSeconds() < 30u) ||
221-
brave_sync_prefs_->GetSyncDevices()->size() < 2)) {
222-
// Store records for now
223-
pending_send_records_.push_back(std::move(records));
224-
return;
225-
}
226-
SendAndPurgePendingRecords();
227212
SendSyncRecords(jslib_const::SyncRecordType_BOOKMARKS, std::move(records));
228213
}
229214
}
@@ -241,27 +226,31 @@ void BraveProfileSyncServiceImpl::OnSetupSyncHaveCode(
241226
return;
242227
}
243228

229+
Uint8Array seed;
230+
if (!crypto::PassphraseToBytes32(sync_words, &seed)) {
231+
OnSyncSetupError("ERR_SYNC_WRONG_WORDS");
232+
return;
233+
}
234+
244235
if (brave_sync_initializing_) {
245236
NotifyLogMessage("currently initializing");
246237
return;
247238
}
248239

249-
if (IsBraveSyncConfigured()) {
240+
if (!brave_sync_prefs_->GetSeed().empty()) {
250241
NotifyLogMessage("already configured");
251242
return;
252243
}
253244

254-
ForceCompleteReset();
255245
DCHECK(!brave_sync_prefs_->GetSyncEnabled());
256246

257247
if (device_name.empty())
258248
brave_sync_prefs_->SetThisDeviceName(GetDeviceName());
259249
else
260250
brave_sync_prefs_->SetThisDeviceName(device_name);
261251
brave_sync_initializing_ = true;
262-
263252
brave_sync_prefs_->SetSyncEnabled(true);
264-
brave_sync_words_ = sync_words;
253+
seed_ = seed;
265254
}
266255

267256
void BraveProfileSyncServiceImpl::OnSetupSyncNewToSync(
@@ -273,18 +262,13 @@ void BraveProfileSyncServiceImpl::OnSetupSyncNewToSync(
273262
return;
274263
}
275264

276-
if (IsBraveSyncConfigured()) {
265+
if (!brave_sync_prefs_->GetSeed().empty()) {
277266
NotifyLogMessage("already configured");
278267
return;
279268
}
280269

281-
ForceCompleteReset();
282270
DCHECK(!brave_sync_prefs_->GetSyncEnabled());
283271

284-
// If the previous attempt was connect to sync chain
285-
// and failed to receive save-init-data
286-
brave_sync_words_.clear();
287-
288272
if (device_name.empty())
289273
brave_sync_prefs_->SetThisDeviceName(GetDeviceName());
290274
else
@@ -305,6 +289,7 @@ void BraveProfileSyncServiceImpl::OnDeleteDevice(const std::string& device_id) {
305289
const std::string object_id = device->object_id_;
306290
SendDeviceSyncRecord(SyncRecord::Action::A_DELETE, device_name, device_id,
307291
object_id);
292+
FetchDevices();
308293
}
309294
}
310295

@@ -321,7 +306,6 @@ void BraveProfileSyncServiceImpl::OnResetSync() {
321306
// we can reset it by ResetSyncInternal()
322307
const std::string device_id = brave_sync_prefs_->GetThisDeviceId();
323308
OnDeleteDevice(device_id);
324-
reseting_ = true;
325309
}
326310
}
327311

@@ -394,12 +378,8 @@ void BraveProfileSyncServiceImpl::OnGetInitData(
394378
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
395379

396380
Uint8Array seed;
397-
if (!brave_sync_words_.empty()) {
398-
VLOG(1) << "[Brave Sync] Init from sync words";
399-
if (!crypto::PassphraseToBytes32(brave_sync_words_, &seed)) {
400-
OnSyncSetupError("ERR_SYNC_WRONG_WORDS");
401-
return;
402-
}
381+
if (!seed_.empty()) {
382+
seed = seed_;
403383
} else if (!brave_sync_prefs_->GetSeed().empty()) {
404384
seed = Uint8ArrayFromString(brave_sync_prefs_->GetSeed());
405385
VLOG(1) << "[Brave Sync] Init from prefs";
@@ -432,7 +412,7 @@ void BraveProfileSyncServiceImpl::OnGetInitData(
432412
void BraveProfileSyncServiceImpl::OnSaveInitData(const Uint8Array& seed,
433413
const Uint8Array& device_id) {
434414
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
435-
DCHECK(!brave_sync_initialized_);
415+
DCHECK(!brave_sync_ready_);
436416
// If we are here and brave_sync_initializing_ is false, we have came
437417
// not from OnSetupSyncNewToSync or OnSetupSyncHaveCode.
438418
// One case is we put wrong code words and then restarted before cleared
@@ -444,27 +424,18 @@ void BraveProfileSyncServiceImpl::OnSaveInitData(const Uint8Array& seed,
444424

445425
std::string prev_seed_str = brave_sync_prefs_->GetPrevSeed();
446426

447-
brave_sync_words_.clear();
427+
seed_.clear();
448428
DCHECK(!seed_str.empty());
449429

450430
if (prev_seed_str == seed_str) { // reconnecting to previous sync chain
451431
brave_sync_prefs_->SetPrevSeed(std::string());
452432
} else if (!prev_seed_str.empty()) { // connect/create to new sync chain
453-
// bookmark_change_processor_->Reset(true);
454433
brave_sync_prefs_->SetPrevSeed(std::string());
455-
} else {
456-
// This is not required, because when there is no previous seed, bookmarks
457-
// should not have a metadata. However, this is done by intention, to be
458-
// a remedy for cases when sync had been reset and prev_seed_str had been
459-
// cleared when it shouldn't (brave-browser#3188).
460-
// bookmark_change_processor_->Reset(true);
461434
}
462435

463436
brave_sync_prefs_->SetSeed(seed_str);
464437
brave_sync_prefs_->SetThisDeviceId(device_id_str);
465438

466-
brave_sync_configured_ = true;
467-
468439
brave_sync_initializing_ = false;
469440
}
470441

@@ -480,8 +451,8 @@ void BraveProfileSyncServiceImpl::OnSyncReady() {
480451
return;
481452
}
482453

483-
DCHECK(false == brave_sync_initialized_);
484-
brave_sync_initialized_ = true;
454+
DCHECK(false == brave_sync_ready_);
455+
brave_sync_ready_ = true;
485456

486457
// For launching from legacy sync profile and also brand new profile
487458
if (brave_sync_prefs_->GetMigratedBookmarksVersion() < 2)
@@ -676,27 +647,16 @@ void BraveProfileSyncServiceImpl::NotifyHaveSyncWords(
676647
void BraveProfileSyncServiceImpl::ResetSyncInternal() {
677648
SignalWaitableEvent();
678649
brave_sync_prefs_->SetPrevSeed(brave_sync_prefs_->GetSeed());
679-
680650
brave_sync_prefs_->Clear();
681651

682-
brave_sync_configured_ = false;
683-
brave_sync_initialized_ = false;
652+
brave_sync_ready_ = false;
684653

685-
brave_sync_prefs_->SetSyncEnabled(false);
686654
ProfileSyncService::GetUserSettings()->SetSyncRequested(false);
687655

688656
// brave sync doesn't support pause sync so treating every new sync chain as
689657
// first time setup
690658
syncer::SyncPrefs sync_prefs(sync_client_->GetPrefService());
691659
sync_prefs.SetLastSyncedTime(base::Time());
692-
693-
reseting_ = false;
694-
}
695-
696-
void BraveProfileSyncServiceImpl::ForceCompleteReset() {
697-
if (reseting_) {
698-
ResetSyncInternal();
699-
}
700660
}
701661

702662
void BraveProfileSyncServiceImpl::SetPermanentNodesOrder(
@@ -881,7 +841,7 @@ void BraveProfileSyncServiceImpl::SendDeviceSyncRecord(
881841
const std::string& device_id,
882842
const std::string& object_id) {
883843
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
884-
RecordsListPtr records = CreateDeviceCreationRecordExtension(
844+
RecordsListPtr records = CreateDeviceRecord(
885845
device_name, object_id, static_cast<SyncRecord::Action>(action),
886846
device_id);
887847
SendSyncRecords(SyncRecordType_PREFERENCES, std::move(records));
@@ -891,10 +851,8 @@ void BraveProfileSyncServiceImpl::OnResolvedPreferences(
891851
const RecordsList& records) {
892852
const std::string this_device_id = brave_sync_prefs_->GetThisDeviceId();
893853
bool this_device_deleted = false;
894-
bool contains_only_one_device = false;
895854

896855
auto sync_devices = brave_sync_prefs_->GetSyncDevices();
897-
auto old_devices_size = sync_devices->size();
898856
for (const auto& record : records) {
899857
DCHECK(record->has_device() || record->has_sitesetting());
900858
if (record->has_device()) {
@@ -907,30 +865,13 @@ void BraveProfileSyncServiceImpl::OnResolvedPreferences(
907865
this_device_deleted ||
908866
(record->deviceId == this_device_id &&
909867
record->action == SyncRecord::Action::A_DELETE && actually_merged);
910-
contains_only_one_device =
911-
sync_devices->size() < 2 &&
912-
record->action == SyncRecord::Action::A_DELETE && actually_merged;
913868
}
914869
} // for each device
915870

916871
brave_sync_prefs_->SetSyncDevices(*sync_devices);
917872

918-
if (old_devices_size < 2 && sync_devices->size() >= 2) {
919-
// Save chain creation time to send bookmarks 30 sec after
920-
chain_created_time_ = base::Time::Now();
921-
}
922-
if (!tools::IsTimeEmpty(chain_created_time_) &&
923-
(base::Time::Now() - chain_created_time_).InSeconds() > 30u) {
924-
SendAndPurgePendingRecords();
925-
}
926-
927873
if (this_device_deleted) {
928874
ResetSyncInternal();
929-
} else if (contains_only_one_device) {
930-
// We see amount of devices had been decreased to 1 and it is not this
931-
// device had been deleted. So call OnResetSync which will send DELETE
932-
// record for this device
933-
OnResetSync();
934875
}
935876
}
936877

@@ -956,31 +897,19 @@ bool BraveProfileSyncServiceImpl::IsBraveSyncEnabled() const {
956897
return brave_sync_prefs_->GetSyncEnabled();
957898
}
958899

959-
bool BraveProfileSyncServiceImpl::IsBraveSyncInitialized() const {
960-
return brave_sync_initialized_;
961-
}
962-
963-
bool BraveProfileSyncServiceImpl::IsBraveSyncConfigured() const {
964-
return brave_sync_configured_ &&
965-
// When there is 0 or 1 device, it means chain is not completely
966-
// created, so we should give a chance to make force reset in
967-
// |OnSetupSyncHaveCode| or in |OnSetupSyncNewToSync|
968-
(brave_sync_prefs_->GetSyncDevices()->size() >= 2);
969-
}
970900
void BraveProfileSyncServiceImpl::FetchDevices() {
971901
DCHECK(sync_client_);
972902
brave_sync_prefs_->SetLastFetchTime(base::Time::Now());
973-
// When the chain is not fully created or our device preferences queue is
974-
// not available, it may happened we will not find required device sync
975-
// record when we will be switched to use SQS instead of S3 in sync lib.
903+
904+
// When we create the device, we also create a set of SQS queues
905+
// For other devices' S3 put lambda hook, some amount of time is required
906+
// to see our queues in the result of listQueues
976907
// Default Chromium fetch interval is 60 sec.
977908
// So during 70 sec after the chain creation forcing use of S3
978909
// by set start_at_time to 0.
979-
980910
base::Time start_at_time;
981-
if (brave_sync_prefs_->GetSyncDevices()->size() <= 1 ||
982-
(!tools::IsTimeEmpty(chain_created_time_) &&
983-
(base::Time::Now() - chain_created_time_).InSeconds() < 70u)) {
911+
if (!tools::IsTimeEmpty(this_device_created_time_) &&
912+
(base::Time::Now() - this_device_created_time_).InSeconds() < 70u) {
984913
start_at_time = base::Time();
985914
} else {
986915
start_at_time = brave_sync_prefs_->GetLatestDeviceRecordTime();
@@ -991,15 +920,17 @@ void BraveProfileSyncServiceImpl::FetchDevices() {
991920
}
992921
void BraveProfileSyncServiceImpl::OnPollSyncCycle(GetRecordsCallback cb,
993922
base::WaitableEvent* wevent) {
994-
if (!IsBraveSyncEnabled())
923+
if (!brave_sync_prefs_->GetSyncEnabled())
995924
return;
996925

997-
if (IsTimeEmpty(brave_sync_prefs_->GetLastFetchTime()))
926+
if (IsTimeEmpty(brave_sync_prefs_->GetLastFetchTime())) {
998927
SendCreateDevice();
928+
this_device_created_time_ = base::Time::Now();
929+
}
999930

1000931
FetchDevices();
1001932

1002-
if (!brave_sync_initialized_) {
933+
if (!brave_sync_ready_) {
1003934
wevent->Signal();
1004935
return;
1005936
}
@@ -1027,14 +958,6 @@ BraveSyncService* BraveProfileSyncServiceImpl::GetSyncService() const {
1027958
const_cast<BraveProfileSyncServiceImpl*>(this));
1028959
}
1029960

1030-
void BraveProfileSyncServiceImpl::SendAndPurgePendingRecords() {
1031-
for (auto& records_to_send : pending_send_records_) {
1032-
SendSyncRecords(jslib_const::SyncRecordType_BOOKMARKS,
1033-
std::move(records_to_send));
1034-
}
1035-
pending_send_records_.clear();
1036-
}
1037-
1038961
void BraveProfileSyncServiceImpl::SendSyncRecords(
1039962
const std::string& category_name,
1040963
RecordsListPtr records) {

0 commit comments

Comments
 (0)