New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 700992 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Bookmarks association fails when siblings have the same title

Project Member Reported by pnoland@chromium.org, Mar 13 2017

Issue description

From CL description: 
When associating bookmarks, sync nodes are matched to BookmarkNodes.
To match a BookmarkNode with a sync node, a BookmarkNodeFinder is
created and asked to find the child node with the matching title. When searching
for the title, BookmarkNodeFinder returns the first matching child in
its child_nodes_ map. This map is an unordered_map with undefined
iteration order. If sync node siblings share a title, the
BookmarkNodeFinder can erroneously swap their respective
BookmarkNodes.
To fix this, this CL makes the iteration order defined with
std::multimap, which stores pairs in insertion order.
This is safe since sync nodes are processed in the same
order as BookmarkNodes are stored.


 
Labels: Merge-Request-58
Labels: -Pri-3 Pri-2
Cr-Commit-Position: refs/heads/master@{#456234}
Committed: https://chromium.googlesource.com/chromium/src/+/e4a5c1d83e0288ebe94f1d7bae7a76d8c01efffe
Project Member

Comment 3 by sheriffbot@chromium.org, Mar 14 2017

Labels: -Merge-Request-58 Hotlist-Merge-Approved Merge-Approved-58
Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), bhthompson@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Your change is approved for M58. Please ensure whether this fix is verified in canary. If yes, please merge ASAP so that it will be picked up for Beta promotion RC cut on 03/15 at 5.00 PM PS

Comment 5 by pav...@chromium.org, Mar 14 2017

Labels: -Merge-Approved-58 merge-merged-3029
The change was merged to M58 (3029). 
https://codereview.chromium.org/2752523003

https://chromium.googlesource.com/chromium/src/+/b3eca2062fda3945fcf759a70f9eb6c10c775d82
Status: Fixed (was: Started)

Sign in to add a comment