From 87648e2f919341fd2aca4f3ec6cb03a6a07e6024 Mon Sep 17 00:00:00 2001 From: AlexeyBarabash Date: Thu, 27 Feb 2020 14:19:53 -0700 Subject: [PATCH] Merge pull request #4710 from brave/sync_fix_obj_id_dup Sync fix bookmarks object id duplication --- .../brave_bookmark_model_loaded_observer.cc | 3 ++ .../brave_profile_sync_service_impl.cc | 11 +++++++ .../brave_profile_sync_service_impl.h | 2 ++ .../brave_sync/brave_sync_service_unittest.cc | 31 +++++++++++++++++++ 4 files changed, 47 insertions(+) diff --git a/browser/profiles/brave_bookmark_model_loaded_observer.cc b/browser/profiles/brave_bookmark_model_loaded_observer.cc index 59e4f191809a..f76f5ca20e83 100644 --- a/browser/profiles/brave_bookmark_model_loaded_observer.cc +++ b/browser/profiles/brave_bookmark_model_loaded_observer.cc @@ -31,8 +31,11 @@ void BraveBookmarkModelLoadedObserver::BookmarkModelLoaded( // it is handled in BraveProfileSyncServiceImpl::OnSyncReady if (brave_profile_service && !brave_profile_service->IsBraveSyncEnabled()) BraveMigrateOtherNode(model); + + BraveProfileSyncServiceImpl::AddNonClonedBookmarkKeys(model); #else BraveMigrateOtherNode(model); #endif + BookmarkModelLoadedObserver::BookmarkModelLoaded(model, ids_reassigned); } diff --git a/components/brave_sync/brave_profile_sync_service_impl.cc b/components/brave_sync/brave_profile_sync_service_impl.cc index 5b24a7df4cdb..b61394e0e588 100644 --- a/components/brave_sync/brave_profile_sync_service_impl.cc +++ b/components/brave_sync/brave_profile_sync_service_impl.cc @@ -516,6 +516,17 @@ void BraveProfileSyncServiceImpl::OnSyncReadyBookmarksModelLoaded() { BraveMigrateOtherNode(model_); } +// static +void BraveProfileSyncServiceImpl::AddNonClonedBookmarkKeys( + BookmarkModel* model) { + DCHECK(model); + DCHECK(model->loaded()); + model->AddNonClonedKey("object_id"); + model->AddNonClonedKey("order"); + model->AddNonClonedKey("sync_timestamp"); + model->AddNonClonedKey("version"); +} + syncer::ModelTypeSet BraveProfileSyncServiceImpl::GetPreferredDataTypes() const { // Force DEVICE_INFO type to have nudge cycle each time to fetch diff --git a/components/brave_sync/brave_profile_sync_service_impl.h b/components/brave_sync/brave_profile_sync_service_impl.h index 2582116779f7..01a4b228c684 100644 --- a/components/brave_sync/brave_profile_sync_service_impl.h +++ b/components/brave_sync/brave_profile_sync_service_impl.h @@ -170,6 +170,8 @@ class BraveProfileSyncServiceImpl BraveSyncClient* GetBraveSyncClient() override; #endif + static void AddNonClonedBookmarkKeys(BookmarkModel* model); + bool IsBraveSyncEnabled() const override; syncer::ModelTypeSet GetPreferredDataTypes() const override; diff --git a/components/brave_sync/brave_sync_service_unittest.cc b/components/brave_sync/brave_sync_service_unittest.cc index 92bf3431d621..2bc6f2621815 100644 --- a/components/brave_sync/brave_sync_service_unittest.cc +++ b/components/brave_sync/brave_sync_service_unittest.cc @@ -1310,3 +1310,34 @@ TEST_F(BraveSyncServiceTest, DeviceIdV2MigrationDupDeviceId) { base::BindOnce(&OnGetRecordsStub); sync_service()->OnPollSyncCycle(std::move(on_get_records), &we); } + +TEST_F(BraveSyncServiceTest, AddNonClonedBookmarkKeys) { + sync_service()->AddNonClonedBookmarkKeys(model()); + const bookmarks::BookmarkNode* bookmark_a1 = + model()->AddURL(model()->other_node(), 0, base::ASCIIToUTF16("A1"), + GURL("https://a1.com")); + + AsMutable(bookmark_a1)->SetMetaInfo("object_id", "object_id_value"); + AsMutable(bookmark_a1)->SetMetaInfo("order", "order_value"); + AsMutable(bookmark_a1)->SetMetaInfo("sync_timestamp", "sync_timestamp_value"); + AsMutable(bookmark_a1)->SetMetaInfo("version", "version_value"); + + model()->Copy(bookmark_a1, model()->other_node(), 1); + + const bookmarks::BookmarkNode* bookmark_copy = + model()->other_node()->children().at(1).get(); + + std::string meta_object_id; + EXPECT_FALSE(bookmark_copy->GetMetaInfo("object_id", &meta_object_id)); + EXPECT_TRUE(meta_object_id.empty()); + std::string meta_order; + EXPECT_FALSE(bookmark_copy->GetMetaInfo("order", &meta_order)); + EXPECT_TRUE(meta_order.empty()); + std::string meta_sync_timestamp; + EXPECT_FALSE( + bookmark_copy->GetMetaInfo("sync_timestamp", &meta_sync_timestamp)); + EXPECT_TRUE(meta_sync_timestamp.empty()); + std::string meta_version; + EXPECT_FALSE(bookmark_copy->GetMetaInfo("version", &meta_version)); + EXPECT_TRUE(meta_version.empty()); +}