Skip to content

Commit

Permalink
Stop caching "originator_cache_guid" and "originator_client_item_id" to
Browse files Browse the repository at this point in the history
prevent generating duplicate tracking entries.
The cached ids are not accurate when receiving remote CREATE and then
committing UPDATE because we never get to propagate the real value to
other devices
  • Loading branch information
darkdh committed Feb 12, 2020
1 parent f2dde80 commit 908ed10
Show file tree
Hide file tree
Showing 5 changed files with 2 additions and 54 deletions.
20 changes: 0 additions & 20 deletions chromium_src/components/sync/engine_impl/commit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,6 @@ brave_sync::RecordsListPtr ConvertCommitsToBraveRecords(
// only mattters for direct children of permanent nodes
bookmark->hideInToolbar = entity.parent_id_string() != kBookmarkBarTag;

std::string originator_cache_guid;
std::string originator_client_item_id;
bool skip_record = false;
for (int i = 0; i < bm_specifics.meta_info_size(); ++i) {
if (bm_specifics.meta_info(i).key() == "order") {
Expand All @@ -95,11 +93,6 @@ brave_sync::RecordsListPtr ConvertCommitsToBraveRecords(
} else if (bm_specifics.meta_info(i).key() == "sync_timestamp") {
record->syncTimestamp = base::Time::FromJsTime(
std::stod(bm_specifics.meta_info(i).value()));
} else if (bm_specifics.meta_info(i).key() == "originator_cache_guid") {
originator_cache_guid = bm_specifics.meta_info(i).value();
} else if (bm_specifics.meta_info(i).key() ==
"originator_client_item_id") {
originator_client_item_id = bm_specifics.meta_info(i).value();
} else if (bm_specifics.meta_info(i).key() == "last_send_time" &&
entity.version() == 0) {
// Upgrade from legacy code, we need to prevent sending duplicate
Expand All @@ -123,19 +116,6 @@ brave_sync::RecordsListPtr ConvertCommitsToBraveRecords(
DCHECK(!record->objectId.empty());

MetaInfo metaInfo;
metaInfo.key = "originator_cache_guid";
if (originator_cache_guid.empty()) {
originator_cache_guid = cache_guid;
}
metaInfo.value = originator_cache_guid;
bookmark->metaInfo.push_back(metaInfo);

metaInfo.key = "originator_client_item_id";
if (originator_client_item_id.empty()) {
originator_client_item_id = entity.id_string();
}
metaInfo.value = originator_client_item_id;
bookmark->metaInfo.push_back(metaInfo);

metaInfo.key = "version";
metaInfo.value = std::to_string(version);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,7 @@ void ExtractBookmarkMeta(sync_pb::SyncEntity* entity,
meta_info->set_key(metaInfo.key);
meta_info->set_value(metaInfo.value);
}
if (metaInfo.key == "originator_cache_guid") {
entity->set_originator_cache_guid(metaInfo.value);
} else if (metaInfo.key == "originator_client_item_id") {
entity->set_originator_client_item_id(metaInfo.value);
} else if (metaInfo.key == "version") {
if (metaInfo.key == "version") {
int64_t version;
bool result = base::StringToInt64(metaInfo.value, &version);
DCHECK(result);
Expand Down
4 changes: 0 additions & 4 deletions components/brave_sync/brave_profile_sync_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -738,8 +738,6 @@ BraveProfileSyncServiceImpl::BookmarkNodeToSyncBookmark(

record->action = jslib::SyncRecord::Action::A_UPDATE;

AddSyncEntityInfo(bookmark.get(), node, "originator_cache_guid");
AddSyncEntityInfo(bookmark.get(), node, "originator_client_item_id");
AddSyncEntityInfo(bookmark.get(), node, "version");
AddSyncEntityInfo(bookmark.get(), node, "position_in_parent");

Expand Down Expand Up @@ -775,8 +773,6 @@ void BraveProfileSyncServiceImpl::LoadSyncEntityInfo(
return;
auto* node = FindByObjectId(model_, record->objectId);
if (node) {
AddSyncEntityInfo(bookmark, node, "originator_cache_guid");
AddSyncEntityInfo(bookmark, node, "originator_client_item_id");
AddSyncEntityInfo(bookmark, node, "position_in_parent");
AddSyncEntityInfo(bookmark, node, "version");
} else { // Assign base version metainfo for remotely created record
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
diff --git a/components/sync_bookmarks/bookmark_remote_updates_handler.cc b/components/sync_bookmarks/bookmark_remote_updates_handler.cc
index c31275713056a2a1dd07872bc3569d54a8cfce0e..8128b7ceb92166d149056c15d9a9f7dc23fb5514 100644
index c31275713056a2a1dd07872bc3569d54a8cfce0e..700877cd135845fe686626e821870ff0bbc3f453 100644
--- a/components/sync_bookmarks/bookmark_remote_updates_handler.cc
+++ b/components/sync_bookmarks/bookmark_remote_updates_handler.cc
@@ -144,6 +144,7 @@ void ApplyRemoteUpdate(
Expand All @@ -10,17 +10,6 @@ index c31275713056a2a1dd07872bc3569d54a8cfce0e..8128b7ceb92166d149056c15d9a9f7dc
ComputeChildNodeIndex(new_parent, update_entity.unique_position, tracker);
tracker->Update(update_entity.id, update.response_version,
update_entity.modification_time,
@@ -241,8 +242,8 @@ void BookmarkRemoteUpdatesHandler::Process(
const bookmarks::BookmarkNode* old_node = old_entity->bookmark_node();
const bookmarks::BookmarkNode* new_node =
tracked_entity->bookmark_node();
- CHECK(old_node->type() == bookmarks::BookmarkNode::URL);
- CHECK(new_node->type() == bookmarks::BookmarkNode::URL);
+ // CHECK(old_node->type() == bookmarks::BookmarkNode::URL);
+ // CHECK(new_node->type() == bookmarks::BookmarkNode::URL);
CHECK(old_node->url() == new_node->url());
bookmark_tracker_->Remove(update_entity.originator_client_item_id);
bookmark_model_->Remove(old_node);
@@ -449,9 +450,11 @@ bool BookmarkRemoteUpdatesHandler::ProcessCreate(
LogProblematicBookmark(RemoteBookmarkUpdateError::kMissingParentNode);
return false;
Expand Down
13 changes: 0 additions & 13 deletions patches/components-sync_bookmarks-synced_bookmark_tracker.cc.patch

This file was deleted.

0 comments on commit 908ed10

Please sign in to comment.