From 62b691afef50e9a7f8eae7e4a5e47c729c88d888 Mon Sep 17 00:00:00 2001 From: AlexeyBarabash Date: Thu, 17 Oct 2019 00:15:59 +0300 Subject: [PATCH 1/6] Mark for sync chain reset and clear prefs if chain contains 1 device when browser starts --- .../brave_sync/brave_profile_sync_service_impl.cc | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/components/brave_sync/brave_profile_sync_service_impl.cc b/components/brave_sync/brave_profile_sync_service_impl.cc index c1c77145068d..3ba07cdfb454 100644 --- a/components/brave_sync/brave_profile_sync_service_impl.cc +++ b/components/brave_sync/brave_profile_sync_service_impl.cc @@ -171,6 +171,19 @@ BraveProfileSyncServiceImpl::BraveProfileSyncServiceImpl(Profile* profile, brave_sync_prefs_ = std::make_unique(sync_client_->GetPrefService()); + auto devices = brave_sync_prefs_->GetSyncDevices(); + if (devices->size() < 2 && + (brave_sync_prefs_->GetSyncEnabled() || + !brave_sync_prefs_->GetSeed().empty() || + !brave_sync_prefs_->GetThisDeviceName().empty())) { + // We want to reset the chain because it was not fully created + // We cannot make it here directly, because some sync classes may not be + // constructed yet + reseting_ = true; + // Need to clear prefs to allow UI page give us code words + brave_sync_prefs_->Clear(); + } + // Moniter syncs prefs required in GetSettingsAndDevices brave_pref_change_registrar_.Init(sync_client_->GetPrefService()); brave_pref_change_registrar_.Add( From 3e479d72ebd23f2f2846b7520c4cf9163e6f0a20 Mon Sep 17 00:00:00 2001 From: AlexeyBarabash Date: Thu, 17 Oct 2019 12:18:11 +0300 Subject: [PATCH 2/6] Testcase for chain reset if we have one device left after browser restart --- .../brave_profile_sync_service_impl.h | 4 ++ .../brave_sync/brave_sync_service_unittest.cc | 47 +++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/components/brave_sync/brave_profile_sync_service_impl.h b/components/brave_sync/brave_profile_sync_service_impl.h index 3e57b93f7fe3..76183a6352f3 100644 --- a/components/brave_sync/brave_profile_sync_service_impl.h +++ b/components/brave_sync/brave_profile_sync_service_impl.h @@ -40,6 +40,8 @@ FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnSetupSyncHaveCode_Reset_SetupAgain); FORWARD_DECLARE_TEST(BraveSyncServiceTest, ExponentialResend); FORWARD_DECLARE_TEST(BraveSyncServiceTest, GetDevicesWithFetchSyncRecords); +FORWARD_DECLARE_TEST(ServiceTestDeviceLeftover, + OnSetupSyncHaveCodeDeviceLeftover); class BraveSyncServiceTest; @@ -146,6 +148,8 @@ class BraveProfileSyncServiceImpl FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, ExponentialResend); FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, GetDevicesWithFetchSyncRecords); + FRIEND_TEST_ALL_PREFIXES(::ServiceTestDeviceLeftover, + OnSetupSyncHaveCodeDeviceLeftover); friend class ::BraveSyncServiceTest; void SignalWaitableEvent(); diff --git a/components/brave_sync/brave_sync_service_unittest.cc b/components/brave_sync/brave_sync_service_unittest.cc index 6be62da4130f..bf1f4d193ca2 100644 --- a/components/brave_sync/brave_sync_service_unittest.cc +++ b/components/brave_sync/brave_sync_service_unittest.cc @@ -195,6 +195,7 @@ class BraveSyncServiceTest : public testing::Test { sync_client_ = new MockBraveSyncClient(); brave_sync::BraveSyncClientImpl::set_for_testing(sync_client_); + PreCreateService(); sync_service_ = static_cast( ProfileSyncServiceFactory::GetAsProfileSyncServiceForProfile( profile())); @@ -214,6 +215,8 @@ class BraveSyncServiceTest : public testing::Test { profile_.reset(); } + virtual void PreCreateService() {} + Profile* profile() { return profile_.get(); } BraveProfileSyncServiceImpl* sync_service() { return sync_service_; } MockBraveSyncClient* sync_client() { return sync_client_; } @@ -240,6 +243,31 @@ class BraveSyncServiceTest : public testing::Test { base::ScopedTempDir temp_dir_; }; +class ServiceTestDeviceLeftover : public BraveSyncServiceTest { + public: + void PreCreateService() override { + // Emulating we have one device and seed saved, like inthe case when + // the browser was closed righ after starting procedure of sync chain + // creation without waiting for the second device to connect + const std::string devices_json = + R"json( + { + "devices": + [{ + "device_id":"0", + "last_active":1.571261287102e+12, + "name":"COMPUTER", + "object_id":"67, 22, 139, 42, 47, 37, 218, 215, + 254, 228, 11, 42, 192, 73, 99, 47" + }] + } + )json"; + profile()->GetPrefs()->SetString(brave_sync::prefs::kSyncDeviceList, + devices_json); + profile()->GetPrefs()->SetString(brave_sync::prefs::kSyncSeed, "1, 2, 3"); + } +}; + TEST_F(BraveSyncServiceTest, SetSyncEnabled) { EXPECT_CALL(*sync_client(), OnSyncEnabledChanged); EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(1); @@ -682,6 +710,8 @@ TEST_F(BraveSyncServiceTest, OnBraveSyncPrefsChanged) { TEST_F(BraveSyncServiceTest, StartSyncNonDeviceRecords) { EXPECT_FALSE(sync_service()->IsBraveSyncInitialized()); + EXPECT_CALL(*sync_client(), OnSyncEnabledChanged).Times(1); + EXPECT_CALL(*observer(), OnSyncStateChanged).Times(AtLeast(1)); sync_service()->OnSetSyncEnabled(true); profile()->GetPrefs()->SetString(brave_sync::prefs::kSyncBookmarksBaseOrder, "1.1."); @@ -737,6 +767,8 @@ TEST_F(BraveSyncServiceTest, GetDisableReasons) { EXPECT_EQ(sync_service()->GetDisableReasons(), syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY | syncer::SyncService::DISABLE_REASON_USER_CHOICE); + EXPECT_CALL(*sync_client(), OnSyncEnabledChanged); + EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(1); sync_service()->OnSetSyncEnabled(true); EXPECT_EQ(sync_service()->GetDisableReasons(), syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY | @@ -744,6 +776,8 @@ TEST_F(BraveSyncServiceTest, GetDisableReasons) { brave_sync_prefs()->SetMigratedBookmarksVersion(2); EXPECT_EQ(sync_service()->GetDisableReasons(), syncer::SyncService::DISABLE_REASON_NONE); + EXPECT_CALL(*sync_client(), OnSyncEnabledChanged); + EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(1); sync_service()->OnSetSyncEnabled(false); EXPECT_TRUE(sync_service()->HasDisableReason( syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY)); @@ -976,3 +1010,16 @@ TEST_F(BraveSyncServiceTest, GetDevicesWithFetchSyncRecords) { sync_service()->FetchDevices(); } } + +TEST_F(ServiceTestDeviceLeftover, OnSetupSyncHaveCodeDeviceLeftover) { + EXPECT_TRUE(sync_service()->reseting_); + EXPECT_EQ(brave_sync_prefs()->GetSyncDevices()->size(), 0u); + + EXPECT_CALL(*sync_client(), OnSyncEnabledChanged).Times(AtLeast(2)); + EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())) + .Times(AtLeast(2)); + sync_service()->OnSetupSyncHaveCode("word1 word2 word3", "test_device"); + EXPECT_TRUE( + profile()->GetPrefs()->GetBoolean(brave_sync::prefs::kSyncEnabled)); + EXPECT_FALSE(sync_service()->reseting_); +} From 0a26226787ba2854e5894803ecab0f4b787d5fae Mon Sep 17 00:00:00 2001 From: AlexeyBarabash Date: Mon, 21 Oct 2019 16:59:26 +0300 Subject: [PATCH 3/6] Resetting sync in BraveProfileSyncServiceImpl::Shutdown if it is enabled but the chain is not fully created --- .../brave_profile_sync_service_impl.cc | 26 +++++++++++++++---- .../brave_profile_sync_service_impl.h | 6 +++-- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/components/brave_sync/brave_profile_sync_service_impl.cc b/components/brave_sync/brave_profile_sync_service_impl.cc index 3ba07cdfb454..a5061fce9676 100644 --- a/components/brave_sync/brave_profile_sync_service_impl.cc +++ b/components/brave_sync/brave_profile_sync_service_impl.cc @@ -171,11 +171,7 @@ BraveProfileSyncServiceImpl::BraveProfileSyncServiceImpl(Profile* profile, brave_sync_prefs_ = std::make_unique(sync_client_->GetPrefService()); - auto devices = brave_sync_prefs_->GetSyncDevices(); - if (devices->size() < 2 && - (brave_sync_prefs_->GetSyncEnabled() || - !brave_sync_prefs_->GetSeed().empty() || - !brave_sync_prefs_->GetThisDeviceName().empty())) { + if (IsSyncSetupIncomplete()) { // We want to reset the chain because it was not fully created // We cannot make it here directly, because some sync classes may not be // constructed yet @@ -245,6 +241,17 @@ BraveProfileSyncServiceImpl::~BraveProfileSyncServiceImpl() { network_connection_tracker_->RemoveNetworkConnectionObserver(this); } +bool BraveProfileSyncServiceImpl::IsSyncSetupIncomplete() const { + auto devices = brave_sync_prefs_->GetSyncDevices(); + if (devices->size() < 2 && + (brave_sync_prefs_->GetSyncEnabled() || + !brave_sync_prefs_->GetSeed().empty() || + !brave_sync_prefs_->GetThisDeviceName().empty())) { + return true; + } + return false; +} + void BraveProfileSyncServiceImpl::OnSetupSyncHaveCode( const std::string& sync_words, const std::string& device_name) { @@ -637,6 +644,15 @@ void BraveProfileSyncServiceImpl::OnConnectionChanged( } void BraveProfileSyncServiceImpl::Shutdown() { + if (IsSyncSetupIncomplete()) { + // Need to clear prefs to allow UI page give us code words next time + // But we want to avoid unload of Extension in d`tor + brave_pref_change_registrar_.RemoveAll(); + // This will clear prefs and prepare Chromium sync for the proper enabling + // when sync will be turned on next time + ResetSyncInternal(); + } + SignalWaitableEvent(); syncer::ProfileSyncService::Shutdown(); } diff --git a/components/brave_sync/brave_profile_sync_service_impl.h b/components/brave_sync/brave_profile_sync_service_impl.h index 76183a6352f3..d1c99964f63d 100644 --- a/components/brave_sync/brave_profile_sync_service_impl.h +++ b/components/brave_sync/brave_profile_sync_service_impl.h @@ -40,7 +40,7 @@ FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnSetupSyncHaveCode_Reset_SetupAgain); FORWARD_DECLARE_TEST(BraveSyncServiceTest, ExponentialResend); FORWARD_DECLARE_TEST(BraveSyncServiceTest, GetDevicesWithFetchSyncRecords); -FORWARD_DECLARE_TEST(ServiceTestDeviceLeftover, +FORWARD_DECLARE_TEST(ServiceStartDeviceLeftoverTest, OnSetupSyncHaveCodeDeviceLeftover); class BraveSyncServiceTest; @@ -148,7 +148,7 @@ class BraveProfileSyncServiceImpl FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, ExponentialResend); FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, GetDevicesWithFetchSyncRecords); - FRIEND_TEST_ALL_PREFIXES(::ServiceTestDeviceLeftover, + FRIEND_TEST_ALL_PREFIXES(::ServiceStartDeviceLeftoverTest, OnSetupSyncHaveCodeDeviceLeftover); friend class ::BraveSyncServiceTest; @@ -196,6 +196,8 @@ class BraveProfileSyncServiceImpl void RecordSyncStateP3A() const; + bool IsSyncSetupIncomplete() const; + static base::TimeDelta GetRetryExponentialWaitAmount(int retry_number); static std::vector GetExponentialWaitsForTests(); static const std::vector kExponentialWaits; From d235975098efb99042f2dda1d91d5df3750920dd Mon Sep 17 00:00:00 2001 From: AlexeyBarabash Date: Mon, 21 Oct 2019 17:04:47 +0300 Subject: [PATCH 4/6] Testcase for resetting sync at BraveProfileSyncServiceImpl::Shutdown when the chain is not fully created --- .../brave_sync/brave_sync_service_unittest.cc | 59 +++++++++++++------ 1 file changed, 41 insertions(+), 18 deletions(-) diff --git a/components/brave_sync/brave_sync_service_unittest.cc b/components/brave_sync/brave_sync_service_unittest.cc index bf1f4d193ca2..f6956aeaf220 100644 --- a/components/brave_sync/brave_sync_service_unittest.cc +++ b/components/brave_sync/brave_sync_service_unittest.cc @@ -155,6 +155,27 @@ std::unique_ptr OverrideForTimeDelta( nullptr); } +void PreparePrefsStateDeviceLeftover(PrefService* pref_service) { + // Emulating we have one device and seed saved, like in the case when + // the browser was closed right after starting sync chain creation without + // waiting for the second device to connect + const std::string devices_json = + R"json( + { + "devices": + [{ + "device_id":"0", + "last_active":1.571261287102e+12, + "name":"COMPUTER", + "object_id":"67, 22, 139, 42, 47, 37, 218, 215, + 254, 228, 11, 42, 192, 73, 99, 47" + }] + } + )json"; + pref_service->SetString(brave_sync::prefs::kSyncDeviceList, devices_json); + pref_service->SetString(brave_sync::prefs::kSyncSeed, "1, 2, 3"); +} + } // namespace class MockBraveSyncServiceObserver : public BraveSyncServiceObserver { @@ -211,11 +232,13 @@ class BraveSyncServiceTest : public testing::Test { sync_service_->BraveSyncService::RemoveObserver(observer_.get()); // this will also trigger a shutdown of the brave sync service sync_service_->Shutdown(); + PostShotdownService(); sync_prefs_.reset(); profile_.reset(); } virtual void PreCreateService() {} + virtual void PostShotdownService() {} Profile* profile() { return profile_.get(); } BraveProfileSyncServiceImpl* sync_service() { return sync_service_; } @@ -243,28 +266,20 @@ class BraveSyncServiceTest : public testing::Test { base::ScopedTempDir temp_dir_; }; -class ServiceTestDeviceLeftover : public BraveSyncServiceTest { +class ServiceStartDeviceLeftoverTest : public BraveSyncServiceTest { public: void PreCreateService() override { // Emulating we have one device and seed saved, like inthe case when // the browser was closed righ after starting procedure of sync chain // creation without waiting for the second device to connect - const std::string devices_json = - R"json( - { - "devices": - [{ - "device_id":"0", - "last_active":1.571261287102e+12, - "name":"COMPUTER", - "object_id":"67, 22, 139, 42, 47, 37, 218, 215, - 254, 228, 11, 42, 192, 73, 99, 47" - }] - } - )json"; - profile()->GetPrefs()->SetString(brave_sync::prefs::kSyncDeviceList, - devices_json); - profile()->GetPrefs()->SetString(brave_sync::prefs::kSyncSeed, "1, 2, 3"); + PreparePrefsStateDeviceLeftover(profile()->GetPrefs()); + } +}; + +class ServiceShotdownDeviceLeftoverTest : public BraveSyncServiceTest { + public: + void PostShotdownService() override { + EXPECT_EQ(brave_sync_prefs()->GetSyncDevices()->size(), 0u); } }; @@ -1011,7 +1026,7 @@ TEST_F(BraveSyncServiceTest, GetDevicesWithFetchSyncRecords) { } } -TEST_F(ServiceTestDeviceLeftover, OnSetupSyncHaveCodeDeviceLeftover) { +TEST_F(ServiceStartDeviceLeftoverTest, OnSetupSyncHaveCodeDeviceLeftover) { EXPECT_TRUE(sync_service()->reseting_); EXPECT_EQ(brave_sync_prefs()->GetSyncDevices()->size(), 0u); @@ -1023,3 +1038,11 @@ TEST_F(ServiceTestDeviceLeftover, OnSetupSyncHaveCodeDeviceLeftover) { profile()->GetPrefs()->GetBoolean(brave_sync::prefs::kSyncEnabled)); EXPECT_FALSE(sync_service()->reseting_); } + +TEST_F(ServiceShotdownDeviceLeftoverTest, OnShutdownDeviceLeftover) { + // Configure state of enabled sync and just only one device in the chain + // before service shotdown + PreparePrefsStateDeviceLeftover(profile()->GetPrefs()); + // Will verify in ServiceShotdownDeviceLeftoverTest::PostShotdownService + // we did a proper cleanup +} From b560dc49c9e125baa6e5c445668ce73146e749dc Mon Sep 17 00:00:00 2001 From: AlexeyBarabash Date: Tue, 5 Nov 2019 12:38:22 +0200 Subject: [PATCH 5/6] Save sync prefs in memory if chain is not fully created --- components/brave_sync/BUILD.gn | 9 +- .../brave_profile_sync_service_impl.cc | 66 +++-- .../brave_profile_sync_service_impl.h | 10 +- components/brave_sync/brave_sync_prefs.h | 98 ++++--- components/brave_sync/brave_sync_prefs_base.h | 88 ++++++ .../brave_sync/brave_sync_prefs_mem_store.cc | 271 ++++++++++++++++++ .../brave_sync/brave_sync_prefs_mem_store.h | 124 ++++++++ .../brave_sync/client/brave_sync_client.h | 2 +- .../client/brave_sync_client_impl.cc | 12 +- .../client/brave_sync_client_impl.h | 2 +- 10 files changed, 600 insertions(+), 82 deletions(-) create mode 100644 components/brave_sync/brave_sync_prefs_base.h create mode 100644 components/brave_sync/brave_sync_prefs_mem_store.cc create mode 100644 components/brave_sync/brave_sync_prefs_mem_store.h diff --git a/components/brave_sync/BUILD.gn b/components/brave_sync/BUILD.gn index e87dcdf5f59d..0f5801ad6056 100644 --- a/components/brave_sync/BUILD.gn +++ b/components/brave_sync/BUILD.gn @@ -35,8 +35,8 @@ if (enable_brave_sync) { "//components/prefs", "//components/prefs", "//components/signin/public/identity_manager", - "//components/sync/driver:driver", "//components/sync:rest_of_sync", + "//components/sync/driver:driver", "//content/public/browser", "//extensions/browser", "//net", @@ -64,6 +64,9 @@ source_set("prefs") { sources = [ "brave_sync_prefs.cc", "brave_sync_prefs.h", + "brave_sync_prefs_base.h", + "brave_sync_prefs_mem_store.cc", + "brave_sync_prefs_mem_store.h", "settings.cc", "settings.h", "sync_devices.cc", @@ -100,9 +103,7 @@ source_set("crypto") { ] if (is_android) { - deps += [ - "//third_party/android_sdk:cpu_features", - ] + deps += [ "//third_party/android_sdk:cpu_features" ] } } diff --git a/components/brave_sync/brave_profile_sync_service_impl.cc b/components/brave_sync/brave_profile_sync_service_impl.cc index a5061fce9676..6c4f7b1cb22a 100644 --- a/components/brave_sync/brave_profile_sync_service_impl.cc +++ b/components/brave_sync/brave_profile_sync_service_impl.cc @@ -121,6 +121,7 @@ std::unique_ptr CreateDeleteBookmarkByObjectId( const std::string& object_id) { auto record = std::make_unique(); record->deviceId = brave_sync_prefs->GetThisDeviceId(); + DCHECK(!record->deviceId.empty()); record->objectData = jslib_const::SyncObjectData_BOOKMARK; record->objectId = object_id; record->action = jslib::SyncRecord::Action::A_DELETE; @@ -168,8 +169,9 @@ BraveProfileSyncServiceImpl::BraveProfileSyncServiceImpl(Profile* profile, : BraveProfileSyncService(std::move(init_params)), brave_sync_client_(BraveSyncClient::Create(this, profile)) { brave_sync_words_ = std::string(); - brave_sync_prefs_ = + brave_sync_prefs_disk_store_ = std::make_unique(sync_client_->GetPrefService()); + brave_sync_prefs_ = brave_sync_prefs_disk_store_.get(); if (IsSyncSetupIncomplete()) { // We want to reset the chain because it was not fully created @@ -182,6 +184,24 @@ BraveProfileSyncServiceImpl::BraveProfileSyncServiceImpl(Profile* profile, // Moniter syncs prefs required in GetSettingsAndDevices brave_pref_change_registrar_.Init(sync_client_->GetPrefService()); + + model_ = BookmarkModelFactory::GetForBrowserContext(profile); + + if (!brave_sync_prefs_->GetSeed().empty() && + !brave_sync_prefs_->GetThisDeviceName().empty()) { + brave_sync_configured_ = true; + SetupDiskPrefsObservers(); + } else { + brave_sync_prefs_ = &brave_sync_prefs_mem_store_; + brave_sync_prefs_mem_store_.AddObserver(base::BindRepeating( + &BraveProfileSyncServiceImpl::OnBraveSyncPrefsChanged, + base::Unretained(this))); + } + network_connection_tracker_->AddNetworkConnectionObserver(this); + RecordSyncStateP3A(); +} + +void BraveProfileSyncServiceImpl::SetupDiskPrefsObservers() { brave_pref_change_registrar_.Add( prefs::kSyncEnabled, base::Bind(&BraveProfileSyncServiceImpl::OnBraveSyncPrefsChanged, @@ -206,15 +226,6 @@ BraveProfileSyncServiceImpl::BraveProfileSyncServiceImpl(Profile* profile, prefs::kSyncHistoryEnabled, base::Bind(&BraveProfileSyncServiceImpl::OnBraveSyncPrefsChanged, base::Unretained(this))); - - model_ = BookmarkModelFactory::GetForBrowserContext(profile); - - if (!brave_sync_prefs_->GetSeed().empty() && - !brave_sync_prefs_->GetThisDeviceName().empty()) { - brave_sync_configured_ = true; - } - network_connection_tracker_->AddNetworkConnectionObserver(this); - RecordSyncStateP3A(); } void BraveProfileSyncServiceImpl::OnNudgeSyncCycle(RecordsListPtr records) { @@ -223,6 +234,7 @@ void BraveProfileSyncServiceImpl::OnNudgeSyncCycle(RecordsListPtr records) { for (auto& record : *records) { record->deviceId = brave_sync_prefs_->GetThisDeviceId(); + DCHECK(!record->deviceId.empty()); } if (!records->empty()) { if (((!brave_sync::tools::IsTimeEmpty(chain_created_time_) && @@ -340,6 +352,7 @@ void BraveProfileSyncServiceImpl::OnResetSync() { // We have to send delete record and wait for library deleted response then // we can reset it by ResetSyncInternal() const std::string device_id = brave_sync_prefs_->GetThisDeviceId(); + DCHECK(!device_id.empty()); OnDeleteDevice(device_id); reseting_ = true; } @@ -644,15 +657,6 @@ void BraveProfileSyncServiceImpl::OnConnectionChanged( } void BraveProfileSyncServiceImpl::Shutdown() { - if (IsSyncSetupIncomplete()) { - // Need to clear prefs to allow UI page give us code words next time - // But we want to avoid unload of Extension in d`tor - brave_pref_change_registrar_.RemoveAll(); - // This will clear prefs and prepare Chromium sync for the proper enabling - // when sync will be turned on next time - ResetSyncInternal(); - } - SignalWaitableEvent(); syncer::ProfileSyncService::Shutdown(); } @@ -700,6 +704,11 @@ void BraveProfileSyncServiceImpl::ResetSyncInternal() { void BraveProfileSyncServiceImpl::ForceCompleteReset() { if (reseting_) { ResetSyncInternal(); + } else { + // Reset only Chromium part + ProfileSyncService::GetUserSettings()->SetSyncRequested(false); + syncer::SyncPrefs sync_prefs(sync_client_->GetPrefService()); + sync_prefs.SetLastSyncedTime(base::Time()); } } @@ -888,6 +897,8 @@ void BraveProfileSyncServiceImpl::SendDeviceSyncRecord( void BraveProfileSyncServiceImpl::OnResolvedPreferences( const RecordsList& records) { const std::string this_device_id = brave_sync_prefs_->GetThisDeviceId(); + DCHECK(!this_device_id.empty()); + bool this_device_deleted = false; bool contains_only_one_device = false; @@ -916,6 +927,7 @@ void BraveProfileSyncServiceImpl::OnResolvedPreferences( if (old_devices_size < 2 && sync_devices->size() >= 2) { // Save chain creation time to send bookmarks 30 sec after chain_created_time_ = base::Time::Now(); + SwitchToDiskPrefs(); } if (!tools::IsTimeEmpty(chain_created_time_) && (base::Time::Now() - chain_created_time_).InSeconds() > 30u) { @@ -935,7 +947,8 @@ void BraveProfileSyncServiceImpl::OnResolvedPreferences( void BraveProfileSyncServiceImpl::OnBraveSyncPrefsChanged( const std::string& pref) { if (pref == prefs::kSyncEnabled) { - brave_sync_client_->OnSyncEnabledChanged(); + bool is_enabled = brave_sync_prefs_->GetSyncEnabled(); + brave_sync_client_->OnSyncEnabledChanged(is_enabled); RecordSyncStateP3A(); } else if (pref == prefs::kSyncDeviceList) { RecordSyncStateP3A(); @@ -1089,8 +1102,8 @@ void BraveProfileSyncServiceImpl::ResendSyncRecords( if (node) { records->push_back(BookmarkNodeToSyncBookmark(node)); } else { - records->push_back( - CreateDeleteBookmarkByObjectId(brave_sync_prefs_.get(), object_id)); + records->push_back(CreateDeleteBookmarkByObjectId( + brave_sync_prefs_disk_store_.get(), object_id)); } } if (!records->empty()) @@ -1127,4 +1140,13 @@ void BraveProfileSyncServiceImpl::RecordSyncStateP3A() const { UMA_HISTOGRAM_EXACT_LINEAR("Brave.Sync.Status", result, 2); } +void BraveProfileSyncServiceImpl::SwitchToDiskPrefs() { + brave_sync_prefs_ = brave_sync_prefs_disk_store_.get(); + brave_sync::prefs::CloneMemPrefsToDisk(&brave_sync_prefs_mem_store_, + brave_sync_prefs_disk_store_.get()); + SetupDiskPrefsObservers(); + brave_sync_prefs_mem_store_.ResetObserver(); + brave_sync_prefs_mem_store_.Clear(); +} + } // namespace brave_sync diff --git a/components/brave_sync/brave_profile_sync_service_impl.h b/components/brave_sync/brave_profile_sync_service_impl.h index d1c99964f63d..660433a8ab42 100644 --- a/components/brave_sync/brave_profile_sync_service_impl.h +++ b/components/brave_sync/brave_profile_sync_service_impl.h @@ -10,6 +10,7 @@ #include #include +#include "brave/components/brave_sync/brave_sync_prefs_mem_store.h" #include "brave/components/brave_sync/brave_sync_service.h" #include "brave/components/brave_sync/client/brave_sync_client.h" #include "brave/components/brave_sync/jslib_messages_fwd.h" @@ -48,6 +49,7 @@ class BraveSyncServiceTest; namespace brave_sync { namespace prefs { class Prefs; +class PrefsBase; } // namespace prefs class BraveProfileSyncServiceImpl @@ -203,7 +205,10 @@ class BraveProfileSyncServiceImpl static const std::vector kExponentialWaits; static const int kMaxSendRetries; - std::unique_ptr brave_sync_prefs_; + brave_sync::prefs::PrefsBase* brave_sync_prefs_ = nullptr; + std::unique_ptr brave_sync_prefs_disk_store_; + brave_sync::prefs::PrefsMemStore brave_sync_prefs_mem_store_; + // True when is in active sync chain bool brave_sync_configured_ = false; @@ -231,6 +236,9 @@ class BraveProfileSyncServiceImpl base::Time chain_created_time_; std::vector pending_send_records_; + void SwitchToDiskPrefs(); + void SetupDiskPrefsObservers(); + // Used to ensure that certain operations are performed on the sequence that // this object was created on. SEQUENCE_CHECKER(sequence_checker_); diff --git a/components/brave_sync/brave_sync_prefs.h b/components/brave_sync/brave_sync_prefs.h index e137a4a9c95c..459123cfeb81 100644 --- a/components/brave_sync/brave_sync_prefs.h +++ b/components/brave_sync/brave_sync_prefs.h @@ -12,6 +12,7 @@ #include "base/macros.h" #include "base/values.h" +#include "brave/components/brave_sync/brave_sync_prefs_base.h" class PrefService; class Profile; @@ -70,60 +71,63 @@ extern const char kSyncRecordsToResend[]; // Meta info of kSyncRecordsToResend extern const char kSyncRecordsToResendMeta[]; -class Prefs { +class Prefs : public PrefsBase { public: explicit Prefs(PrefService* pref_service); + virtual ~Prefs() = default; static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry); - std::string GetSeed() const; - void SetSeed(const std::string& seed); - std::string GetPrevSeed() const; - void SetPrevSeed(const std::string& seed); - std::string GetThisDeviceId() const; - void SetThisDeviceId(const std::string& device_id); - std::string GetThisDeviceName() const; - void SetThisDeviceName(const std::string& device_name); - std::string GetBookmarksBaseOrder(); - void SetBookmarksBaseOrder(const std::string& order); - - bool GetSyncEnabled() const; - void SetSyncEnabled(const bool sync_this_device); - bool GetSyncBookmarksEnabled() const; - void SetSyncBookmarksEnabled(const bool sync_bookmarks_enabled); - bool GetSyncSiteSettingsEnabled() const; - void SetSyncSiteSettingsEnabled(const bool sync_site_settings); - bool GetSyncHistoryEnabled() const; - void SetSyncHistoryEnabled(const bool sync_history_enabled); - - void SetLatestRecordTime(const base::Time &time); - base::Time GetLatestRecordTime(); - void SetLatestDeviceRecordTime(const base::Time& time); - base::Time GetLatestDeviceRecordTime(); - void SetLastFetchTime(const base::Time &time); - base::Time GetLastFetchTime(); - - std::unique_ptr GetSyncDevices(); - void SetSyncDevices(const SyncDevices& sync_devices); - - std::string GetApiVersion(); - void SetApiVersion(const std::string& api_version); - - std::unique_ptr GetBraveSyncSettings() const; - - int GetMigratedBookmarksVersion(); - void SetMigratedBookmarksVersion(const int); - - std::vector GetRecordsToResend() const; - void AddToRecordsToResend(const std::string& object_id, - std::unique_ptr meta); - void RemoveFromRecordsToResend(const std::string& object_id); + std::string GetSeed() const override; + void SetSeed(const std::string& seed) override; + std::string GetPrevSeed() const override; + void SetPrevSeed(const std::string& seed) override; + std::string GetThisDeviceId() const override; + void SetThisDeviceId(const std::string& device_id) override; + std::string GetThisDeviceName() const override; + void SetThisDeviceName(const std::string& device_name) override; + std::string GetBookmarksBaseOrder() override; + void SetBookmarksBaseOrder(const std::string& order) override; + + bool GetSyncEnabled() const override; + void SetSyncEnabled(const bool sync_this_device) override; + bool GetSyncBookmarksEnabled() const override; + void SetSyncBookmarksEnabled(const bool sync_bookmarks_enabled) override; + bool GetSyncSiteSettingsEnabled() const override; + void SetSyncSiteSettingsEnabled(const bool sync_site_settings) override; + bool GetSyncHistoryEnabled() const override; + void SetSyncHistoryEnabled(const bool sync_history_enabled) override; + + void SetLatestRecordTime(const base::Time& time) override; + base::Time GetLatestRecordTime() override; + void SetLatestDeviceRecordTime(const base::Time& time) override; + base::Time GetLatestDeviceRecordTime() override; + void SetLastFetchTime(const base::Time& time) override; + base::Time GetLastFetchTime() override; + + std::unique_ptr GetSyncDevices() override; + void SetSyncDevices(const SyncDevices& sync_devices) override; + + std::string GetApiVersion() override; + void SetApiVersion(const std::string& api_version) override; + + std::unique_ptr GetBraveSyncSettings() const override; + + int GetMigratedBookmarksVersion() override; + void SetMigratedBookmarksVersion(const int) override; + + std::vector GetRecordsToResend() const override; + void AddToRecordsToResend( + const std::string& object_id, + std::unique_ptr meta) override; + void RemoveFromRecordsToResend(const std::string& object_id) override; const base::DictionaryValue* GetRecordToResendMeta( - const std::string& object_id) const; - void SetRecordToResendMeta(const std::string& object_id, - std::unique_ptr meta); + const std::string& object_id) const override; + void SetRecordToResendMeta( + const std::string& object_id, + std::unique_ptr meta) override; - void Clear(); + void Clear() override; private: // May be null. diff --git a/components/brave_sync/brave_sync_prefs_base.h b/components/brave_sync/brave_sync_prefs_base.h new file mode 100644 index 000000000000..170b91131c88 --- /dev/null +++ b/components/brave_sync/brave_sync_prefs_base.h @@ -0,0 +1,88 @@ +/* Copyright 2019 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_COMPONENTS_BRAVE_SYNC_BRAVE_SYNC_PREFS_BASE_H_ +#define BRAVE_COMPONENTS_BRAVE_SYNC_BRAVE_SYNC_PREFS_BASE_H_ + +#include +#include +#include + +#include "base/macros.h" +#include "base/values.h" + +namespace base { +class Time; +} + +namespace user_prefs { +class PrefRegistrySyncable; +} + +namespace brave_sync { + +class Settings; +class SyncDevices; + +namespace prefs { + +class PrefsBase { + public: + virtual std::string GetSeed() const = 0; + virtual void SetSeed(const std::string& seed) = 0; + virtual std::string GetPrevSeed() const = 0; + virtual void SetPrevSeed(const std::string& seed) = 0; + virtual std::string GetThisDeviceId() const = 0; + virtual void SetThisDeviceId(const std::string& device_id) = 0; + virtual std::string GetThisDeviceName() const = 0; + virtual void SetThisDeviceName(const std::string& device_name) = 0; + virtual std::string GetBookmarksBaseOrder() = 0; + virtual void SetBookmarksBaseOrder(const std::string& order) = 0; + + virtual bool GetSyncEnabled() const = 0; + virtual void SetSyncEnabled(const bool sync_this_device) = 0; + virtual bool GetSyncBookmarksEnabled() const = 0; + virtual void SetSyncBookmarksEnabled(const bool sync_bookmarks_enabled) = 0; + virtual bool GetSyncSiteSettingsEnabled() const = 0; + virtual void SetSyncSiteSettingsEnabled(const bool sync_site_settings) = 0; + virtual bool GetSyncHistoryEnabled() const = 0; + virtual void SetSyncHistoryEnabled(const bool sync_history_enabled) = 0; + + virtual void SetLatestRecordTime(const base::Time& time) = 0; + virtual base::Time GetLatestRecordTime() = 0; + virtual void SetLatestDeviceRecordTime(const base::Time& time) = 0; + virtual base::Time GetLatestDeviceRecordTime() = 0; + virtual void SetLastFetchTime(const base::Time& time) = 0; + virtual base::Time GetLastFetchTime() = 0; + + virtual std::unique_ptr GetSyncDevices() = 0; + virtual void SetSyncDevices(const SyncDevices& sync_devices) = 0; + + virtual std::string GetApiVersion() = 0; + virtual void SetApiVersion(const std::string& api_version) = 0; + + virtual std::unique_ptr GetBraveSyncSettings() const = 0; + + virtual int GetMigratedBookmarksVersion() = 0; + virtual void SetMigratedBookmarksVersion(const int) = 0; + + virtual std::vector GetRecordsToResend() const = 0; + virtual void AddToRecordsToResend( + const std::string& object_id, + std::unique_ptr meta) = 0; + virtual void RemoveFromRecordsToResend(const std::string& object_id) = 0; + virtual const base::DictionaryValue* GetRecordToResendMeta( + const std::string& object_id) const = 0; + virtual void SetRecordToResendMeta( + const std::string& object_id, + std::unique_ptr meta) = 0; + + virtual void Clear() = 0; +}; + +} // namespace prefs +} // namespace brave_sync + +#endif // BRAVE_COMPONENTS_BRAVE_SYNC_BRAVE_SYNC_PREFS_BASE_H_ diff --git a/components/brave_sync/brave_sync_prefs_mem_store.cc b/components/brave_sync/brave_sync_prefs_mem_store.cc new file mode 100644 index 000000000000..cbee5a926627 --- /dev/null +++ b/components/brave_sync/brave_sync_prefs_mem_store.cc @@ -0,0 +1,271 @@ +/* Copyright 2019 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "brave/components/brave_sync/brave_sync_prefs_mem_store.h" + +#include + +#include "brave/components/brave_sync/brave_sync_prefs.h" +#include "brave/components/brave_sync/settings.h" +#include "brave/components/brave_sync/sync_devices.h" +#include "components/pref_registry/pref_registry_syncable.h" +#include "components/prefs/pref_service.h" +#include "components/prefs/scoped_user_pref_update.h" + +namespace brave_sync { +namespace prefs { + +PrefsMemStore::PrefsMemStore() {} + +PrefsMemStore::~PrefsMemStore() { + ResetObserver(); +} + +void PrefsMemStore::AddObserver(const NamedChangeCallback& obs) { + obs_ = obs; +} + +void PrefsMemStore::ResetObserver() { + obs_.Reset(); +} + +std::string PrefsMemStore::GetSeed() const { + return seed_; +} + +void PrefsMemStore::SetSeed(const std::string& seed) { + DCHECK(!seed.empty()); + seed_ = seed; +} + +std::string PrefsMemStore::GetPrevSeed() const { + return prev_seed_; +} + +void PrefsMemStore::SetPrevSeed(const std::string& seed) { + // This may be invoked by tests + prev_seed_ = seed; +} + +std::string PrefsMemStore::GetThisDeviceId() const { + return this_device_id_; +} + +void PrefsMemStore::SetThisDeviceId(const std::string& device_id) { + DCHECK(!device_id.empty()); + this_device_id_ = device_id; +} + +std::string PrefsMemStore::GetThisDeviceName() const { + return this_device_name_; +} + +void PrefsMemStore::SetThisDeviceName(const std::string& device_name) { + DCHECK(!device_name.empty()); + bool should_fire_callback = (this_device_name_ != device_name); + this_device_name_ = device_name; + if (should_fire_callback) + FireCallback(prefs::kSyncDeviceName); +} + +std::string PrefsMemStore::GetBookmarksBaseOrder() { + return bookmark_base_order_; +} +void PrefsMemStore::SetBookmarksBaseOrder(const std::string& order) { + bookmark_base_order_ = order; +} + +bool PrefsMemStore::GetSyncEnabled() const { + return sync_enabled_; +} + +void PrefsMemStore::SetSyncEnabled(const bool sync_this_device) { + bool should_fire_callback = (sync_enabled_ != sync_this_device); + sync_enabled_ = sync_this_device; + if (should_fire_callback) + FireCallback(prefs::kSyncEnabled); +} + +bool PrefsMemStore::GetSyncBookmarksEnabled() const { + return bookmarks_enabled_; +} + +void PrefsMemStore::SetSyncBookmarksEnabled(const bool sync_bookmarks_enabled) { + bool should_fire_callback = (bookmarks_enabled_ != sync_bookmarks_enabled); + bookmarks_enabled_ = sync_bookmarks_enabled; + if (should_fire_callback) + FireCallback(prefs::kSyncBookmarksEnabled); +} + +bool PrefsMemStore::GetSyncSiteSettingsEnabled() const { + return site_settings_enabled_; +} + +void PrefsMemStore::SetSyncSiteSettingsEnabled( + const bool sync_site_settings_enabled) { + bool should_fire_callback = + (site_settings_enabled_ != sync_site_settings_enabled); + site_settings_enabled_ = sync_site_settings_enabled; + if (should_fire_callback) + FireCallback(prefs::kSyncSiteSettingsEnabled); +} + +bool PrefsMemStore::GetSyncHistoryEnabled() const { + return history_enabled_; +} + +void PrefsMemStore::SetSyncHistoryEnabled(const bool sync_history_enabled) { + bool should_fire_callback = (history_enabled_ != sync_history_enabled); + history_enabled_ = sync_history_enabled; + if (should_fire_callback) + FireCallback(prefs::kSyncHistoryEnabled); +} + +std::unique_ptr PrefsMemStore::GetBraveSyncSettings() + const { + auto settings = std::make_unique(); + + settings->this_device_name_ = GetThisDeviceName(); + settings->this_device_id_ = GetThisDeviceId(); + settings->sync_this_device_ = GetSyncEnabled(); + settings->sync_bookmarks_ = GetSyncBookmarksEnabled(); + settings->sync_settings_ = GetSyncSiteSettingsEnabled(); + settings->sync_history_ = GetSyncHistoryEnabled(); + + settings->sync_configured_ = + !GetSeed().empty() && !GetThisDeviceName().empty(); + + return settings; +} + +void PrefsMemStore::SetLatestRecordTime(const base::Time& time) { + latest_record_time_ = time; +} + +base::Time PrefsMemStore::GetLatestRecordTime() { + return latest_record_time_; +} + +void PrefsMemStore::SetLatestDeviceRecordTime(const base::Time& time) { + latest_device_record_time_ = time; +} + +base::Time PrefsMemStore::GetLatestDeviceRecordTime() { + return latest_device_record_time_; +} + +void PrefsMemStore::SetLastFetchTime(const base::Time& time) { + last_fetch_time_ = time; +} + +base::Time PrefsMemStore::GetLastFetchTime() { + return last_fetch_time_; +} + +std::unique_ptr PrefsMemStore::GetSyncDevices() { + auto existing_sync_devices = std::make_unique(); + if (!json_device_list_.empty()) + existing_sync_devices->FromJson(json_device_list_); + + return existing_sync_devices; +} + +void PrefsMemStore::SetSyncDevices(const SyncDevices& devices) { + const std::string json_device_list_to_set = devices.ToJson(); + bool should_fire_callback = (json_device_list_ != json_device_list_to_set); + json_device_list_ = json_device_list_to_set; + if (should_fire_callback) + FireCallback(prefs::kSyncDeviceList); +} + +std::string PrefsMemStore::GetApiVersion() { + return api_version_; +} + +void PrefsMemStore::SetApiVersion(const std::string& api_version) { + api_version_ = api_version; +} + +int PrefsMemStore::GetMigratedBookmarksVersion() { + return migrate_bookmarks_version_; +} + +void PrefsMemStore::SetMigratedBookmarksVersion(const int migrate_bookmarks) { + migrate_bookmarks_version_ = migrate_bookmarks; +} + +std::vector PrefsMemStore::GetRecordsToResend() const { + // Until sync chain is fully created we never have records to resend + return std::vector(); +} + +void PrefsMemStore::AddToRecordsToResend( + const std::string& object_id, + std::unique_ptr meta) { + NOTREACHED(); +} + +void PrefsMemStore::RemoveFromRecordsToResend(const std::string& object_id) { + NOTREACHED(); +} + +const base::DictionaryValue* PrefsMemStore::GetRecordToResendMeta( + const std::string& object_id) const { + NOTREACHED(); + return nullptr; +} + +void PrefsMemStore::SetRecordToResendMeta( + const std::string& object_id, + std::unique_ptr meta) { + NOTREACHED(); +} + +void PrefsMemStore::Clear() { + seed_.clear(); + this_device_id_.clear(); + this_device_name_.clear(); + sync_enabled_ = false; + json_device_list_.clear(); + bookmarks_enabled_ = false; + site_settings_enabled_ = false; + history_enabled_ = false; + + migrate_bookmarks_version_ = 0; + api_version_.clear(); +} + +void PrefsMemStore::FireCallback(const std::string& name) { + if (obs_) { + obs_.Run(name); + } +} + +void CloneMemPrefsToDisk(PrefsMemStore* prefs_mem, PrefsBase* prefs_disk) { + prefs_disk->SetSeed(prefs_mem->GetSeed()); + prefs_disk->SetPrevSeed(prefs_mem->GetPrevSeed()); + prefs_disk->SetThisDeviceId(prefs_mem->GetThisDeviceId()); + prefs_disk->SetThisDeviceName(prefs_mem->GetThisDeviceName()); + prefs_disk->SetSyncEnabled(prefs_mem->GetSyncEnabled()); + prefs_disk->SetSyncDevices(*(prefs_mem->GetSyncDevices())); + + prefs_disk->SetMigratedBookmarksVersion( + prefs_mem->GetMigratedBookmarksVersion()); + prefs_disk->SetApiVersion(prefs_mem->GetApiVersion()); + prefs_disk->SetBookmarksBaseOrder(prefs_mem->GetBookmarksBaseOrder()); + + prefs_disk->SetLastFetchTime(prefs_mem->GetLastFetchTime()); + + prefs_disk->SetLatestDeviceRecordTime(prefs_mem->GetLatestDeviceRecordTime()); + prefs_disk->SetLatestDeviceRecordTime(prefs_mem->GetLatestDeviceRecordTime()); + + prefs_disk->SetSyncBookmarksEnabled(prefs_mem->GetSyncBookmarksEnabled()); + prefs_disk->SetSyncSiteSettingsEnabled( + prefs_mem->GetSyncSiteSettingsEnabled()); + prefs_disk->SetSyncHistoryEnabled(prefs_mem->GetSyncHistoryEnabled()); +} + +} // namespace prefs +} // namespace brave_sync diff --git a/components/brave_sync/brave_sync_prefs_mem_store.h b/components/brave_sync/brave_sync_prefs_mem_store.h new file mode 100644 index 000000000000..1cfefe1df662 --- /dev/null +++ b/components/brave_sync/brave_sync_prefs_mem_store.h @@ -0,0 +1,124 @@ +/* Copyright 2019 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_COMPONENTS_BRAVE_SYNC_BRAVE_SYNC_PREFS_MEM_STORE_H_ +#define BRAVE_COMPONENTS_BRAVE_SYNC_BRAVE_SYNC_PREFS_MEM_STORE_H_ + +#include +#include +#include + +#include "base/callback.h" +#include "base/macros.h" +#include "base/values.h" +#include "brave/components/brave_sync/brave_sync_prefs_base.h" + +namespace base { +class Time; +} + +namespace brave_sync { + +class Settings; +class SyncDevices; + +namespace prefs { + +class PrefsMemStore : public PrefsBase { + public: + using NamedChangeCallback = base::RepeatingCallback; + + PrefsMemStore(); + virtual ~PrefsMemStore(); + + void AddObserver(const NamedChangeCallback& obs); + void ResetObserver(); + + std::string GetSeed() const override; + void SetSeed(const std::string& seed) override; + std::string GetPrevSeed() const override; + void SetPrevSeed(const std::string& seed) override; + std::string GetThisDeviceId() const override; + void SetThisDeviceId(const std::string& device_id) override; + std::string GetThisDeviceName() const override; + void SetThisDeviceName(const std::string& device_name) override; + std::string GetBookmarksBaseOrder() override; + void SetBookmarksBaseOrder(const std::string& order) override; + + bool GetSyncEnabled() const override; + void SetSyncEnabled(const bool sync_this_device) override; + bool GetSyncBookmarksEnabled() const override; + void SetSyncBookmarksEnabled(const bool sync_bookmarks_enabled) override; + bool GetSyncSiteSettingsEnabled() const override; + void SetSyncSiteSettingsEnabled(const bool sync_site_settings) override; + bool GetSyncHistoryEnabled() const override; + void SetSyncHistoryEnabled(const bool sync_history_enabled) override; + + void SetLatestRecordTime(const base::Time& time) override; + base::Time GetLatestRecordTime() override; + void SetLatestDeviceRecordTime(const base::Time& time) override; + base::Time GetLatestDeviceRecordTime() override; + void SetLastFetchTime(const base::Time& time) override; + base::Time GetLastFetchTime() override; + + std::unique_ptr GetSyncDevices() override; + void SetSyncDevices(const SyncDevices& sync_devices) override; + + std::string GetApiVersion() override; + void SetApiVersion(const std::string& api_version) override; + + std::unique_ptr GetBraveSyncSettings() const override; + + int GetMigratedBookmarksVersion() override; + void SetMigratedBookmarksVersion(const int) override; + + std::vector GetRecordsToResend() const override; + void AddToRecordsToResend( + const std::string& object_id, + std::unique_ptr meta) override; + void RemoveFromRecordsToResend(const std::string& object_id) override; + const base::DictionaryValue* GetRecordToResendMeta( + const std::string& object_id) const override; + void SetRecordToResendMeta( + const std::string& object_id, + std::unique_ptr meta) override; + + void Clear() override; + + private: + std::string seed_; + std::string prev_seed_; + std::string this_device_id_; + std::string this_device_name_; + bool sync_enabled_ = false; + std::string json_device_list_; + + bool bookmarks_enabled_ = false; + bool site_settings_enabled_ = false; + bool history_enabled_ = false; + + int migrate_bookmarks_version_ = 0; + std::string api_version_; + std::string bookmark_base_order_; + + base::Time last_fetch_time_; + // Bookmarks are always enabled on start of sync + base::Time latest_record_time_; + // We are using latest device record until chain is not fully created + base::Time latest_device_record_time_; + + NamedChangeCallback obs_; + void FireCallback(const std::string& name); + + PrefsMemStore(const PrefsMemStore&) = delete; + PrefsMemStore& operator=(const PrefsMemStore&) = delete; +}; + +void CloneMemPrefsToDisk(PrefsMemStore* prefs_mem, PrefsBase* prefs_disk); + +} // namespace prefs +} // namespace brave_sync + +#endif // BRAVE_COMPONENTS_BRAVE_SYNC_BRAVE_SYNC_PREFS_MEM_STORE_H_ diff --git a/components/brave_sync/client/brave_sync_client.h b/components/brave_sync/client/brave_sync_client.h index 34049ca6d540..a7515e423ffe 100644 --- a/components/brave_sync/client/brave_sync_client.h +++ b/components/brave_sync/client/brave_sync_client.h @@ -81,7 +81,7 @@ class BraveSyncClient { virtual void OnExtensionInitialized() = 0; - virtual void OnSyncEnabledChanged() = 0; + virtual void OnSyncEnabledChanged(bool is_enabled) = 0; }; } // namespace brave_sync diff --git a/components/brave_sync/client/brave_sync_client_impl.cc b/components/brave_sync/client/brave_sync_client_impl.cc index 76aafab0f47e..ebc37d7276b0 100644 --- a/components/brave_sync/client/brave_sync_client_impl.cc +++ b/components/brave_sync/client/brave_sync_client_impl.cc @@ -12,14 +12,14 @@ #include "base/logging.h" #include "base/one_shot_event.h" #include "brave/browser/extensions/api/brave_sync_event_router.h" -#include "brave/components/brave_sync/client/client_ext_impl_data.h" -#include "brave/components/brave_sync/grit/brave_sync_resources.h" -#include "brave/components/brave_sync/brave_sync_prefs.h" #include "brave/common/extensions/api/brave_sync.h" #include "brave/common/extensions/extension_constants.h" -#include "chrome/browser/profiles/profile.h" +#include "brave/components/brave_sync/brave_sync_prefs.h" +#include "brave/components/brave_sync/client/client_ext_impl_data.h" +#include "brave/components/brave_sync/grit/brave_sync_resources.h" #include "chrome/browser/extensions/component_loader.h" #include "chrome/browser/extensions/extension_service.h" +#include "chrome/browser/profiles/profile.h" #include "content/public/browser/browser_thread.h" #include "extensions/browser/extension_registry.h" #include "extensions/browser/extension_system.h" @@ -126,9 +126,9 @@ void BraveSyncClientImpl::OnExtensionInitialized() { brave_sync_event_router_->LoadClient(); } -void BraveSyncClientImpl::OnSyncEnabledChanged() { +void BraveSyncClientImpl::OnSyncEnabledChanged(bool is_enabled) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); - if (sync_prefs_->GetSyncEnabled()) { + if (is_enabled) { LoadOrUnloadExtension(true); } else { LoadOrUnloadExtension(false); diff --git a/components/brave_sync/client/brave_sync_client_impl.h b/components/brave_sync/client/brave_sync_client_impl.h index 56c63e6b3d48..01ddaa038499 100644 --- a/components/brave_sync/client/brave_sync_client_impl.h +++ b/components/brave_sync/client/brave_sync_client_impl.h @@ -69,7 +69,7 @@ class BraveSyncClientImpl : public BraveSyncClient, explicit BraveSyncClientImpl(SyncMessageHandler* handler, Profile* profile); void OnExtensionInitialized() override; - void OnSyncEnabledChanged() override; + void OnSyncEnabledChanged(bool is_enabled) override; // ExtensionRegistryObserver: void OnExtensionReady(content::BrowserContext* browser_context, From 99a262be0f1f29b0e3eb71cf68e48bd0a298ead4 Mon Sep 17 00:00:00 2001 From: AlexeyBarabash Date: Tue, 5 Nov 2019 12:41:54 +0200 Subject: [PATCH 6/6] Updated testcases for saving sync prefs in memory if chain is not fully created --- .../brave_sync/brave_sync_service_unittest.cc | 168 +++++++++++++----- components/brave_sync/test_util.h | 2 +- 2 files changed, 129 insertions(+), 41 deletions(-) diff --git a/components/brave_sync/brave_sync_service_unittest.cc b/components/brave_sync/brave_sync_service_unittest.cc index f6956aeaf220..9a7c4b95a376 100644 --- a/components/brave_sync/brave_sync_service_unittest.cc +++ b/components/brave_sync/brave_sync_service_unittest.cc @@ -245,8 +245,14 @@ class BraveSyncServiceTest : public testing::Test { MockBraveSyncClient* sync_client() { return sync_client_; } BookmarkModel* model() { return model_; } MockBraveSyncServiceObserver* observer() { return observer_.get(); } - brave_sync::prefs::Prefs* brave_sync_prefs() { - return sync_service_->brave_sync_prefs_.get(); + brave_sync::prefs::PrefsBase* brave_sync_prefs() { + return sync_service_->brave_sync_prefs_; + } + brave_sync::prefs::Prefs* brave_sync_prefs_disk() { + return sync_service_->brave_sync_prefs_disk_store_.get(); + } + brave_sync::prefs::PrefsMemStore* brave_sync_prefs_mem() { + return &sync_service_->brave_sync_prefs_mem_store_; } syncer::SyncPrefs* sync_prefs() { return sync_prefs_.get(); } @@ -279,7 +285,8 @@ class ServiceStartDeviceLeftoverTest : public BraveSyncServiceTest { class ServiceShotdownDeviceLeftoverTest : public BraveSyncServiceTest { public: void PostShotdownService() override { - EXPECT_EQ(brave_sync_prefs()->GetSyncDevices()->size(), 0u); + EXPECT_EQ(brave_sync_prefs_disk()->GetSyncDevices()->size(), 0u); + EXPECT_NE(brave_sync_prefs_mem()->GetSyncDevices()->size(), 0u); } }; @@ -289,8 +296,9 @@ TEST_F(BraveSyncServiceTest, SetSyncEnabled) { EXPECT_FALSE( profile()->GetPrefs()->GetBoolean(brave_sync::prefs::kSyncEnabled)); sync_service()->OnSetSyncEnabled(true); - EXPECT_TRUE( + EXPECT_FALSE( profile()->GetPrefs()->GetBoolean(brave_sync::prefs::kSyncEnabled)); + EXPECT_TRUE(brave_sync_prefs()->GetSyncEnabled()); EXPECT_FALSE(sync_service()->IsBraveSyncInitialized()); EXPECT_FALSE(sync_service()->IsBraveSyncConfigured()); } @@ -299,12 +307,14 @@ TEST_F(BraveSyncServiceTest, SetSyncDisabled) { EXPECT_CALL(*sync_client(), OnSyncEnabledChanged).Times(1); EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(1); sync_service()->OnSetSyncEnabled(true); - EXPECT_TRUE( + EXPECT_FALSE( profile()->GetPrefs()->GetBoolean(brave_sync::prefs::kSyncEnabled)); + EXPECT_TRUE(brave_sync_prefs()->GetSyncEnabled()); EXPECT_CALL(*sync_client(), OnSyncEnabledChanged).Times(1); EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(1); sync_service()->OnSetSyncEnabled(false); + EXPECT_FALSE(brave_sync_prefs()->GetSyncEnabled()); EXPECT_FALSE( profile()->GetPrefs()->GetBoolean(brave_sync::prefs::kSyncEnabled)); EXPECT_FALSE(sync_service()->IsBraveSyncInitialized()); @@ -324,8 +334,9 @@ TEST_F(BraveSyncServiceTest, OnSetupSyncHaveCode) { // Expecting sync state changed twice: for enabled state and for device name EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(2); sync_service()->OnSetupSyncHaveCode("word1 word2 word3", "test_device"); - EXPECT_TRUE( + EXPECT_FALSE( profile()->GetPrefs()->GetBoolean(brave_sync::prefs::kSyncEnabled)); + EXPECT_TRUE(brave_sync_prefs()->GetSyncEnabled()); } TEST_F(BraveSyncServiceTest, OnSetupSyncHaveCodeEmptyDeviceName) { @@ -333,11 +344,12 @@ TEST_F(BraveSyncServiceTest, OnSetupSyncHaveCodeEmptyDeviceName) { // Expecting sync state changed twice: for enabled state and for device name EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(2); sync_service()->OnSetupSyncHaveCode("word1 word2 word3", ""); - EXPECT_TRUE( + EXPECT_FALSE( profile()->GetPrefs()->GetBoolean(brave_sync::prefs::kSyncEnabled)); + EXPECT_TRUE(brave_sync_prefs()->GetSyncEnabled()); EXPECT_EQ( - profile()->GetPrefs()->GetString(brave_sync::prefs::kSyncDeviceName), - net::GetHostName()); + profile()->GetPrefs()->GetString(brave_sync::prefs::kSyncDeviceName), ""); + EXPECT_EQ(brave_sync_prefs()->GetThisDeviceName(), net::GetHostName()); } TEST_F(BraveSyncServiceTest, OnSetupSyncNewToSync) { @@ -345,8 +357,9 @@ TEST_F(BraveSyncServiceTest, OnSetupSyncNewToSync) { // Expecting sync state changed twice: for enabled state and for device name EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(2); sync_service()->OnSetupSyncNewToSync("test_device"); - EXPECT_TRUE( + EXPECT_FALSE( profile()->GetPrefs()->GetBoolean(brave_sync::prefs::kSyncEnabled)); + EXPECT_TRUE(brave_sync_prefs()->GetSyncEnabled()); } TEST_F(BraveSyncServiceTest, OnSetupSyncNewToSyncEmptyDeviceName) { @@ -354,11 +367,12 @@ TEST_F(BraveSyncServiceTest, OnSetupSyncNewToSyncEmptyDeviceName) { // Expecting sync state changed twice: for enabled state and for device name EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(2); sync_service()->OnSetupSyncNewToSync(""); - EXPECT_TRUE( + EXPECT_FALSE( profile()->GetPrefs()->GetBoolean(brave_sync::prefs::kSyncEnabled)); + EXPECT_TRUE(brave_sync_prefs()->GetSyncEnabled()); EXPECT_EQ( - profile()->GetPrefs()->GetString(brave_sync::prefs::kSyncDeviceName), - net::GetHostName()); + profile()->GetPrefs()->GetString(brave_sync::prefs::kSyncDeviceName), ""); + EXPECT_EQ(brave_sync_prefs()->GetThisDeviceName(), net::GetHostName()); } TEST_F(BraveSyncServiceTest, GetSettingsAndDevices) { @@ -381,8 +395,9 @@ TEST_F(BraveSyncServiceTest, GetSettingsAndDevices) { // Expecting sync state changed twice: for enabled state and for device name EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(2); sync_service()->OnSetupSyncNewToSync("test_device"); - EXPECT_TRUE( + EXPECT_FALSE( profile()->GetPrefs()->GetBoolean(brave_sync::prefs::kSyncEnabled)); + EXPECT_TRUE(brave_sync_prefs()->GetSyncEnabled()); auto callback2 = base::BindRepeating([](std::unique_ptr settings, std::unique_ptr devices) { @@ -415,8 +430,9 @@ TEST_F(BraveSyncServiceTest, GetSeed) { EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())) .Times(AtLeast(2)); sync_service()->OnSetupSyncNewToSync("test_device"); - EXPECT_TRUE( + EXPECT_FALSE( profile()->GetPrefs()->GetBoolean(brave_sync::prefs::kSyncEnabled)); + EXPECT_TRUE(brave_sync_prefs()->GetSyncEnabled()); // Service gets seed from client via BraveSyncServiceImpl::OnSaveInitData const auto binary_seed = brave_sync::Uint8Array(16, 77); @@ -429,6 +445,11 @@ TEST_F(BraveSyncServiceTest, GetSeed) { } TEST_F(BraveSyncServiceTest, OnDeleteDevice) { + brave_sync_prefs()->SetSeed("10, 20, 30"); + EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(1); + brave_sync_prefs()->SetThisDeviceName("COMPUTER"); + brave_sync_prefs()->SetThisDeviceId("1"); + RecordsList records; records.push_back( SimpleDeviceRecord(SyncRecord::Action::A_CREATE, "1", "device1")); @@ -439,7 +460,6 @@ TEST_F(BraveSyncServiceTest, OnDeleteDevice) { EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(1); sync_service()->OnResolvedPreferences(records); - brave_sync_prefs()->SetThisDeviceId("1"); auto devices = brave_sync_prefs()->GetSyncDevices(); EXPECT_TRUE(DevicesContains(devices.get(), "1", "device1")); @@ -474,6 +494,9 @@ TEST_F(BraveSyncServiceTest, OnDeleteDevice) { } TEST_F(BraveSyncServiceTest, OnDeleteDeviceWhenOneDevice) { + brave_sync_prefs()->SetSeed("10, 20, 30"); + EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(1); + brave_sync_prefs()->SetThisDeviceName("device1"); brave_sync_prefs()->SetThisDeviceId("1"); RecordsList records; records.push_back( @@ -507,15 +530,25 @@ TEST_F(BraveSyncServiceTest, OnDeleteDeviceWhenOneDevice) { EXPECT_TRUE(DevicesContains(devices_semi_final.get(), "1", "device1")); // Enabled->Disabled - EXPECT_CALL(*sync_client(), OnSyncEnabledChanged).Times(1); + EXPECT_CALL(*sync_client(), OnSyncEnabledChanged).Times(1 + 1); // Emulate sending DELETE for this device RecordsList resolved_records2; auto resolved_record2 = SyncRecord::Clone(*records.at(0)); resolved_record2->action = SyncRecord::Action::A_DELETE; resolved_records2.push_back(std::move(resolved_record2)); - EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(3); - + // OnResolvedPreferences + // 1. kSyncDeviceList + // ResetSyncInternal() + // prefs->Clear() + // 2. kSyncDeviceName + // 3. kSyncEnabled + // 4. kSyncBookmarksEnabled + // 5. kSyncSiteSettingsEnabled + // 6. kSyncHistoryEnabled + // 7. kSyncDeviceList again + // 8. kSyncEnabled + EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(8); sync_service()->OnResolvedPreferences(resolved_records2); auto devices_final = brave_sync_prefs()->GetSyncDevices(); @@ -525,6 +558,9 @@ TEST_F(BraveSyncServiceTest, OnDeleteDeviceWhenOneDevice) { } TEST_F(BraveSyncServiceTest, OnDeleteDeviceWhenSelfDeleted) { + brave_sync_prefs()->SetSeed("10, 20, 30"); + EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(1); + brave_sync_prefs()->SetThisDeviceName("device1"); brave_sync_prefs()->SetThisDeviceId("1"); RecordsList records; records.push_back( @@ -547,14 +583,25 @@ TEST_F(BraveSyncServiceTest, OnDeleteDeviceWhenSelfDeleted) { sync_service()->OnDeleteDevice("1"); // Enabled->Disabled - EXPECT_CALL(*sync_client(), OnSyncEnabledChanged).Times(1); + EXPECT_CALL(*sync_client(), OnSyncEnabledChanged).Times(1 + 1); RecordsList resolved_records; auto resolved_record = SyncRecord::Clone(*records.at(0)); resolved_record->action = SyncRecord::Action::A_DELETE; resolved_records.push_back(std::move(resolved_record)); - // If you have to modify .Times(3) to another value, double re-check - EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(3); + // If you have to modify .Times(8) to another value, double re-check + // OnResolvedPreferences + // 1. kSyncDeviceList + // ResetSyncInternal() + // prefs->Clear() + // 2. kSyncDeviceName + // 3. kSyncEnabled + // 4. kSyncBookmarksEnabled + // 5. kSyncSiteSettingsEnabled + // 6. kSyncHistoryEnabled + // 7. kSyncDeviceList again + // 8. kSyncEnabled + EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(8); sync_service()->OnResolvedPreferences(resolved_records); auto devices_final = brave_sync_prefs()->GetSyncDevices(); @@ -565,12 +612,14 @@ TEST_F(BraveSyncServiceTest, OnDeleteDeviceWhenSelfDeleted) { } TEST_F(BraveSyncServiceTest, OnResetSync) { + brave_sync_prefs()->SetSeed("10, 20, 30"); EXPECT_CALL(*sync_client(), OnSyncEnabledChanged).Times(AtLeast(1)); EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())) .Times(AtLeast(3)); sync_service()->OnSetupSyncNewToSync("this_device"); - EXPECT_TRUE( + EXPECT_FALSE( profile()->GetPrefs()->GetBoolean(brave_sync::prefs::kSyncEnabled)); + EXPECT_TRUE(brave_sync_prefs()->GetSyncEnabled()); brave_sync_prefs()->SetThisDeviceId("0"); RecordsList records; @@ -652,15 +701,18 @@ TEST_F(BraveSyncServiceTest, OnResetSync) { TEST_F(BraveSyncServiceTest, OnSetSyncBookmarks) { EXPECT_FALSE(profile()->GetPrefs()->GetBoolean( brave_sync::prefs::kSyncBookmarksEnabled)); + EXPECT_FALSE(brave_sync_prefs()->GetSyncBookmarksEnabled()); EXPECT_CALL(*observer(), OnSyncStateChanged).Times(1); sync_service()->OnSetSyncBookmarks(true); EXPECT_TRUE(profile()->GetPrefs()->GetBoolean(syncer::prefs::kSyncBookmarks)); - EXPECT_TRUE(profile()->GetPrefs()->GetBoolean( + EXPECT_FALSE(profile()->GetPrefs()->GetBoolean( brave_sync::prefs::kSyncBookmarksEnabled)); + EXPECT_TRUE(brave_sync_prefs()->GetSyncBookmarksEnabled()); EXPECT_CALL(*observer(), OnSyncStateChanged).Times(1); sync_service()->OnSetSyncBookmarks(false); EXPECT_FALSE(profile()->GetPrefs()->GetBoolean( brave_sync::prefs::kSyncBookmarksEnabled)); + EXPECT_FALSE(brave_sync_prefs()->GetSyncBookmarksEnabled()); EXPECT_FALSE( profile()->GetPrefs()->GetBoolean(syncer::prefs::kSyncBookmarks)); EXPECT_CALL(*observer(), OnSyncStateChanged).Times(0); @@ -670,27 +722,33 @@ TEST_F(BraveSyncServiceTest, OnSetSyncBookmarks) { TEST_F(BraveSyncServiceTest, OnSetSyncBrowsingHistory) { EXPECT_FALSE(profile()->GetPrefs()->GetBoolean( brave_sync::prefs::kSyncHistoryEnabled)); + EXPECT_FALSE(brave_sync_prefs()->GetSyncHistoryEnabled()); EXPECT_CALL(*observer(), OnSyncStateChanged).Times(1); sync_service()->OnSetSyncBrowsingHistory(true); - EXPECT_TRUE(profile()->GetPrefs()->GetBoolean( + EXPECT_FALSE(profile()->GetPrefs()->GetBoolean( brave_sync::prefs::kSyncHistoryEnabled)); + EXPECT_TRUE(brave_sync_prefs()->GetSyncHistoryEnabled()); EXPECT_CALL(*observer(), OnSyncStateChanged).Times(1); sync_service()->OnSetSyncBrowsingHistory(false); EXPECT_FALSE(profile()->GetPrefs()->GetBoolean( brave_sync::prefs::kSyncHistoryEnabled)); + EXPECT_FALSE(brave_sync_prefs()->GetSyncHistoryEnabled()); } TEST_F(BraveSyncServiceTest, OnSetSyncSavedSiteSettings) { EXPECT_FALSE(profile()->GetPrefs()->GetBoolean( brave_sync::prefs::kSyncSiteSettingsEnabled)); + EXPECT_FALSE(brave_sync_prefs()->GetSyncSiteSettingsEnabled()); EXPECT_CALL(*observer(), OnSyncStateChanged).Times(1); sync_service()->OnSetSyncSavedSiteSettings(true); - EXPECT_TRUE(profile()->GetPrefs()->GetBoolean( + EXPECT_FALSE(profile()->GetPrefs()->GetBoolean( brave_sync::prefs::kSyncSiteSettingsEnabled)); + EXPECT_TRUE(brave_sync_prefs()->GetSyncSiteSettingsEnabled()); EXPECT_CALL(*observer(), OnSyncStateChanged).Times(1); sync_service()->OnSetSyncSavedSiteSettings(false); EXPECT_FALSE(profile()->GetPrefs()->GetBoolean( brave_sync::prefs::kSyncSiteSettingsEnabled)); + EXPECT_FALSE(brave_sync_prefs()->GetSyncSiteSettingsEnabled()); } TEST_F(BraveSyncServiceTest, OnGetInitData) { @@ -704,9 +762,8 @@ TEST_F(BraveSyncServiceTest, OnSaveBookmarksBaseOrder) { EXPECT_CALL(*observer(), OnSyncStateChanged).Times(2); sync_service()->OnSetSyncEnabled(true); sync_service()->OnSaveBookmarksBaseOrder("1.1."); - EXPECT_EQ(profile()->GetPrefs()->GetString( - brave_sync::prefs::kSyncBookmarksBaseOrder), - "1.1."); + EXPECT_EQ(brave_sync_prefs()->GetBookmarksBaseOrder(), "1.1."); + // Permanent node order std::string order; model()->bookmark_bar_node()->GetMetaInfo("order", &order); @@ -724,6 +781,9 @@ TEST_F(BraveSyncServiceTest, OnBraveSyncPrefsChanged) { } TEST_F(BraveSyncServiceTest, StartSyncNonDeviceRecords) { + brave_sync_prefs()->SetSeed("10, 20, 30"); + EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(1); + brave_sync_prefs()->SetThisDeviceName("device1"); EXPECT_FALSE(sync_service()->IsBraveSyncInitialized()); EXPECT_CALL(*sync_client(), OnSyncEnabledChanged).Times(1); EXPECT_CALL(*observer(), OnSyncStateChanged).Times(AtLeast(1)); @@ -748,13 +808,13 @@ TEST_F(BraveSyncServiceTest, StartSyncNonDeviceRecords) { TEST_F(BraveSyncServiceTest, OnSyncReadyNewToSync) { sync_prefs()->SetSyncRequested(false); - EXPECT_CALL(*observer(), OnSyncStateChanged); EXPECT_CALL(*sync_client(), SendGetBookmarksBaseOrder).Times(1); + brave_sync_prefs()->SetThisDeviceId("0"); sync_service()->OnSyncReady(); // simulate OnSaveBookmarksBaseOrder - profile()->GetPrefs()->SetString(brave_sync::prefs::kSyncBookmarksBaseOrder, - "1.1."); + brave_sync_prefs()->SetBookmarksBaseOrder("1.1."); + EXPECT_CALL(*observer(), OnSyncStateChanged); sync_service()->OnSyncReady(); EXPECT_TRUE(sync_prefs()->IsSyncRequested()); @@ -804,8 +864,10 @@ TEST_F(BraveSyncServiceTest, OnSetupSyncHaveCode_Reset_SetupAgain) { // Expecting sync state changed twice: for enabled state and for device name EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(2); sync_service()->OnSetupSyncHaveCode("word1 word2 word3", "test_device"); - EXPECT_TRUE( + EXPECT_FALSE( profile()->GetPrefs()->GetBoolean(brave_sync::prefs::kSyncEnabled)); + EXPECT_TRUE(brave_sync_prefs_mem()->GetSyncEnabled()); + EXPECT_FALSE(brave_sync_prefs_disk()->GetSyncEnabled()); brave_sync::Uint8Array seed = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}; @@ -831,8 +893,10 @@ TEST_F(BraveSyncServiceTest, OnSetupSyncHaveCode_Reset_SetupAgain) { // Here we have marked service as in resetting state // Actual kSyncEnabled will go to false after receiving confirmation of // this_device DELETE - EXPECT_TRUE( + EXPECT_FALSE( profile()->GetPrefs()->GetBoolean(brave_sync::prefs::kSyncEnabled)); + EXPECT_TRUE(brave_sync_prefs_mem()->GetSyncEnabled()); + EXPECT_FALSE(brave_sync_prefs_disk()->GetSyncEnabled()); EXPECT_TRUE(sync_service()->GetResettingForTest()); @@ -859,11 +923,21 @@ TEST_F(BraveSyncServiceTest, OnSetupSyncHaveCode_Reset_SetupAgain) { sync_service()->OnSetupSyncHaveCode("word1 word2 word5", "test_device"); EXPECT_FALSE(sync_service()->GetResettingForTest()); - EXPECT_TRUE( + EXPECT_FALSE( profile()->GetPrefs()->GetBoolean(brave_sync::prefs::kSyncEnabled)); + EXPECT_TRUE(brave_sync_prefs_mem()->GetSyncEnabled()); + EXPECT_FALSE(brave_sync_prefs_disk()->GetSyncEnabled()); } TEST_F(BraveSyncServiceTest, ExponentialResend) { + // We are expecting resend works when the chain contains two or more devices + // and disk prefs is used + brave_sync_prefs()->SetSeed("10, 20, 30"); + EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(1); + brave_sync_prefs()->SetThisDeviceName("device1"); + brave_sync_prefs()->SetThisDeviceId("1"); + sync_service()->SwitchToDiskPrefs(); + bookmarks::AddIfNotBookmarked(model(), GURL("https://a.com/"), base::ASCIIToUTF16("A.com")); // Explicitly set sync_timestamp, object_id and order because it is supposed @@ -878,7 +952,6 @@ TEST_F(BraveSyncServiceTest, ExponentialResend) { const char* record_a_order = "1.1.1.1"; model()->SetNodeMetaInfo(node, "order", record_a_order); - brave_sync_prefs()->SetThisDeviceId("1"); std::unique_ptr records = std::make_unique(); records->push_back(SimpleBookmarkSyncRecord( SyncRecord::Action::A_CREATE, record_a_object_id, "https://a.com/", @@ -959,6 +1032,10 @@ TEST_F(BraveSyncServiceTest, ExponentialResend) { TEST_F(BraveSyncServiceTest, GetDevicesWithFetchSyncRecords) { using brave_sync::jslib_const::kPreferences; + brave_sync_prefs()->SetSeed("10, 20, 30"); + EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())).Times(1); + brave_sync_prefs()->SetThisDeviceName("device1"); + // Expecting SendFetchSyncRecords will be invoked EXPECT_EQ(brave_sync_prefs()->GetLastFetchTime(), base::Time()); EXPECT_EQ(brave_sync_prefs()->GetLatestDeviceRecordTime(), base::Time()); @@ -1030,19 +1107,30 @@ TEST_F(ServiceStartDeviceLeftoverTest, OnSetupSyncHaveCodeDeviceLeftover) { EXPECT_TRUE(sync_service()->reseting_); EXPECT_EQ(brave_sync_prefs()->GetSyncDevices()->size(), 0u); - EXPECT_CALL(*sync_client(), OnSyncEnabledChanged).Times(AtLeast(2)); + EXPECT_CALL(*sync_client(), OnSyncEnabledChanged).Times(AtLeast(1)); EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service())) .Times(AtLeast(2)); sync_service()->OnSetupSyncHaveCode("word1 word2 word3", "test_device"); - EXPECT_TRUE( + EXPECT_FALSE( profile()->GetPrefs()->GetBoolean(brave_sync::prefs::kSyncEnabled)); + EXPECT_TRUE(brave_sync_prefs()->GetSyncEnabled()); EXPECT_FALSE(sync_service()->reseting_); } TEST_F(ServiceShotdownDeviceLeftoverTest, OnShutdownDeviceLeftover) { // Configure state of enabled sync and just only one device in the chain // before service shotdown - PreparePrefsStateDeviceLeftover(profile()->GetPrefs()); + + brave_sync::SyncDevices devices; + brave_sync::SyncDevice device("COMPUTER", + "67, 22, 139, 42, 47, 37, 218, 215," + "254, 228, 11, 42, 192, 73, 99, 47", + "0", 1.571261287102e+12); + devices.devices_.push_back(device); + + brave_sync_prefs()->SetSyncDevices(devices); + brave_sync_prefs()->SetSeed("10, 20, 30"); + // Will verify in ServiceShotdownDeviceLeftoverTest::PostShotdownService - // we did a proper cleanup + // we will not save prefs on disk, they remain in memory only } diff --git a/components/brave_sync/test_util.h b/components/brave_sync/test_util.h index 635cc514c18c..d8ec71a23ea3 100644 --- a/components/brave_sync/test_util.h +++ b/components/brave_sync/test_util.h @@ -52,7 +52,7 @@ class MockBraveSyncClient : public BraveSyncClient { MOCK_METHOD2(SendGetBookmarksBaseOrder, void(const std::string& device_id, const std::string& platform)); MOCK_METHOD0(OnExtensionInitialized, void()); - MOCK_METHOD0(OnSyncEnabledChanged, void()); + MOCK_METHOD1(OnSyncEnabledChanged, void(bool is_enabled)); MOCK_METHOD0(ClearOrderMap, void()); };