Prevent Meta Info from being overwritten due to reordering of entries.

chrome/browser/sync/glue/bookmark_change_processor.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/sync/glue/bookmark_change_processor.h"
 
 #include <map>
 #include <stack>
 #include <vector>
 
@@ skipping 826 matching lines @@
 BookmarkChangeProcessor::GetBookmarkMetaInfo(
     const syncer::BaseNode* sync_node) {
   const sync_pb::BookmarkSpecifics& specifics =
       sync_node->GetBookmarkSpecifics();
   scoped_ptr<BookmarkNode::MetaInfoMap> meta_info_map(
       new BookmarkNode::MetaInfoMap);
   for (int i = 0; i < specifics.meta_info_size(); ++i) {
     (*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 below)

[stanisc, 2015/02/05 01:40:15]

Done.

   return meta_info_map.Pass();
 }
 
 // static
 void BookmarkChangeProcessor::SetSyncNodeMetaInfo(
     const BookmarkNode* node,
     syncer::WriteNode* sync_node) {
   sync_pb::BookmarkSpecifics specifics = sync_node->GetBookmarkSpecifics();
+  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();
-  const BookmarkNode::MetaInfoMap* meta_info_map = node->GetMetaInfoMap();
   if (meta_info_map) {
     for (BookmarkNode::MetaInfoMap::const_iterator it = meta_info_map->begin();
         it != meta_info_map->end(); ++it) {
       sync_pb::MetaInfo* meta_info = specifics.add_meta_info();
       meta_info->set_key(it->first);
       meta_info->set_value(it->second);
     }
   }
+
   sync_node->SetBookmarkSpecifics(specifics);
 }
 
 // static
 void BookmarkChangeProcessor::ApplyBookmarkFavicon(
     const BookmarkNode* bookmark_node,
     Profile* profile,
     const GURL& icon_url,
     const scoped_refptr<base::RefCountedMemory>& bitmap_data) {
   HistoryService* history = HistoryServiceFactory::GetForProfile(
@@ skipping 30 matching lines @@
     updated_specifics.set_icon_url(bookmark_node->icon_url().spec());
     sync_node->SetBookmarkSpecifics(updated_specifics);
   }
 }
 
 bool BookmarkChangeProcessor::CanSyncNode(const BookmarkNode* node) {
   return bookmark_model_->client()->CanSyncNode(node);
 }
 
 }  // namespace browser_sync