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

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

This fixes a client side issue that contributes to crbug.com/439110.

The code that updates Sync Data with meta info from bookmark model is changed
to verify whether the new meta info is equivalent to the original one to avoid
doing an unnecessary mutation due to reordering of entries. The reordering occurs
due to sorting by key when MetaInfo is retrieved from BookmarkSpecifics in the
first place and added to MetaInfoMap.

BUG=455291
Committed: https://crrev.com/a2ef17dfc09c595f16c5a097fd1a483acb66a481
Cr-Commit-Position: refs/heads/master@{#314840}

[stanisc, 2015-02-04 18:35:32]

stanisc@chromium.org changed reviewers:
+ zea@chromium.org

[stanisc, 2015-02-04 18:35:32]

PTAL.
I created a separate crbug for this issue rather than using 439110 to
avoid giving too much visibility to a partial client side fix.
Once it is fixed I'll merge it into the main bug.

[Nicolas Zea, 2015-02-04 22:46:20]

zea@chromium.org changed reviewers:
+ danduong@chromium.org

[Nicolas Zea, 2015-02-04 22:46:21]

+Dan FYI

LGTM with nits

https://codereview.chromium.org/896073002/diff/1/chrome/browser/sync/glue/boo...
File chrome/browser/sync/glue/bookmark_change_processor.cc (right):

https://codereview.chromium.org/896073002/diff/1/chrome/browser/sync/glue/boo...
chrome/browser/sync/glue/bookmark_change_processor.cc:848:
DCHECK(meta_info_map->size() == (size_t)specifics.meta_info_size());
nit: style guide says to use static_cast (here and below)

https://codereview.chromium.org/896073002/diff/1/chrome/browser/sync/profile_...
File chrome/browser/sync/profile_sync_service_bookmark_unittest.cc (right):

https://codereview.chromium.org/896073002/diff/1/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service_bookmark_unittest.cc:267:
BookmarkNode::MetaInfoMap::const_iterator it = meta_info_map.end();
nit: leave as for loop?

https://codereview.chromium.org/896073002/diff/1/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service_bookmark_unittest.cc:2032: // Tests
that
Finish comment?

[stanisc, 2015-02-05 01:36:52]

New patchsets have been uploaded after l-g-t-m from zea@chromium.org

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

https://codereview.chromium.org/896073002/diff/1/chrome/browser/sync/glue/boo...
File chrome/browser/sync/glue/bookmark_change_processor.cc (right):

https://codereview.chromium.org/896073002/diff/1/chrome/browser/sync/glue/boo...
chrome/browser/sync/glue/bookmark_change_processor.cc:848:
DCHECK(meta_info_map->size() == (size_t)specifics.meta_info_size());
On 2015/02/04 22:46:21, Nicolas Zea wrote:
> nit: style guide says to use static_cast (here and below)

Done.

https://codereview.chromium.org/896073002/diff/1/chrome/browser/sync/profile_...
File chrome/browser/sync/profile_sync_service_bookmark_unittest.cc (right):

https://codereview.chromium.org/896073002/diff/1/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service_bookmark_unittest.cc:267:
BookmarkNode::MetaInfoMap::const_iterator it = meta_info_map.end();
On 2015/02/04 22:46:21, Nicolas Zea wrote:
> nit: leave as for loop?

I have considered the for loop, but couldn't find an elegant way to code it. For
loop doesn't work well in this case because the end isn't included (which means
it needs to be decremented first) but the start has to be included (which means
the body has to be executed one more time after detecting that it ==
meta_info_map.begin().

https://codereview.chromium.org/896073002/diff/1/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service_bookmark_unittest.cc:2032: // Tests
that
On 2015/02/04 22:46:21, Nicolas Zea wrote:
> Finish comment?

Done.

[stanisc, 2015-02-05 18:26:58]

The CQ bit was checked by stanisc@chromium.org

[commit-bot: I haz the power, 2015-02-05 18:27:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/896073002/20001

[commit-bot: I haz the power, 2015-02-05 18:31:01]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-05 18:31:55]

Patchset 2 (id:??) landed as
https://crrev.com/a2ef17dfc09c595f16c5a097fd1a483acb66a481
Cr-Commit-Position: refs/heads/master@{#314840}