[Sync] Act defensively when parent lookups fail on commit.

components/sync/engine_impl/get_commit_ids.cc

Index: components/sync/engine_impl/get_commit_ids.cc
diff --git a/components/sync/engine_impl/get_commit_ids.cc b/components/sync/engine_impl/get_commit_ids.cc
index a8dc995bd4cc0ffb8eedd6ebd10d26d89d89d458..4b6573c50c53dcc70adf31031934c1f6a6f5da45 100644
--- a/components/sync/engine_impl/get_commit_ids.cc
+++ b/components/sync/engine_impl/get_commit_ids.cc
@@ -162,9 +162,9 @@ class Traversal {
  private:
   // The following functions do not modify the traversal directly. They return
   // their results in the |result| vector instead.
-  void AddUncommittedParents(const std::set<int64_t>& ready_unsynced_set,
-                             const Entry& item,
-                             Directory::Metahandles* result) const;
+  bool TryAddUncommittedParents(const std::set<int64_t>& ready_unsynced_set,
+                                const Entry& item,
+                                Directory::Metahandles* result) const;
 
   bool TryAddItem(const std::set<int64_t>& ready_unsynced_set,
                   const Entry& item,
@@ -202,7 +202,7 @@ Traversal::Traversal(syncable::BaseTransaction* trans,
 
 Traversal::~Traversal() {}
 
-void Traversal::AddUncommittedParents(
+bool Traversal::TryAddUncommittedParents(
     const std::set<int64_t>& ready_unsynced_set,
     const Entry& item,
     Directory::Metahandles* result) const {
@@ -213,7 +213,16 @@ void Traversal::AddUncommittedParents(
   // Climb the tree adding entries leaf -> root.
   while (!parent_id.ServerKnows()) {
     Entry parent(trans_, syncable::GET_BY_ID, parent_id);
-    CHECK(parent.good()) << "Bad user-only parent in item path.";
+
+    // This apparently does happen, see crbug.com/711381. Someone is violating
+    // some constraint and some ancestor isn't current present in the directory
+    // while the child is. Because we do not know where this comes from, be
+    // defensive and skip this inclusion instead.
+    if (!parent.good()) {
+      DVLOG(1) << "Bad user-only parent in item path with id " << parent_id;
+      return false;
+    }
+
     int64_t handle = parent.GetMetahandle();
     if (HaveItem(handle)) {
       // We've already added this parent (and therefore all of its parents).
@@ -231,6 +240,7 @@ void Traversal::AddUncommittedParents(
 
   // Reverse what we added to get the correct order.
   result->insert(result->end(), dependencies.rbegin(), dependencies.rend());
+  return true;
 }
 
 // Adds the given item to the list if it is unsynced and ready for commit.
@@ -337,7 +347,14 @@ void Traversal::AddCreatesAndMoves(
         // We only commit an item + its dependencies if it and all its
         // dependencies are not in conflict.
         Directory::Metahandles item_dependencies;
-        AddUncommittedParents(ready_unsynced_set, entry, &item_dependencies);
+
+        // If we fail to add a required parent, give up on this entry.
+        if (!TryAddUncommittedParents(ready_unsynced_set, entry,
+                                      &item_dependencies)) {
+          continue;
+        }
+
+        // Okay if this fails, the parents were still valid.
         TryAddItem(ready_unsynced_set, entry, &item_dependencies);
         AppendManyToTraversal(item_dependencies);
       } else {
@@ -398,8 +415,7 @@ void Traversal::AddDeletes(const std::set<int64_t>& ready_unsynced_set) {
     out_->resize(max_entries_);
 }
 
-// Excludes ancestors of deleted conflicted items from
-// |ready_unsynced_set|.
+// Excludes ancestors of deleted conflicted items from |ready_unsynced_set|.
 void ExcludeDeletedAncestors(
     syncable::BaseTransaction* trans,
     const std::vector<int64_t>& deleted_conflicted_items,