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

chrome/browser/sync/engine/get_commit_ids_command.cc

 // Copyright (c) 2011 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 "chrome/browser/sync/engine/get_commit_ids_command.h"
 
 #include <set>
 #include <utility>
 #include <vector>
 
@@ skipping 58 matching lines @@
 
 namespace {
 
 // An entry ready for commit is defined as:
 // 1. Not in conflict (SERVER_VERSION == BASE_VERSION || SERVER_VERSION == 0)
 // and not requiring encryption (any entry containing an encrypted datatype
 // while the cryptographer requires a passphrase is not ready for commit.)
 // 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))) {
     // The local and server versions don't match. The item must be in
     // conflict, so there's no point in attempting to commit.
     DCHECK(entry.Get(syncable::IS_UNAPPLIED_UPDATE));  // In conflict.
     DVLOG(1) << "Excluding entry from commit due to version mismatch "
              << entry;
     return false;
   }
@@ skipping 27 matching lines @@
     syncable::BaseTransaction* trans,
     syncable::ModelTypeSet throttled_types,
     syncable::Directory::UnsyncedMetaHandles* unsynced_handles) {
   syncable::Directory::UnsyncedMetaHandles::iterator iter;
   syncable::Directory::UnsyncedMetaHandles new_unsynced_handles;
   new_unsynced_handles.reserve(unsynced_handles->size());
   for (iter = unsynced_handles->begin();
        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);
 }
 
 void GetCommitIdsCommand::AddUncommittedParentsAndTheirPredecessors(
     syncable::BaseTransaction* trans,
     syncable::Id parent_id,
     const ModelSafeRoutingInfo& routes,
     syncable::ModelTypeSet throttled_types) {
   OrderedCommitSet item_dependencies(routes);
 
   // Climb the tree adding entries leaf -> root.
   while (!parent_id.ServerKnows()) {
     syncable::Entry parent(trans, syncable::GET_BY_ID, parent_id);
     CHECK(parent.good()) << "Bad user-only parent in item path.";
     int64 handle = parent.Get(syncable::META_HANDLE);
     if (ordered_commit_set_->HaveCommitItem(handle) ||
         item_dependencies.HaveCommitItem(handle)) {
       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);
   }
 
   // Reverse what we added to get the correct order.
   ordered_commit_set_->AppendReverse(item_dependencies);
 }
 
 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)) {
     return false;
   }
   result->AddCommitItem(item_handle, item->Get(syncable::ID),
                         item->GetModelType());
   return true;
 }
 
 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;
   if (item->Get(syncable::IS_DEL))
     return true;  // Deleted items have no predecessors.
 
   syncable::Id prev_id = item->Get(syncable::PREV_ID);
   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;
 }
 
 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);
 }
 
 bool GetCommitIdsCommand::IsCommitBatchFull() {
   return ordered_commit_set_->Size() >= requested_commit_batch_size_;
 }
 
 void GetCommitIdsCommand::AddCreatesAndMoves(
     const vector<int64>& unsynced_handles,
     syncable::WriteTransaction* write_transaction,
     const ModelSafeRoutingInfo& routes,
     syncable::ModelTypeSet throttled_types) {
   // Add moves and creates, and prepend their uncommitted parents.
   for (CommitMetahandleIterator iterator(unsynced_handles, write_transaction,
                                          ordered_commit_set_.get());
        !IsCommitBatchFull() && iterator.Valid();
        iterator.Increment()) {
     int64 metahandle = iterator.Current();
 
     syncable::Entry entry(write_transaction,
                           syncable::GET_BY_HANDLE,
                           metahandle);
     if (!entry.Get(syncable::IS_DEL)) {
       AddUncommittedParentsAndTheirPredecessors(write_transaction,
           entry.Get(syncable::PARENT_ID), routes, throttled_types);
       AddPredecessorsThenItem(write_transaction, throttled_types, &entry,
-                              syncable::IS_UNSYNCED, routes);
+                              routes);
     }
   }
 
   // It's possible that we overcommitted while trying to expand dependent
   // items.  If so, truncate the set down to the allowed size.
   ordered_commit_set_->Truncate(requested_commit_batch_size_);
 }
 
 void GetCommitIdsCommand::AddDeletes(const vector<int64>& unsynced_handles,
     syncable::WriteTransaction* write_transaction) {
@@ skipping 95 matching lines @@
 
   // Add moves and creates, and prepend their uncommitted parents.
   AddCreatesAndMoves(unsynced_handles, write_transaction, routes,
                      throttled_types);
 
   // Add all deletes.
   AddDeletes(unsynced_handles, write_transaction);
 }
 
 }  // namespace browser_sync