[Sync] Don't commit items with predecessors/parents in conflict.

chrome/browser/sync/engine/get_commit_ids_command.cc

Index: chrome/browser/sync/engine/get_commit_ids_command.cc
diff --git a/chrome/browser/sync/engine/get_commit_ids_command.cc b/chrome/browser/sync/engine/get_commit_ids_command.cc
index 720c063f23d13663a8e7cc5f34ec3340dbbb7203..9363e79494be761c99c922f1015d231552a4ef55 100644
--- a/chrome/browser/sync/engine/get_commit_ids_command.cc
+++ b/chrome/browser/sync/engine/get_commit_ids_command.cc
@@ -76,11 +76,8 @@ namespace {
 // 2. Its type is not currently throttled.
 bool IsEntryReadyForCommit(syncable::ModelTypeSet encrypted_types,
                            bool passphrase_missing,
-                           const syncable::Entry& entry,
-                           syncable::ModelTypeSet throttled_types) {
-  if (!entry.Get(syncable::IS_UNSYNCED))
-    return false;
-
+                           syncable::ModelTypeSet throttled_types,
+                           const syncable::Entry& entry) {
   if (entry.Get(syncable::SERVER_VERSION) > 0 &&
       (entry.Get(syncable::SERVER_VERSION) >
        entry.Get(syncable::BASE_VERSION))) {
@@ -128,11 +125,13 @@ void GetCommitIdsCommand::FilterUnreadyEntries(
        iter != unsynced_handles->end();
        ++iter) {
     syncable::Entry entry(trans, syncable::GET_BY_HANDLE, *iter);
-    if (IsEntryReadyForCommit(encrypted_types_,
+    if (entry.Get(syncable::IS_UNSYNCED) &&
+        IsEntryReadyForCommit(encrypted_types_,
                               passphrase_missing_,
-                              entry,
-                              throttled_types))
+                              throttled_types,
+                              entry)) {
       new_unsynced_handles.push_back(*iter);
+    }
   }
   if (new_unsynced_handles.size() != unsynced_handles->size())
     unsynced_handles->swap(new_unsynced_handles);
@@ -155,9 +154,13 @@ void GetCommitIdsCommand::AddUncommittedParentsAndTheirPredecessors(
       break;
     }
     if (!AddItemThenPredecessors(trans, throttled_types, &parent,
-                                 syncable::IS_UNSYNCED,
                                  &item_dependencies)) {
-      break;  // Parent was already present in the set.
+      // Either the parent/it's predecessor's are not ready for commit, in which

[akalin, 2011/12/14 03:39:52]

it's -> its
predecessor's -> predecessors

+      // case item_dependencies has been cleared and we should return, or the
+      // parent has already been added to the commit set, in which case it's
+      // predecessors and parent's must already be in there too. Either way,
+      // no more work to do in this loop.
+      return;
     }
     parent_id = parent.Get(syncable::PARENT_ID);
   }
@@ -169,9 +172,14 @@ void GetCommitIdsCommand::AddUncommittedParentsAndTheirPredecessors(
 bool GetCommitIdsCommand::AddItem(syncable::Entry* item,
                                   syncable::ModelTypeSet throttled_types,
                                   OrderedCommitSet* result) {
-  if (!IsEntryReadyForCommit(encrypted_types_, passphrase_missing_, *item,
-                             throttled_types))
+  DCHECK(item->Get(syncable::IS_UNSYNCED));
+  if (!IsEntryReadyForCommit(encrypted_types_, passphrase_missing_,
+                             throttled_types, *item)) {
+    // If the item is not ready for commit, then all items that depend on this
+    // one are also not ready for commit. Clear the OrderedCommitSet and return.
+    result->Clear();
     return false;
+  }
   int64 item_handle = item->Get(syncable::META_HANDLE);
   if (result->HaveCommitItem(item_handle) ||
       ordered_commit_set_->HaveCommitItem(item_handle)) {
@@ -186,7 +194,6 @@ bool GetCommitIdsCommand::AddItemThenPredecessors(
     syncable::BaseTransaction* trans,
     syncable::ModelTypeSet throttled_types,
     syncable::Entry* item,
-    syncable::IndexedBitField inclusion_filter,
     OrderedCommitSet* result) {
   if (!AddItem(item, throttled_types, result))
     return false;
@@ -197,10 +204,10 @@ bool GetCommitIdsCommand::AddItemThenPredecessors(
   while (!prev_id.IsRoot()) {
     syncable::Entry prev(trans, syncable::GET_BY_ID, prev_id);
     CHECK(prev.good()) << "Bad id when walking predecessors.";
-    if (!prev.Get(inclusion_filter))
+    if (!prev.Get(syncable::IS_UNSYNCED))
       break;
     if (!AddItem(&prev, throttled_types, result))
-      break;
+      return false;
     prev_id = prev.Get(syncable::PREV_ID);
   }
   return true;
@@ -210,11 +217,14 @@ void GetCommitIdsCommand::AddPredecessorsThenItem(
     syncable::BaseTransaction* trans,
     syncable::ModelTypeSet throttled_types,
     syncable::Entry* item,
-    syncable::IndexedBitField inclusion_filter,
     const ModelSafeRoutingInfo& routes) {
   OrderedCommitSet item_dependencies(routes);
-  AddItemThenPredecessors(trans, throttled_types, item, inclusion_filter,
-                          &item_dependencies);
+  if (!AddItemThenPredecessors(trans, throttled_types, item,
+                               &item_dependencies)) {
+    // Either the item or it's predecessors are not ready for commit, so don't
+    // add any items to the commit set.
+    return;
+  }
 
   // Reverse what we added to get the correct order.
   ordered_commit_set_->AppendReverse(item_dependencies);
@@ -243,7 +253,7 @@ void GetCommitIdsCommand::AddCreatesAndMoves(
       AddUncommittedParentsAndTheirPredecessors(write_transaction,
           entry.Get(syncable::PARENT_ID), routes, throttled_types);
       AddPredecessorsThenItem(write_transaction, throttled_types, &entry,
-                              syncable::IS_UNSYNCED, routes);
+                              routes);
     }
   }