Merge pull request #4710 from brave/sync_fix_obj_id_dup

Sync fix bookmarks object id duplication
brave · Feb 27, 2020 · 84a0f9e · 84a0f9e
1 parent b19c5b1
commit 84a0f9e
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 0 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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());
+}