Revert of Use external ID to match native model nodes during bookmark association...
commitf44a826b177af9fc9fc3c81590b0cc15267cbf76
authorrch <rch@chromium.org>
Tue, 10 Feb 2015 00:07:17 +0000 (9 16:07 -0800)
committerCommit bot <commit-bot@chromium.org>
Tue, 10 Feb 2015 00:07:56 +0000 (10 00:07 +0000)
treee8f59a4af0c2eb0dc29394ea03e306f23b1d74d5
parent3a75e7a319c93e3375d3a1f18040f86f9eed9296
Revert of Use external ID to match native model nodes during bookmark association. (patchset #2 id:40001 of https://codereview.chromium.org/904083002/)

Reason for revert:
Looks like it broke a sync_unittest

http://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/35199

(But maybe it's a flake?)

Original issue's description:
> Use external ID to match native model nodes during bookmark association.
>
> The fix improves matching of nodes in BookmarkModelAssociator in
> situations where there are multiple bookmarks or folders with the
> same titles or URLs.
> This will address one particular scenario leading to bookmark
> duplication (see crbug.com/118105).
>
> 1) In BookmarkModelAssociator::BuildAssociations, when there are
> multiple native model nodes with matching title / URL, a secondary
> match on external ID is used to pick a preferred one; otherwise
> the first matching node is returned.
> The preferred match on external ID should be applicable in most
> situations except when the native model has been rebuilt from scratch.
> Picking a wrong folder during the association process results in
> duplicating the entire subtree within the wrong folder. This issue
> should be addressed now.
> 2) In BookmarkModelAssociator::ApplyDeletesFromSyncJournal the external
> ID match is now the primary criteria for selecting a native model
> node to be deleted. The previous implementation would pick an arbitrary
> native model node based on just the title / URL match anywhere in the
> node hierarchy. That would happen every time after deleting a bookmark
> or folder and recreating it in another place.
> Since external IDs might be reused, there is a secondary match on
> title and URL to ensure that the right node gets deleted. To avoid
> costly O(N*M) algorithm (where N is number of bookmarks and M is
> number of entries in delete journal), the implementation uses a set
> of external IDs to reduce the cost to O(N*logM).
>
> BUG=456228
>
> Committed: https://crrev.com/89fce43a19e7f8b0225a0d750ec3eb3cb0939e3d
> Cr-Commit-Position: refs/heads/master@{#315401}

TBR=zea@chromium.org,stanisc@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=456228

Review URL: https://codereview.chromium.org/906403003

Cr-Commit-Position: refs/heads/master@{#315444}
chrome/browser/sync/glue/bookmark_model_associator.cc
chrome/browser/sync/glue/bookmark_model_associator.h
chrome/browser/sync/profile_sync_service_bookmark_unittest.cc
sync/internal_api/delete_journal.cc
sync/internal_api/public/delete_journal.h