[Sync] Act defensively when parent lookups fail on commit

Desktop / Chromium - skym [chromium.org] - 17 April 2017 17:54 EDT

When committing a bookmark (a hierarchical type), we follow parent links going upwards making sure that we also include all parents that are so far client side only. We will ultimately reverse this list so that we ensure parents always reach the server before children.

What happened in the crash (see bug) this CL is trying to fix is that one of the parent look ups failed. Someone's parent id was a client id, and the directory didn't have a copy of it. In a simple world, this wouldn't happen, the bookmarks model should have sent the parent bookmark along (or even before) the child bookmark.

Our working assumption is there's some race somewhere involving potentially multiple clients doing weird things like renaming, moving, and deleting folders, and re-associating. Somehow a constraint is broken, and on commit we have a child without a parent. Investigation into the root cause of this did not turn up anything easily, so instead we're changing the code to simply be more defensive and not crash in this circumstance. It would not be safe to send a child without a parent to the server, so our reaction is to skip committing the orphaned bookmarks. There are already several other cases that take the approach of not committing hierarchical things that are in a somewhat bad state, so this approach is somewhat consistent.

The long term goal is to have all Bookmarks (the only hierarchical type) logic converted over to a USS implementation, and all of this code will be obsolete in the not terribly distant future anyways.

BUG=711381

Review-Url: https://codereview.chromium.org/2826503002 Cr-Commit-Position: refs/heads/master@{#465033}

85ee7d9 [Sync] Act defensively when parent lookups fail on commit.
.../directory_commit_contribution_unittest.cc | 29 ++++++++++++++++++++
components/sync/engine_impl/get_commit_ids.cc | 32 ++++++++++++++++------
2 files changed, 53 insertions(+), 8 deletions(-)

Upstream: git.chromium.org


  • Share