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

components/sync/engine_impl/get_commit_ids.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/sync/engine_impl/get_commit_ids.h"
 
 #include <set>
 
 #include "base/macros.h"
 #include "components/sync/base/cryptographer.h"
@@ skipping 144 matching lines @@
 
   // First step of traversal building. Adds non-deleted items in order.
   void AddCreatesAndMoves(const std::set<int64_t>& ready_unsynced_set);
 
   // Second step of traverals building. Appends deleted items.
   void AddDeletes(const std::set<int64_t>& ready_unsynced_set);
 
  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,
                   Directory::Metahandles* result) const;
 
   void AddDeletedParents(const std::set<int64_t>& ready_unsynced_set,
                          const Entry& item,
                          const Directory::Metahandles& traversed,
                          Directory::Metahandles* result) const;
 
@@ skipping 17 matching lines @@
   DISALLOW_COPY_AND_ASSIGN(Traversal);
 };
 
 Traversal::Traversal(syncable::BaseTransaction* trans,
                      int64_t max_entries,
                      Directory::Metahandles* out)
     : out_(out), max_entries_(max_entries), trans_(trans) {}
 
 Traversal::~Traversal() {}
 
-void Traversal::AddUncommittedParents(
+bool Traversal::TryAddUncommittedParents(
     const std::set<int64_t>& ready_unsynced_set,
     const Entry& item,
     Directory::Metahandles* result) const {
   DCHECK(SupportsHierarchy(item));
   Directory::Metahandles dependencies;
   syncable::Id parent_id = item.GetParentId();
 
   // 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).
       // We can return early.
       break;
     }
 
     if (!TryAddItem(ready_unsynced_set, parent, &dependencies)) {
       // The parent isn't in |ready_unsynced_set|.
       break;
     }
 
     parent_id = parent.GetParentId();
   }
 
   // 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.
 bool Traversal::TryAddItem(const std::set<int64_t>& ready_unsynced_set,
                            const Entry& item,
                            Directory::Metahandles* result) const {
   DCHECK(item.GetIsUnsynced());
   int64_t item_handle = item.GetMetahandle();
   if (ready_unsynced_set.count(item_handle) != 0) {
     result->push_back(item_handle);
@@ skipping 86 matching lines @@
     }
     if (HaveItem(handle))
       continue;
 
     Entry entry(trans_, syncable::GET_BY_HANDLE, handle);
     if (!entry.GetIsDel()) {
       if (SupportsHierarchy(entry)) {
         // 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 {
         // No hierarchy dependencies, just commit the item itself.
         AppendToTraversal(handle);
       }
     }
   }
 
   // It's possible that we over committed while trying to expand dependent
@@ skipping 40 matching lines @@
   AppendManyToTraversal(deletion_list);
 
   // It's possible that we over committed while trying to expand dependent
   // items. If so, truncate the set down to the allowed size. This is safe
   // because of the reverse above, which should guarantee the leafy nodes are
   // in the front of the ancestors nodes.
   if (out_->size() > max_entries_)
     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,
     std::set<int64_t>* ready_unsynced_set) {
   for (const int64_t& deleted_conflicted_handle : deleted_conflicted_items) {
     Entry item(trans, syncable::GET_BY_HANDLE, deleted_conflicted_handle);
     syncable::Id parent_id = item.GetParentId();
     DCHECK(!parent_id.IsNull());
 
     while (!parent_id.IsRoot()) {
@@ skipping 126 matching lines @@
                        &ready_unsynced_set);
 
   OrderCommitIds(trans, max_entries, ready_unsynced_set, out);
 
   for (const int64_t& handle : *out) {
     DVLOG(1) << "Debug commit batch result:" << handle;
   }
 }
 
 }  // namespace syncer