Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 3628 offline reset #4089

Merged
merged 2 commits into from
Nov 28, 2019
Merged

Fix 3628 offline reset #4089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e57ff14
Reset sync when offline: call ResetSyncInternal from OnRecordsSent
AlexeyBarabash Nov 27, 2019
1e0c338
Testcase for resetting sync when offline
AlexeyBarabash Nov 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions components/brave_sync/brave_profile_sync_service_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,12 @@ void BraveProfileSyncServiceImpl::OnDeleteDevice(const std::string& device_id) {
const std::string object_id = device->object_id_;
SendDeviceSyncRecord(SyncRecord::Action::A_DELETE, device_name, device_id,
object_id);
if (device_id == brave_sync_prefs_->GetThisDeviceId()) {
// Mark state we have sent DELETE for own device and we are going to
// call ResetSyncInternal() at OnRecordsSent after ensuring we had made
// a proper attemp to send the record
pending_self_reset_ = true;
}
FetchDevices();
}
}
Expand Down Expand Up @@ -578,11 +584,14 @@ void BraveProfileSyncServiceImpl::OnCompactComplete(
void BraveProfileSyncServiceImpl::OnRecordsSent(
const std::string& category,
std::unique_ptr<brave_sync::RecordsList> records) {
for (auto& record : *records) {
if (category == kBookmarks) {
if (category == kBookmarks) {
for (auto& record : *records) {
// Remove Acked sent records
brave_sync_prefs_->RemoveFromRecordsToResend(record->objectId);
}
} else if (category == kPreferences && pending_self_reset_) {
ResetSyncInternal();
pending_self_reset_ = false;
}
}

Expand Down
4 changes: 4 additions & 0 deletions components/brave_sync/brave_profile_sync_service_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnDeleteDevice);
FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnDeleteDeviceWhenOneDevice);
FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnDeleteDeviceWhenSelfDeleted);
FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnResetSync);
FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnResetSyncWhenOffline);
FORWARD_DECLARE_TEST(BraveSyncServiceTest, ClientOnGetInitData);
FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnGetInitData);
FORWARD_DECLARE_TEST(BraveSyncServiceTest, OnSaveBookmarksBaseOrder);
Expand Down Expand Up @@ -140,6 +141,7 @@ class BraveProfileSyncServiceImpl
FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest,
OnDeleteDeviceWhenSelfDeleted);
FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, OnResetSync);
FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, OnResetSyncWhenOffline);
FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, ClientOnGetInitData);
FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, OnSaveBookmarksBaseOrder);
FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, OnGetInitData);
Expand Down Expand Up @@ -237,6 +239,8 @@ class BraveProfileSyncServiceImpl
// Time when current device sent CREATE device record
base::Time this_device_created_time_;

bool pending_self_reset_ = false;

// Used to ensure that certain operations are performed on the sequence that
// this object was created on.
SEQUENCE_CHECKER(sequence_checker_);
Expand Down
133 changes: 93 additions & 40 deletions components/brave_sync/brave_sync_service_unittest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,9 @@ class BraveSyncServiceTest : public testing::Test {
}
syncer::SyncPrefs* sync_prefs() { return sync_prefs_.get(); }

// Common part of OnResetSync and OnResetSyncWhenOffline tests
void VerifyResetDone();

private:
// Need this as a very first member to run tests in UI thread
// When this is set, class should not install any other MessageLoops, like
Expand Down Expand Up @@ -530,7 +533,54 @@ TEST_F(BraveSyncServiceTest, OnDeleteDeviceWhenSelfDeleted) {
EXPECT_FALSE(DevicesContains(devices_final.get(), "2", "device2"));
}

void BraveSyncServiceTest::VerifyResetDone() {
EXPECT_TRUE(profile()
->GetPrefs()
->GetString(brave_sync::prefs::kSyncDeviceId)
.empty());
EXPECT_TRUE(
profile()->GetPrefs()->GetString(brave_sync::prefs::kSyncSeed).empty());
EXPECT_TRUE(profile()
->GetPrefs()
->GetString(brave_sync::prefs::kSyncDeviceName)
.empty());
EXPECT_FALSE(
profile()->GetPrefs()->GetBoolean(brave_sync::prefs::kSyncEnabled));
EXPECT_FALSE(profile()->GetPrefs()->GetBoolean(
brave_sync::prefs::kSyncBookmarksEnabled));
EXPECT_TRUE(profile()
->GetPrefs()
->GetString(brave_sync::prefs::kSyncBookmarksBaseOrder)
.empty());
EXPECT_FALSE(profile()->GetPrefs()->GetBoolean(
brave_sync::prefs::kSyncSiteSettingsEnabled));
EXPECT_FALSE(profile()->GetPrefs()->GetBoolean(
brave_sync::prefs::kSyncHistoryEnabled));
EXPECT_TRUE(profile()
->GetPrefs()
->GetTime(brave_sync::prefs::kSyncLatestRecordTime)
.is_null());
EXPECT_TRUE(profile()
->GetPrefs()
->GetTime(brave_sync::prefs::kSyncLastFetchTime)
.is_null());
EXPECT_TRUE(profile()
->GetPrefs()
->GetString(brave_sync::prefs::kSyncDeviceList)
.empty());
EXPECT_EQ(
profile()->GetPrefs()->GetString(brave_sync::prefs::kSyncApiVersion),
"0");

EXPECT_FALSE(sync_service()->brave_sync_ready_);

EXPECT_FALSE(sync_prefs()->IsSyncRequested());
EXPECT_FALSE(
profile()->GetPrefs()->GetBoolean(syncer::prefs::kSyncBookmarks));
}

TEST_F(BraveSyncServiceTest, OnResetSync) {
using brave_sync::jslib_const::kPreferences;
EXPECT_CALL(*sync_client(), OnSyncEnabledChanged).Times(AtLeast(1));
EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service()))
.Times(AtLeast(3));
Expand All @@ -553,7 +603,7 @@ TEST_F(BraveSyncServiceTest, OnResetSync) {
EXPECT_TRUE(DevicesContains(devices.get(), "1", "device1"));

EXPECT_CALL(*sync_client(),
SendSyncRecords("PREFERENCES",
SendSyncRecords(kPreferences,
ContainsDeviceRecord(SyncRecord::Action::A_DELETE,
"this_device")))
.Times(1);
Expand All @@ -571,49 +621,52 @@ TEST_F(BraveSyncServiceTest, OnResetSync) {
EXPECT_FALSE(DevicesContains(devices_final.get(), "0", "this_device"));
EXPECT_FALSE(DevicesContains(devices_final.get(), "1", "device1"));

EXPECT_TRUE(profile()
->GetPrefs()
->GetString(brave_sync::prefs::kSyncDeviceId)
.empty());
VerifyResetDone();
}

TEST_F(BraveSyncServiceTest, OnResetSyncWhenOffline) {
using brave_sync::jslib_const::kPreferences;
EXPECT_CALL(*sync_client(), OnSyncEnabledChanged).Times(AtLeast(1));
EXPECT_CALL(*observer(), OnSyncStateChanged(sync_service()))
.Times(AtLeast(3));
sync_service()->OnSetupSyncNewToSync("this_device");
EXPECT_TRUE(
profile()->GetPrefs()->GetString(brave_sync::prefs::kSyncSeed).empty());
EXPECT_TRUE(profile()
->GetPrefs()
->GetString(brave_sync::prefs::kSyncDeviceName)
.empty());
EXPECT_FALSE(
profile()->GetPrefs()->GetBoolean(brave_sync::prefs::kSyncEnabled));
EXPECT_FALSE(profile()->GetPrefs()->GetBoolean(
brave_sync::prefs::kSyncBookmarksEnabled));
EXPECT_TRUE(profile()
->GetPrefs()
->GetString(brave_sync::prefs::kSyncBookmarksBaseOrder)
.empty());
EXPECT_FALSE(profile()->GetPrefs()->GetBoolean(
brave_sync::prefs::kSyncSiteSettingsEnabled));
EXPECT_FALSE(profile()->GetPrefs()->GetBoolean(
brave_sync::prefs::kSyncHistoryEnabled));
EXPECT_TRUE(profile()
->GetPrefs()
->GetTime(brave_sync::prefs::kSyncLatestRecordTime)
.is_null());
EXPECT_TRUE(profile()
->GetPrefs()
->GetTime(brave_sync::prefs::kSyncLastFetchTime)
.is_null());
EXPECT_TRUE(profile()
->GetPrefs()
->GetString(brave_sync::prefs::kSyncDeviceList)
.empty());
EXPECT_EQ(
profile()->GetPrefs()->GetString(brave_sync::prefs::kSyncApiVersion),
"0");
brave_sync_prefs()->SetThisDeviceId("0");

EXPECT_FALSE(sync_service()->brave_sync_ready_);
RecordsList records;
records.push_back(
SimpleDeviceRecord(SyncRecord::Action::A_CREATE, "0", "this_device"));
records.push_back(
SimpleDeviceRecord(SyncRecord::Action::A_CREATE, "1", "device1"));

EXPECT_FALSE(sync_prefs()->IsSyncRequested());
EXPECT_FALSE(
profile()->GetPrefs()->GetBoolean(syncer::prefs::kSyncBookmarks));
sync_service()->OnResolvedPreferences(records);

auto devices = brave_sync_prefs()->GetSyncDevices();

EXPECT_TRUE(DevicesContains(devices.get(), "0", "this_device"));
EXPECT_TRUE(DevicesContains(devices.get(), "1", "device1"));

EXPECT_CALL(*sync_client(),
SendSyncRecords(kPreferences,
ContainsDeviceRecord(SyncRecord::Action::A_DELETE,
"this_device")))
.Times(1);

EXPECT_CALL(*sync_client(), SendFetchSyncRecords);
EXPECT_FALSE(sync_service()->pending_self_reset_);
sync_service()->OnResetSync();
EXPECT_TRUE(sync_service()->pending_self_reset_);

auto recordsSent = std::make_unique<RecordsList>();
sync_service()->OnRecordsSent(kPreferences, std::move(recordsSent));
EXPECT_FALSE(sync_service()->pending_self_reset_);

auto devices_final = brave_sync_prefs()->GetSyncDevices();
EXPECT_FALSE(DevicesContains(devices_final.get(), "0", "this_device"));
EXPECT_FALSE(DevicesContains(devices_final.get(), "1", "device1"));

VerifyResetDone();
}

TEST_F(BraveSyncServiceTest, OnSetSyncBookmarks) {
Expand Down