Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(6968)

Unified Diff: chrome/browser/sync/glue/bookmark_change_processor.cc

Issue 896073002: Prevent Meta Info from being overwritten due to reordering of entries. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/sync/glue/bookmark_change_processor.cc
diff --git a/chrome/browser/sync/glue/bookmark_change_processor.cc b/chrome/browser/sync/glue/bookmark_change_processor.cc
index ecd8ce686bd544ba1d3eed5fa4ce6750bc93a506..6787a0f669ed6ca67997bb08fd74f45d816c4201 100644
--- a/chrome/browser/sync/glue/bookmark_change_processor.cc
+++ b/chrome/browser/sync/glue/bookmark_change_processor.cc
@@ -844,6 +844,8 @@ BookmarkChangeProcessor::GetBookmarkMetaInfo(
(*meta_info_map)[specifics.meta_info(i).key()] =
specifics.meta_info(i).value();
}
+ // Verifies that all entries had unique keys.
+ DCHECK(meta_info_map->size() == (size_t)specifics.meta_info_size());
Nicolas Zea 2015/02/04 22:46:21 nit: style guide says to use static_cast (here and
stanisc 2015/02/05 01:40:15 Done.
return meta_info_map.Pass();
}
@@ -852,8 +854,34 @@ void BookmarkChangeProcessor::SetSyncNodeMetaInfo(
const BookmarkNode* node,
syncer::WriteNode* sync_node) {
sync_pb::BookmarkSpecifics specifics = sync_node->GetBookmarkSpecifics();
- specifics.clear_meta_info();
const BookmarkNode::MetaInfoMap* meta_info_map = node->GetMetaInfoMap();
+
+ // Compare specifics meta info to node meta info before making the change.
+ // Please note that the original specifics meta info is unordered while
+ // meta_info_map is ordered by key. Setting the meta info blindly into
+ // the specifics might cause an unnecessary change.
+ size_t size = meta_info_map ? meta_info_map->size() : 0;
+ if ((size_t)specifics.meta_info_size() == size) {
+ size_t index = 0;
+ for (; index < size; index++) {
+ const sync_pb::MetaInfo& meta_info = specifics.meta_info(index);
+ BookmarkNode::MetaInfoMap::const_iterator it =
+ meta_info_map->find(meta_info.key());
+ if (it == meta_info_map->end() || it->second != meta_info.value()) {
+ // One of original meta info entries is missing in |meta_info_map| or
+ // different.
+ break;
+ }
+ }
+ if (index == size) {
+ // The original meta info from the sync model is already equivalent to
+ // |meta_info_map|.
+ return;
+ }
+ }
+
+ // Clear and reset meta info in bookmark specifics.
+ specifics.clear_meta_info();
if (meta_info_map) {
for (BookmarkNode::MetaInfoMap::const_iterator it = meta_info_map->begin();
it != meta_info_map->end(); ++it) {
@@ -862,6 +890,7 @@ void BookmarkChangeProcessor::SetSyncNodeMetaInfo(
meta_info->set_value(it->second);
}
}
+
sync_node->SetBookmarkSpecifics(specifics);
}

Powered by Google App Engine
This is Rietveld 408576698