For sync, make ProcessCommitResponseCommand handle commit entries one ModelSafeGroup at a time.

Enforce ModelSafeGroup restrictions in the StatusController by DCHECKing if out-of-bounds.
Move OrderedCommitSet to its own file.
Removed some unused error counters and ResetTransientState calls that were useless.

BUG=31911
TEST=ProcessCommitResponseTest, OrderedCommitSetTest

Review URL: http://codereview.chromium.org/604045

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@39139 0039d316-1c4b-4281-b951-d872f2087c98

26 files changed, 688 insertions, 466 deletions

diff --git a/chrome/browser/sync/engine/build_commit_command.cc b/chrome/browser/sync/engine/build_commit_command.cc
index f1c3d52..cf6b4f53 100755
--- a/chrome/browser/sync/engine/build_commit_command.cc
+++ b/chrome/browser/sync/engine/build_commit_command.cc
@@ -35,6 +35,11 @@ BuildCommitCommand::~BuildCommitCommand() {}
 
 void BuildCommitCommand::AddExtensionsActivityToMessage(
     SyncSession* session, CommitMessage* message) {
+  // We only send ExtensionsActivity to the server if bookmarks are being
+  // committed.
+  if (!session->status_controller()->HasBookmarkCommitActivity())
+    return;
+
   const ExtensionsActivityMonitor::Records& records =
       session->extensions_activity();
   for (ExtensionsActivityMonitor::Records::const_iterator it = records.begin();
diff --git a/chrome/browser/sync/engine/download_updates_command.cc b/chrome/browser/sync/engine/download_updates_command.cc
index cc58874..0dd198d 100644
--- a/chrome/browser/sync/engine/download_updates_command.cc
+++ b/chrome/browser/sync/engine/download_updates_command.cc
@@ -63,7 +63,6 @@ void DownloadUpdatesCommand::ExecuteImpl(SyncSession* session) {
 
   StatusController* status = session->status_controller();
   if (!ok) {
-    status->increment_num_consecutive_problem_get_updates();
     status->increment_num_consecutive_errors();
     LOG(ERROR) << "PostClientToServerMessage() failed";
     return;
diff --git a/chrome/browser/sync/engine/get_commit_ids_command.cc b/chrome/browser/sync/engine/get_commit_ids_command.cc
index 781c545..5b6d228 100644
--- a/chrome/browser/sync/engine/get_commit_ids_command.cc
+++ b/chrome/browser/sync/engine/get_commit_ids_command.cc
@@ -17,6 +17,7 @@ using std::vector;
 
 namespace browser_sync {
 
+using sessions::OrderedCommitSet;
 using sessions::SyncSession;
 using sessions::StatusController;
 
@@ -38,51 +39,50 @@ void GetCommitIdsCommand::ExecuteImpl(SyncSession* session) {
                  session->routing_info());
 
   const vector<syncable::Id>& verified_commit_ids =
-      ordered_commit_set_.GetAllCommitIds();
+      ordered_commit_set_->GetAllCommitIds();
 
   for (size_t i = 0; i < verified_commit_ids.size(); i++)
     LOG(INFO) << "Debug commit batch result:" << verified_commit_ids[i];
 
-  status->set_commit_ids(verified_commit_ids);
+  status->set_commit_set(*ordered_commit_set_.get());
 }
 
 void GetCommitIdsCommand::AddUncommittedParentsAndTheirPredecessors(
     syncable::BaseTransaction* trans,
     syncable::Id parent_id,
-    const ModelSafeRoutingInfo& routing_info) {
+    const ModelSafeRoutingInfo& routes) {
   using namespace syncable;
-  OrderedCommitSet item_dependencies;
+  OrderedCommitSet item_dependencies(routes);
 
   // Climb the tree adding entries leaf -> root.
   while (!parent_id.ServerKnows()) {
     Entry parent(trans, GET_BY_ID, parent_id);
     CHECK(parent.good()) << "Bad user-only parent in item path.";
     int64 handle = parent.Get(META_HANDLE);
-    if (ordered_commit_set_.HaveCommitItem(handle) ||
+    if (ordered_commit_set_->HaveCommitItem(handle) ||
         item_dependencies.HaveCommitItem(handle)) {
       break;
     }
     if (!AddItemThenPredecessors(trans, &parent, IS_UNSYNCED,
-                                 &item_dependencies, routing_info)) {
+                                 &item_dependencies)) {
       break;  // Parent was already present in the set.
     }
     parent_id = parent.Get(PARENT_ID);
   }
 
   // Reverse what we added to get the correct order.
-  ordered_commit_set_.AppendReverse(item_dependencies);
+  ordered_commit_set_->AppendReverse(item_dependencies);
 }
 
 bool GetCommitIdsCommand::AddItem(syncable::Entry* item,
-                                  OrderedCommitSet* result,
-                                  const ModelSafeRoutingInfo& routing_info) {
+                                  OrderedCommitSet* result) {
   int64 item_handle = item->Get(syncable::META_HANDLE);
   if (result->HaveCommitItem(item_handle) ||
-      ordered_commit_set_.HaveCommitItem(item_handle)) {
+      ordered_commit_set_->HaveCommitItem(item_handle)) {
     return false;
   }
   result->AddCommitItem(item_handle, item->Get(syncable::ID),
-      GetGroupForEntry(item, routing_info));
+                        item->GetModelType());
   return true;
 }
 
@@ -90,9 +90,8 @@ bool GetCommitIdsCommand::AddItemThenPredecessors(
     syncable::BaseTransaction* trans,
     syncable::Entry* item,
     syncable::IndexedBitField inclusion_filter,
-    OrderedCommitSet* result,
-    const ModelSafeRoutingInfo& routing_info) {
-  if (!AddItem(item, result, routing_info))
+    OrderedCommitSet* result) {
+  if (!AddItem(item, result))
     return false;
   if (item->Get(syncable::IS_DEL))
     return true;  // Deleted items have no predecessors.
@@ -103,7 +102,7 @@ bool GetCommitIdsCommand::AddItemThenPredecessors(
     CHECK(prev.good()) << "Bad id when walking predecessors.";
     if (!prev.Get(inclusion_filter))
       break;
-    if (!AddItem(&prev, result, routing_info))
+    if (!AddItem(&prev, result))
       break;
     prev_id = prev.Get(syncable::PREV_ID);
   }
@@ -114,26 +113,25 @@ void GetCommitIdsCommand::AddPredecessorsThenItem(
     syncable::BaseTransaction* trans,
     syncable::Entry* item,
     syncable::IndexedBitField inclusion_filter,
-    const ModelSafeRoutingInfo& routing_info) {
-  OrderedCommitSet item_dependencies;
-  AddItemThenPredecessors(trans, item, inclusion_filter, &item_dependencies,
-                          routing_info);
+    const ModelSafeRoutingInfo& routes) {
+  OrderedCommitSet item_dependencies(routes);
+  AddItemThenPredecessors(trans, item, inclusion_filter, &item_dependencies);
 
   // Reverse what we added to get the correct order.
-  ordered_commit_set_.AppendReverse(item_dependencies);
+  ordered_commit_set_->AppendReverse(item_dependencies);
 }
 
 bool GetCommitIdsCommand::IsCommitBatchFull() {
-  return ordered_commit_set_.Size() >= requested_commit_batch_size_;
+  return ordered_commit_set_->Size() >= requested_commit_batch_size_;
 }
 
 void GetCommitIdsCommand::AddCreatesAndMoves(
     const vector<int64>& unsynced_handles,
     syncable::WriteTransaction* write_transaction,
-    const ModelSafeRoutingInfo& routing_info) {
+    const ModelSafeRoutingInfo& routes) {
   // Add moves and creates, and prepend their uncommitted parents.
   for (CommitMetahandleIterator iterator(unsynced_handles, write_transaction,
-                                         &ordered_commit_set_);
+                                         ordered_commit_set_.get());
        !IsCommitBatchFull() && iterator.Valid();
        iterator.Increment()) {
     int64 metahandle = iterator.Current();
@@ -143,25 +141,23 @@ void GetCommitIdsCommand::AddCreatesAndMoves(
                           metahandle);
     if (!entry.Get(syncable::IS_DEL)) {
       AddUncommittedParentsAndTheirPredecessors(write_transaction,
-                                                entry.Get(syncable::PARENT_ID),
-                                                routing_info);
-      AddPredecessorsThenItem(write_transaction, &entry, syncable::IS_UNSYNCED,
-                              routing_info);
+          entry.Get(syncable::PARENT_ID), routes);
+      AddPredecessorsThenItem(write_transaction, &entry,
+          syncable::IS_UNSYNCED, 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_);
+  ordered_commit_set_->Truncate(requested_commit_batch_size_);
 }
 
 void GetCommitIdsCommand::AddDeletes(const vector<int64>& unsynced_handles,
-    syncable::WriteTransaction* write_transaction,
-    const ModelSafeRoutingInfo& routing_info) {
+    syncable::WriteTransaction* write_transaction) {
   set<syncable::Id> legal_delete_parents;
 
   for (CommitMetahandleIterator iterator(unsynced_handles, write_transaction,
-                                         &ordered_commit_set_);
+                                         ordered_commit_set_.get());
        !IsCommitBatchFull() && iterator.Valid();
        iterator.Increment()) {
     int64 metahandle = iterator.Current();
@@ -194,9 +190,9 @@ void GetCommitIdsCommand::AddDeletes(const vector<int64>& unsynced_handles,
           LOG(INFO) << "Inserting moved and deleted entry, will be missed by"
             " delete roll." << entry.Get(syncable::ID);
 
-          ordered_commit_set_.AddCommitItem(metahandle,
+          ordered_commit_set_->AddCommitItem(metahandle,
               entry.Get(syncable::ID),
-              GetGroupForEntry(&entry, routing_info));
+              entry.GetModelType());
         }
 
         // Skip this entry since it's a child of a parent that will be
@@ -224,7 +220,7 @@ void GetCommitIdsCommand::AddDeletes(const vector<int64>& unsynced_handles,
   //   parent was not deleted
 
   for (CommitMetahandleIterator iterator(unsynced_handles, write_transaction,
-                                         &ordered_commit_set_);
+                                         ordered_commit_set_.get());
       !IsCommitBatchFull() && iterator.Valid();
       iterator.Increment()) {
     int64 metahandle = iterator.Current();
@@ -233,8 +229,8 @@ void GetCommitIdsCommand::AddDeletes(const vector<int64>& unsynced_handles,
     if (entry.Get(syncable::IS_DEL)) {
       syncable::Id parent_id = entry.Get(syncable::PARENT_ID);
       if (legal_delete_parents.count(parent_id)) {
-        ordered_commit_set_.AddCommitItem(metahandle, entry.Get(syncable::ID),
-            GetGroupForEntry(&entry, routing_info));
+        ordered_commit_set_->AddCommitItem(metahandle, entry.Get(syncable::ID),
+            entry.GetModelType());
       }
     }
   }
@@ -242,7 +238,8 @@ void GetCommitIdsCommand::AddDeletes(const vector<int64>& unsynced_handles,
 
 void GetCommitIdsCommand::BuildCommitIds(const vector<int64>& unsynced_handles,
     syncable::WriteTransaction* write_transaction,
-    const ModelSafeRoutingInfo& routing_info) {
+    const ModelSafeRoutingInfo& routes) {
+  ordered_commit_set_.reset(new OrderedCommitSet(routes));
   // Commits follow these rules:
   // 1. Moves or creates are preceded by needed folder creates, from
   //    root to leaf.  For folders whose contents are ordered, moves
@@ -253,10 +250,10 @@ void GetCommitIdsCommand::BuildCommitIds(const vector<int64>& unsynced_handles,
   // delete trees.
 
   // Add moves and creates, and prepend their uncommitted parents.
-  AddCreatesAndMoves(unsynced_handles, write_transaction, routing_info);
+  AddCreatesAndMoves(unsynced_handles, write_transaction, routes);
 
   // Add all deletes.
-  AddDeletes(unsynced_handles, write_transaction, routing_info);
+  AddDeletes(unsynced_handles, write_transaction);
 }
 
 }  // namespace browser_sync
diff --git a/chrome/browser/sync/engine/get_commit_ids_command.h b/chrome/browser/sync/engine/get_commit_ids_command.h
index 8ed0f92..40f8134 100644
--- a/chrome/browser/sync/engine/get_commit_ids_command.h
+++ b/chrome/browser/sync/engine/get_commit_ids_command.h
@@ -10,6 +10,7 @@
 
 #include "chrome/browser/sync/engine/syncer_command.h"
 #include "chrome/browser/sync/engine/syncer_util.h"
+#include "chrome/browser/sync/sessions/ordered_commit_set.h"
 #include "chrome/browser/sync/sessions/sync_session.h"
 #include "chrome/browser/sync/util/sync_types.h"
 
@@ -31,139 +32,7 @@ class GetCommitIdsCommand : public SyncerCommand {
   // Builds a vector of IDs that should be committed.
   void BuildCommitIds(const vector<int64>& unsynced_handles,
                       syncable::WriteTransaction* write_transaction,
-                      const ModelSafeRoutingInfo& routing_info);
-
-  // These classes are public for testing.
-  // TODO(ncarter): This code is more generic than just Commit and can
-  // be reused elsewhere (e.g. ChangeReorderBuffer do similar things).  Merge
-  // all these implementations.
-  // TODO(tim): Move this to its own file.  Also, SyncSession should now
-  // hold an OrderedCommitSet instead of just the vector<syncable::Id>.  Will
-  // do this in upcoming CL.
-  class OrderedCommitSet {
-   public:
-    // A list of indices into the full list of commit ids such that:
-    // 1 - each element is an index belonging to a particular ModelSafeGroup.
-    // 2 - the vector is in sorted (smallest to largest) order.
-    // 3 - each element is a valid index for GetCommitItemAt.
-    // See GetCommitIdProjection for usage.
-    typedef std::vector<size_t> Projection;
-
-    // TODO(chron): Reserve space according to batch size?
-    OrderedCommitSet() {}
-    ~OrderedCommitSet() {}
-
-    bool HaveCommitItem(const int64 metahandle) const {
-      return inserted_metahandles_.count(metahandle) > 0;
-    }
-
-    void AddCommitItem(const int64 metahandle, const syncable::Id& commit_id,
-                       ModelSafeGroup group) {
-      if (!HaveCommitItem(metahandle)) {
-        inserted_metahandles_.insert(metahandle);
-        metahandle_order_.push_back(metahandle);
-        commit_ids_.push_back(commit_id);
-        projections_[group].push_back(commit_ids_.size() - 1);
-        groups_.push_back(group);
-      }
-    }
-
-    const vector<syncable::Id>& GetAllCommitIds() const {
-      return commit_ids_;
-    }
-
-    // Return the Id at index |position| in this OrderedCommitSet.  Note that
-    // the index uniquely identifies the same logical item in each of:
-    // 1) this OrderedCommitSet
-    // 2) the CommitRequest sent to the server
-    // 3) the list of EntryResponse objects in the CommitResponse.
-    // These together allow re-association of the pre-commit Id with the
-    // actual committed entry.
-    const syncable::Id& GetCommitIdAt(const size_t position) const {
-      return commit_ids_[position];
-    }
-
-    // Get the projection of commit ids onto the space of commit ids
-    // belonging to |group|.  This is useful when you need to process a commit
-    // response one ModelSafeGroup at a time. See GetCommitIdAt for how the
-    // indices contained in the returned Projection can be used.
-    const Projection& GetCommitIdProjection(ModelSafeGroup group) {
-      return projections_[group];
-    }
-
-    int Size() const {
-      return commit_ids_.size();
-    }
-
-    void AppendReverse(const OrderedCommitSet& other) {
-      for (int i = other.Size() - 1; i >= 0; i--) {
-        CommitItem item = other.GetCommitItemAt(i);
-        AddCommitItem(item.meta, item.id, item.group);
-      }
-    }
-
-    void Truncate(size_t max_size) {
-      if (max_size < metahandle_order_.size()) {
-        for (size_t i = max_size; i < metahandle_order_.size(); ++i) {
-          inserted_metahandles_.erase(metahandle_order_[i]);
-        }
-
-        // Some projections may refer to indices that are getting chopped.
-        // Since projections are in increasing order, it's easy to fix. Except
-        // that you can't erase(..) using a reverse_iterator, so we use binary
-        // search to find the chop point.
-        Projections::iterator it = projections_.begin();
-        for (; it != projections_.end(); ++it) {
-          // For each projection, chop off any indices larger than or equal to
-          // max_size by looking for max_size using binary search.
-          Projection& p = it->second;
-          Projection::iterator element = std::lower_bound(p.begin(), p.end(),
-                                                          max_size);
-          if (element != p.end())
-            p.erase(element, p.end());
-        }
-        commit_ids_.resize(max_size);
-        metahandle_order_.resize(max_size);
-        groups_.resize(max_size);
-      }
-    }
-
-   private:
-    // A set of CommitIdProjections associated with particular ModelSafeGroups.
-    typedef std::map<ModelSafeGroup, Projection> Projections;
-
-    // Helper container for return value of GetCommitItemAt.
-    struct CommitItem {
-      int64 meta;
-      syncable::Id id;
-      ModelSafeGroup group;
-    };
-
-    CommitItem GetCommitItemAt(const int position) const {
-      DCHECK(position < Size());
-      CommitItem return_item = {metahandle_order_[position],
-                                commit_ids_[position],
-                                groups_[position]};
-      return return_item;
-    }
-
-    // These lists are different views of the same items; e.g they are
-    // isomorphic.
-    syncable::MetahandleSet inserted_metahandles_;
-    vector<syncable::Id> commit_ids_;
-    vector<int64> metahandle_order_;
-    Projections projections_;
-
-    // We need this because of operations like AppendReverse that take ids from
-    // one OrderedCommitSet and insert into another -- we need to know the
-    // group for each ID so that the insertion can update the appropriate
-    // projection.  We could store it in commit_ids_, but sometimes we want
-    // to just return the vector of Ids, so this is more straightforward
-    // and shouldn't take up too much extra space since commit lists are small.
-    std::vector<ModelSafeGroup> groups_;
-
-    DISALLOW_COPY_AND_ASSIGN(OrderedCommitSet);
-  };
+                      const ModelSafeRoutingInfo& routes);
 
   // TODO(chron): Remove writes from this iterator. As a warning, this
   // iterator causes writes to entries and so isn't a pure iterator.
@@ -174,7 +43,7 @@ class GetCommitIdsCommand : public SyncerCommand {
     // UTF8 conversion and filename checking
     CommitMetahandleIterator(const vector<int64>& unsynced_handles,
                              syncable::WriteTransaction* write_transaction,
-                             OrderedCommitSet* commit_set)
+                             sessions::OrderedCommitSet* commit_set)
         : write_transaction_(write_transaction),
           handle_iterator_(unsynced_handles.begin()),
           unsynced_handles_end_(unsynced_handles.end()),
@@ -232,7 +101,7 @@ class GetCommitIdsCommand : public SyncerCommand {
      syncable::WriteTransaction* const write_transaction_;
      vector<int64>::const_iterator handle_iterator_;
      vector<int64>::const_iterator unsynced_handles_end_;
-     OrderedCommitSet* commit_set_;
+     sessions::OrderedCommitSet* commit_set_;
 
      DISALLOW_COPY_AND_ASSIGN(CommitMetahandleIterator);
   };
@@ -241,34 +110,31 @@ class GetCommitIdsCommand : public SyncerCommand {
   void AddUncommittedParentsAndTheirPredecessors(
       syncable::BaseTransaction* trans,
       syncable::Id parent_id,
-      const ModelSafeRoutingInfo& routing_info);
+      const ModelSafeRoutingInfo& routes);
 
   // OrderedCommitSet helpers for adding predecessors in order.
   // TODO(ncarter): Refactor these so that the |result| parameter goes away,
   // and AddItem doesn't need to consider two OrderedCommitSets.
-  bool AddItem(syncable::Entry* item, OrderedCommitSet* result,
-               const ModelSafeRoutingInfo& routing_info);
+  bool AddItem(syncable::Entry* item, sessions::OrderedCommitSet* result);
   bool AddItemThenPredecessors(syncable::BaseTransaction* trans,
                                syncable::Entry* item,
                                syncable::IndexedBitField inclusion_filter,
-                               OrderedCommitSet* result,
-                               const ModelSafeRoutingInfo& routing_info);
+                               sessions::OrderedCommitSet* result);
   void AddPredecessorsThenItem(syncable::BaseTransaction* trans,
                                syncable::Entry* item,
                                syncable::IndexedBitField inclusion_filter,
-                               const ModelSafeRoutingInfo& routing_info);
+                               const ModelSafeRoutingInfo& routes);
 
   bool IsCommitBatchFull();
 
   void AddCreatesAndMoves(const vector<int64>& unsynced_handles,
                           syncable::WriteTransaction* write_transaction,
-                          const ModelSafeRoutingInfo& routing_info);
+                          const ModelSafeRoutingInfo& routes);
 
   void AddDeletes(const vector<int64>& unsynced_handles,
-                  syncable::WriteTransaction* write_transaction,
-                  const ModelSafeRoutingInfo& routing_info);
+                  syncable::WriteTransaction* write_transaction);
 
-  OrderedCommitSet ordered_commit_set_;
+  scoped_ptr<sessions::OrderedCommitSet> ordered_commit_set_;
 
   int requested_commit_batch_size_;
 
diff --git a/chrome/browser/sync/engine/mock_model_safe_workers.h b/chrome/browser/sync/engine/mock_model_safe_workers.h
new file mode 100644
index 0000000..e776596
--- /dev/null
+++ b/chrome/browser/sync/engine/mock_model_safe_workers.h
@@ -0,0 +1,25 @@
+// Copyright (c) 2010 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.
+
+#ifndef CHROME_BROWSER_SYNC_ENGINE_MOCK_MODEL_SAFE_WORKERS_H_
+#define CHROME_BROWSER_SYNC_ENGINE_MOCK_MODEL_SAFE_WORKERS_H_
+
+#include "chrome/browser/sync/engine/model_safe_worker.h"
+
+namespace browser_sync {
+
+class MockUIModelWorker : public ModelSafeWorker {
+ public:
+  virtual ModelSafeGroup GetModelSafeGroup() { return GROUP_UI; }
+};
+
+class MockDBModelWorker : public ModelSafeWorker {
+ public:
+  virtual ModelSafeGroup GetModelSafeGroup() { return GROUP_DB; }
+};
+
+}  // namespace browser_sync
+
+#endif  // CHROME_BROWSER_SYNC_ENGINE_MOCK_MODEL_SAFE_WORKERS_H_
+
diff --git a/chrome/browser/sync/engine/model_changing_syncer_command.cc b/chrome/browser/sync/engine/model_changing_syncer_command.cc
index f531a70..e6d34a2 100644
--- a/chrome/browser/sync/engine/model_changing_syncer_command.cc
+++ b/chrome/browser/sync/engine/model_changing_syncer_command.cc
@@ -12,6 +12,8 @@ namespace browser_sync {
 
 void ModelChangingSyncerCommand::ExecuteImpl(sessions::SyncSession* session) {
   work_session_ = session;
+  ModelNeutralExecuteImpl(work_session_);
+
   for (size_t i = 0; i < session->workers().size(); ++i) {
     ModelSafeWorker* worker = session->workers()[i];
     ModelSafeGroup group = worker->GetModelSafeGroup();
diff --git a/chrome/browser/sync/engine/model_changing_syncer_command.h b/chrome/browser/sync/engine/model_changing_syncer_command.h
index b2ebee1..eb8003a 100644
--- a/chrome/browser/sync/engine/model_changing_syncer_command.h
+++ b/chrome/browser/sync/engine/model_changing_syncer_command.h
@@ -35,7 +35,16 @@ class ModelChangingSyncerCommand : public SyncerCommand {
     ModelChangingExecuteImpl(work_session_);
   }
 
-  // Abstract method to be implemented by subclasses.
+  // Sometimes, a command has work to do that needs to touch global state
+  // belonging to multiple ModelSafeGroups, but in a way that is known to be
+  // safe.  This will be called once, prior to ModelChangingExecuteImpl,
+  // *without* a ModelSafeGroup restriction in place on the SyncSession.
+  virtual void ModelNeutralExecuteImpl(sessions::SyncSession* session) {}
+
+  // Abstract method to be implemented by subclasses to handle logic that
+  // operates on the model.  This is invoked with a SyncSession ModelSafeGroup
+  // restriction in place so that bits of state belonging to data types
+  // running on an unsafe thread are siloed away.
   virtual void ModelChangingExecuteImpl(sessions::SyncSession* session) = 0;
 
  private:
diff --git a/chrome/browser/sync/engine/model_safe_worker.cc b/chrome/browser/sync/engine/model_safe_worker.cc
index 9c71fd3..3d42164 100644
--- a/chrome/browser/sync/engine/model_safe_worker.cc
+++ b/chrome/browser/sync/engine/model_safe_worker.cc
@@ -3,13 +3,12 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/sync/engine/model_safe_worker.h"
-#include "chrome/browser/sync/syncable/syncable.h"
 
 namespace browser_sync {
 
-ModelSafeGroup GetGroupForEntry(const syncable::Entry* e,
-                                const ModelSafeRoutingInfo& routes) {
-  ModelSafeRoutingInfo::const_iterator it = routes.find(e->GetModelType());
+ModelSafeGroup GetGroupForModelType(const syncable::ModelType type,
+                                    const ModelSafeRoutingInfo& routes) {
+  ModelSafeRoutingInfo::const_iterator it = routes.find(type);
   if (it == routes.end()) {
     NOTREACHED() << "Entry does not belong to active ModelSafeGroup!";
     return GROUP_PASSIVE;
diff --git a/chrome/browser/sync/engine/model_safe_worker.h b/chrome/browser/sync/engine/model_safe_worker.h
index e6552db..5e5b70d 100644
--- a/chrome/browser/sync/engine/model_safe_worker.h
+++ b/chrome/browser/sync/engine/model_safe_worker.h
@@ -13,10 +13,6 @@
 #include "chrome/browser/sync/util/closure.h"
 #include "chrome/browser/sync/util/sync_types.h"
 
-namespace syncable {
-class Entry;
-}
-
 namespace browser_sync {
 
 enum ModelSafeGroup {
@@ -63,8 +59,8 @@ class ModelSafeWorker : public base::RefCountedThreadSafe<ModelSafeWorker> {
 typedef std::map<syncable::ModelType, ModelSafeGroup>
     ModelSafeRoutingInfo;
 
-ModelSafeGroup GetGroupForEntry(const syncable::Entry* e,
-                                const ModelSafeRoutingInfo& routes);
+ModelSafeGroup GetGroupForModelType(const syncable::ModelType type,
+                                    const ModelSafeRoutingInfo& routes);
 
 // Maintain the up-to-date state regarding which ModelSafeWorkers exist and
 // which types get routed to which worker.  When a sync session begins, it will
diff --git a/chrome/browser/sync/engine/post_commit_message_command.cc b/chrome/browser/sync/engine/post_commit_message_command.cc
index 9136df7..c0a7c98 100644
--- a/chrome/browser/sync/engine/post_commit_message_command.cc
+++ b/chrome/browser/sync/engine/post_commit_message_command.cc
@@ -34,7 +34,6 @@ void PostCommitMessageCommand::ExecuteImpl(sessions::SyncSession* session) {
     // set to true during BuildCommitCommand, and which may still be true.
     // Not to be confused with IS_UNSYNCED.  This bit is used to detect local
     // changes to items that happen during the server Commit operation.
-    status->increment_num_consecutive_problem_commits();
     status->increment_num_consecutive_errors();
     syncable::WriteTransaction trans(dir, syncable::SYNCER, __FILE__, __LINE__);
     const vector<syncable::Id>& commit_ids = status->commit_ids();
diff --git a/chrome/browser/sync/engine/process_commit_response_command.cc b/chrome/browser/sync/engine/process_commit_response_command.cc
index 19cd9bd..33b188e 100755
--- a/chrome/browser/sync/engine/process_commit_response_command.cc
+++ b/chrome/browser/sync/engine/process_commit_response_command.cc
@@ -40,34 +40,23 @@ using syncable::SYNCING;
 
 namespace browser_sync {
 
+using sessions::OrderedCommitSet;
 using sessions::StatusController;
 using sessions::SyncSession;
 using sessions::ConflictProgress;
 
 void IncrementErrorCounters(StatusController* status) {
-  status->increment_num_consecutive_problem_commits();
   status->increment_num_consecutive_errors();
 }
 void ResetErrorCounters(StatusController* status) {
-  status->set_num_consecutive_problem_commits(0);
   status->set_num_consecutive_errors(0);
 }
 
 ProcessCommitResponseCommand::ProcessCommitResponseCommand() {}
 ProcessCommitResponseCommand::~ProcessCommitResponseCommand() {}
 
-void ProcessCommitResponseCommand::ModelChangingExecuteImpl(
-    SyncSession* session) {
-  ProcessCommitResponse(session);
-  ExtensionsActivityMonitor* monitor = session->context()->extensions_monitor();
-  if (session->status_controller()->syncer_status().num_successful_commits == 0)
-    monitor->PutRecords(session->extensions_activity());
-}
-
-void ProcessCommitResponseCommand::ProcessCommitResponse(
-    SyncSession* session) {
-  // TODO(sync): This function returns if it sees problems. We probably want
-  // to flag the need for an update or similar.
+void ProcessCommitResponseCommand::ModelNeutralExecuteImpl(
+    sessions::SyncSession* session) {
   ScopedDirLookup dir(session->context()->directory_manager(),
                       session->context()->account_name());
   if (!dir.good()) {
@@ -100,6 +89,33 @@ void ProcessCommitResponseCommand::ProcessCommitResponse(
     IncrementErrorCounters(status);
     return;
   }
+}
+
+void ProcessCommitResponseCommand::ModelChangingExecuteImpl(
+    SyncSession* session) {
+  ProcessCommitResponse(session);
+  ExtensionsActivityMonitor* monitor = session->context()->extensions_monitor();
+  if (session->status_controller()->HasBookmarkCommitActivity() &&
+      session->status_controller()->syncer_status()
+          .num_successful_bookmark_commits == 0) {
+    monitor->PutRecords(session->extensions_activity());
+  }
+}
+
+void ProcessCommitResponseCommand::ProcessCommitResponse(
+    SyncSession* session) {
+  // TODO(sync): This function returns if it sees problems. We probably want
+  // to flag the need for an update or similar.
+  ScopedDirLookup dir(session->context()->directory_manager(),
+                      session->context()->account_name());
+  if (!dir.good()) {
+    LOG(ERROR) << "Scoped dir lookup failed!";
+    return;
+  }
+
+  StatusController* status = session->status_controller();
+  const ClientToServerResponse& response(status->commit_response());
+  const CommitResponse& cr = response.commit();
 
   // If we try to commit a parent and child together and the parent conflicts
   // the child will have a bad parent causing an error. As this is not a
@@ -113,12 +129,13 @@ void ProcessCommitResponseCommand::ProcessCommitResponse(
   set<syncable::Id> conflicting_new_folder_ids;
   set<syncable::Id> deleted_folders;
   ConflictProgress* conflict_progress = status->mutable_conflict_progress();
+  OrderedCommitSet::Projection proj = status->commit_id_projection();
   { // Scope for WriteTransaction.
     WriteTransaction trans(dir, SYNCER, __FILE__, __LINE__);
-    for (int i = 0; i < cr.entryresponse_size(); i++) {
+    for (size_t i = 0; i < proj.size(); i++) {
       CommitResponse::ResponseType response_type =
-          ProcessSingleCommitResponse(&trans, cr.entryresponse(i),
-                                      commit_ids[i],
+          ProcessSingleCommitResponse(&trans, cr.entryresponse(proj[i]),
+                                      status->GetCommitIdAt(proj[i]),
                                       &conflicting_new_folder_ids,
                                       &deleted_folders);
       switch (response_type) {
@@ -128,11 +145,14 @@ void ProcessCommitResponseCommand::ProcessCommitResponse(
         case CommitResponse::CONFLICT:
           ++conflicting_commits;
           // Only server CONFLICT responses will activate conflict resolution.
-          conflict_progress->AddConflictingItemById(commit_ids[i]);
+          conflict_progress->AddConflictingItemById(
+              status->GetCommitIdAt(proj[i]));
           break;
         case CommitResponse::SUCCESS:
           // TODO(sync): worry about sync_rate_ rate calc?
           ++successes;
+          if (status->GetCommitIdModelTypeAt(proj[i]) == syncable::BOOKMARKS)
+            status->increment_num_successful_bookmark_commits();
           status->increment_num_successful_commits();
           break;
         case CommitResponse::OVER_QUOTA:
@@ -153,7 +173,7 @@ void ProcessCommitResponseCommand::ProcessCommitResponse(
   }
 
   // TODO(sync): move status reporting elsewhere.
-  status->set_num_conflicting_commits(conflicting_commits);
+  status->increment_num_conflicting_commits_by(conflicting_commits);
   if (0 == successes) {
     status->increment_num_consecutive_transient_error_commits_by(
         transient_error_commits);
@@ -162,6 +182,7 @@ void ProcessCommitResponseCommand::ProcessCommitResponse(
     status->set_num_consecutive_transient_error_commits(0);
     status->set_num_consecutive_errors(0);
   }
+  int commit_count = static_cast<int>(proj.size());
   if (commit_count != (conflicting_commits + error_commits +
                        transient_error_commits)) {
     ResetErrorCounters(status);
diff --git a/chrome/browser/sync/engine/process_commit_response_command.h b/chrome/browser/sync/engine/process_commit_response_command.h
index b1ba659..e83d075 100644
--- a/chrome/browser/sync/engine/process_commit_response_command.h
+++ b/chrome/browser/sync/engine/process_commit_response_command.h
@@ -26,6 +26,7 @@ class ProcessCommitResponseCommand : public ModelChangingSyncerCommand {
   virtual ~ProcessCommitResponseCommand();
 
   // ModelChangingSyncerCommand implementation.
+  virtual void ModelNeutralExecuteImpl(sessions::SyncSession* session);
   virtual void ModelChangingExecuteImpl(sessions::SyncSession* session);
 
  private:
diff --git a/chrome/browser/sync/engine/process_commit_response_command_unittest.cc b/chrome/browser/sync/engine/process_commit_response_command_unittest.cc
index c8b5d14..70c6732 100755
--- a/chrome/browser/sync/engine/process_commit_response_command_unittest.cc
+++ b/chrome/browser/sync/engine/process_commit_response_command_unittest.cc
@@ -5,6 +5,7 @@
 #include <vector>
 
 #include "base/string_util.h"
+#include "chrome/browser/sync/engine/mock_model_safe_workers.h"
 #include "chrome/browser/sync/engine/process_commit_response_command.h"
 #include "chrome/browser/sync/protocol/bookmark_specifics.pb.h"
 #include "chrome/browser/sync/protocol/sync.pb.h"
@@ -35,6 +36,20 @@ using syncable::WriteTransaction;
 // A test fixture for tests exercising ProcessCommitResponseCommand.
 class ProcessCommitResponseCommandTest : public SyncerCommandTest {
  public:
+  virtual void SetUp() {
+    workers()->clear();
+    mutable_routing_info()->clear();
+
+    workers()->push_back(new ModelSafeWorker());    // GROUP_PASSIVE worker.
+    workers()->push_back(new MockUIModelWorker());  // GROUP_UI worker.
+    (*mutable_routing_info())[syncable::BOOKMARKS] = GROUP_UI;
+    (*mutable_routing_info())[syncable::PREFERENCES] = GROUP_UI;
+    (*mutable_routing_info())[syncable::AUTOFILL] = GROUP_PASSIVE;
+
+    commit_set_.reset(new sessions::OrderedCommitSet(routing_info()));
+    SyncerCommandTest::SetUp();
+  }
+
  protected:
   ProcessCommitResponseCommandTest()
       : next_old_revision_(1),
@@ -42,13 +57,26 @@ class ProcessCommitResponseCommandTest : public SyncerCommandTest {
         next_server_position_(10000) {
   }
 
+  void CheckEntry(Entry* e, const std::string& name,
+                  syncable::ModelType model_type, const Id& parent_id) {
+     EXPECT_TRUE(e->good());
+     ASSERT_EQ(name, e->Get(NON_UNIQUE_NAME));
+     ASSERT_EQ(model_type, e->GetModelType());
+     ASSERT_EQ(parent_id, e->Get(syncable::PARENT_ID));
+     ASSERT_LT(0, e->Get(BASE_VERSION))
+         << "Item should have a valid (positive) server base revision";
+  }
+
   // Create an unsynced item in the database.  If item_id is a local ID, it
   // will be treated as a create-new.  Otherwise, if it's a server ID, we'll
   // fake the server data so that it looks like it exists on the server.
+  // Returns the methandle of the created item in |metahandle_out| if not NULL.
   void CreateUnsyncedItem(const Id& item_id,
                           const Id& parent_id,
                           const string& name,
-                          bool is_folder) {
+                          bool is_folder,
+                          syncable::ModelType model_type,
+                          int64* metahandle_out) {
     ScopedDirLookup dir(syncdb().manager(), syncdb().name());
     ASSERT_TRUE(dir.good());
     WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__);
@@ -63,15 +91,17 @@ class ProcessCommitResponseCommandTest : public SyncerCommandTest {
     entry.Put(syncable::IS_DEL, false);
     entry.Put(syncable::PARENT_ID, parent_id);
     entry.PutPredecessor(predecessor_id);
-    sync_pb::EntitySpecifics default_bookmark_specifics;
-    default_bookmark_specifics.MutableExtension(sync_pb::bookmark);
-    entry.Put(syncable::SPECIFICS, default_bookmark_specifics);
+    sync_pb::EntitySpecifics default_specifics;
+    syncable::AddDefaultExtensionValue(model_type, &default_specifics);
+    entry.Put(syncable::SPECIFICS, default_specifics);
     if (item_id.ServerKnows()) {
-      entry.Put(syncable::SERVER_SPECIFICS, default_bookmark_specifics);
+      entry.Put(syncable::SERVER_SPECIFICS, default_specifics);
       entry.Put(syncable::SERVER_IS_DIR, is_folder);
       entry.Put(syncable::SERVER_PARENT_ID, parent_id);
       entry.Put(syncable::SERVER_IS_DEL, false);
     }
+    if (metahandle_out)
+      *metahandle_out = entry.Get(syncable::META_HANDLE);
   }
 
   // Create a new unsynced item in the database, and synthesize a commit
@@ -80,16 +110,18 @@ class ProcessCommitResponseCommandTest : public SyncerCommandTest {
   // will be an edit.
   void CreateUnprocessedCommitResult(const Id& item_id,
                                      const Id& parent_id,
-                                     const string& name) {
+                                     const string& name,
+                                     syncable::ModelType model_type) {
     sessions::StatusController* sync_state = session()->status_controller();
     bool is_folder = true;
-    CreateUnsyncedItem(item_id, parent_id, name, is_folder);
+    int64 metahandle = 0;
+    CreateUnsyncedItem(item_id, parent_id, name, is_folder, model_type,
+                       &metahandle);
 
     // ProcessCommitResponseCommand consumes commit_ids from the session
     // state, so we need to update that.  O(n^2) because it's a test.
-    std::vector<Id> commit_ids = sync_state->commit_ids();
-    commit_ids.push_back(item_id);
-    sync_state->set_commit_ids(commit_ids);
+    commit_set_->AddCommitItem(metahandle, item_id, model_type);
+    sync_state->set_commit_set(*commit_set_.get());
 
     ScopedDirLookup dir(syncdb().manager(), syncdb().name());
     ASSERT_TRUE(dir.good());
@@ -142,7 +174,7 @@ class ProcessCommitResponseCommandTest : public SyncerCommandTest {
 
   ProcessCommitResponseCommand command_;
   TestIdFactory id_factory_;
-
+  scoped_ptr<sessions::OrderedCommitSet> commit_set_;
  private:
   int64 next_old_revision_;
   int64 next_new_revision_;
@@ -150,6 +182,66 @@ class ProcessCommitResponseCommandTest : public SyncerCommandTest {
   DISALLOW_COPY_AND_ASSIGN(ProcessCommitResponseCommandTest);
 };
 
+TEST_F(ProcessCommitResponseCommandTest, MultipleCommitIdProjections) {
+  Id bookmark_folder_id = id_factory_.NewLocalId();
+  Id bookmark_id1 = id_factory_.NewLocalId();
+  Id bookmark_id2 = id_factory_.NewLocalId();
+  Id pref_id1 = id_factory_.NewLocalId(), pref_id2 = id_factory_.NewLocalId();
+  Id autofill_id1 = id_factory_.NewLocalId();
+  Id autofill_id2 = id_factory_.NewLocalId();
+  CreateUnprocessedCommitResult(bookmark_folder_id, id_factory_.root(),
+                                "A bookmark folder", syncable::BOOKMARKS);
+  CreateUnprocessedCommitResult(bookmark_id1, bookmark_folder_id,
+                                "bookmark 1", syncable::BOOKMARKS);
+  CreateUnprocessedCommitResult(bookmark_id2, bookmark_folder_id,
+                                "bookmark 2", syncable::BOOKMARKS);
+  CreateUnprocessedCommitResult(pref_id1, id_factory_.root(),
+                                "Pref 1", syncable::PREFERENCES);
+  CreateUnprocessedCommitResult(pref_id2, id_factory_.root(),
+                                "Pref 2", syncable::PREFERENCES);
+  CreateUnprocessedCommitResult(autofill_id1, id_factory_.root(),
+                                "Autofill 1", syncable::AUTOFILL);
+  CreateUnprocessedCommitResult(autofill_id2, id_factory_.root(),
+                                "Autofill 2", syncable::AUTOFILL);
+
+  command_.ExecuteImpl(session());
+
+  ScopedDirLookup dir(syncdb().manager(), syncdb().name());
+  ASSERT_TRUE(dir.good());
+  ReadTransaction trans(dir, __FILE__, __LINE__);
+  Id new_fid = dir->GetFirstChildId(&trans, id_factory_.root());
+  ASSERT_FALSE(new_fid.IsRoot());
+  EXPECT_TRUE(new_fid.ServerKnows());
+  EXPECT_FALSE(bookmark_folder_id.ServerKnows());
+  EXPECT_FALSE(new_fid == bookmark_folder_id);
+  Entry b_folder(&trans, syncable::GET_BY_ID, new_fid);
+  ASSERT_TRUE(b_folder.good());
+  ASSERT_EQ("A bookmark folder", b_folder.Get(NON_UNIQUE_NAME))
+      << "Name of bookmark folder should not change.";
+  ASSERT_LT(0, b_folder.Get(BASE_VERSION))
+      << "Bookmark folder should have a valid (positive) server base revision";
+
+  // Look at the two bookmarks in bookmark_folder.
+  Id cid = dir->GetFirstChildId(&trans, new_fid);
+  Entry b1(&trans, syncable::GET_BY_ID, cid);
+  Entry b2(&trans, syncable::GET_BY_ID, b1.Get(syncable::NEXT_ID));
+  CheckEntry(&b1, "bookmark 1", syncable::BOOKMARKS, new_fid);
+  CheckEntry(&b2, "bookmark 2", syncable::BOOKMARKS, new_fid);
+  ASSERT_TRUE(b2.Get(syncable::NEXT_ID).IsRoot());
+
+  // Look at the prefs and autofill items.
+  Entry p1(&trans, syncable::GET_BY_ID, b_folder.Get(syncable::NEXT_ID));
+  Entry p2(&trans, syncable::GET_BY_ID, p1.Get(syncable::NEXT_ID));
+  CheckEntry(&p1, "Pref 1", syncable::PREFERENCES, id_factory_.root());
+  CheckEntry(&p2, "Pref 2", syncable::PREFERENCES, id_factory_.root());
+
+  Entry a1(&trans, syncable::GET_BY_ID, p2.Get(syncable::NEXT_ID));
+  Entry a2(&trans, syncable::GET_BY_ID, a1.Get(syncable::NEXT_ID));
+  CheckEntry(&a1, "Autofill 1", syncable::AUTOFILL, id_factory_.root());
+  CheckEntry(&a2, "Autofill 2", syncable::AUTOFILL, id_factory_.root());
+  ASSERT_TRUE(a2.Get(syncable::NEXT_ID).IsRoot());
+}
+
 // In this test, we test processing a commit response for a commit batch that
 // includes a newly created folder and some (but not all) of its children.
 // In particular, the folder has 50 children, which alternate between being
@@ -163,7 +255,8 @@ class ProcessCommitResponseCommandTest : public SyncerCommandTest {
 TEST_F(ProcessCommitResponseCommandTest, NewFolderCommitKeepsChildOrder) {
   // Create the parent folder, a new item whose ID will change on commit.
   Id folder_id = id_factory_.NewLocalId();
-  CreateUnprocessedCommitResult(folder_id, id_factory_.root(), "A");
+  CreateUnprocessedCommitResult(folder_id, id_factory_.root(), "A",
+                                syncable::BOOKMARKS);
 
   // Verify that the item is reachable.
   {
@@ -180,7 +273,8 @@ TEST_F(ProcessCommitResponseCommandTest, NewFolderCommitKeepsChildOrder) {
   for (; i < batch_size; ++i) {
     // Alternate between new and old child items, just for kicks.
     Id id = (i % 4 < 2) ? id_factory_.NewLocalId() : id_factory_.NewServerId();
-    CreateUnprocessedCommitResult(id, folder_id, StringPrintf("Item %d", i));
+    CreateUnprocessedCommitResult(id, folder_id, StringPrintf("Item %d", i),
+                                  syncable::BOOKMARKS);
   }
   // The second 25 children will be unsynced items but NOT part of the commit
   // batch.  When the ID of the parent folder changes during the commit,
@@ -189,14 +283,15 @@ TEST_F(ProcessCommitResponseCommandTest, NewFolderCommitKeepsChildOrder) {
   for (; i < 2*batch_size; ++i) {
     // Alternate between new and old child items, just for kicks.
     Id id = (i % 4 < 2) ? id_factory_.NewLocalId() : id_factory_.NewServerId();
-    CreateUnsyncedItem(id, folder_id, StringPrintf("Item %d", i), false);
+    CreateUnsyncedItem(id, folder_id, StringPrintf("Item %d", i), false,
+                       syncable::BOOKMARKS, NULL);
   }
 
   // Process the commit response for the parent folder and the first
   // 25 items.  This should apply the values indicated by
   // each CommitResponse_EntryResponse to the syncable Entries.  All new
   // items in the commit batch should have their IDs changed to server IDs.
-  command_.ModelChangingExecuteImpl(session());
+  command_.ExecuteImpl(session());
 
   ScopedDirLookup dir(syncdb().manager(), syncdb().name());
   ASSERT_TRUE(dir.good());
diff --git a/chrome/browser/sync/engine/process_updates_command.cc b/chrome/browser/sync/engine/process_updates_command.cc
index a54481f..f17a05d 100755
--- a/chrome/browser/sync/engine/process_updates_command.cc
+++ b/chrome/browser/sync/engine/process_updates_command.cc
@@ -107,7 +107,6 @@ void ProcessUpdatesCommand::ModelChangingExecuteImpl(SyncSession* session) {
     status->set_timestamp_dirty(true);
   }
 
-  status->set_num_consecutive_problem_get_updates(0);
   status->set_num_consecutive_errors(0);
   status->set_current_sync_timestamp(dir->last_sync_timestamp());
   status->set_syncing(true);
diff --git a/chrome/browser/sync/engine/syncer.cc b/chrome/browser/sync/engine/syncer.cc
index 8316390..9170a46 100755
--- a/chrome/browser/sync/engine/syncer.cc
+++ b/chrome/browser/sync/engine/syncer.cc
@@ -81,7 +81,6 @@ bool Syncer::SyncShare(sessions::SyncSession::Delegate* delegate) {
 }
 
 bool Syncer::SyncShare(sessions::SyncSession* session) {
-  session->status_controller()->ResetTransientState();
   session->set_source(TestAndSetUpdatesSource());
   // This isn't perfect, as we can end up bundling extensions activity
   // intended for the next session into the current one.  We could do a
@@ -197,6 +196,7 @@ void Syncer::SyncShare(sessions::SyncSession* session,
       }
       case PROCESS_COMMIT_RESPONSE: {
         LOG(INFO) << "Processing the commit response";
+        session->status_controller()->reset_num_conflicting_commits();
         ProcessCommitResponseCommand process_response_command;
         process_response_command.Execute(session);
         next_step = BUILD_AND_PROCESS_CONFLICT_SETS;
diff --git a/chrome/browser/sync/engine/syncer_unittest.cc b/chrome/browser/sync/engine/syncer_unittest.cc
index 5c9bea9..7805dae 100755
--- a/chrome/browser/sync/engine/syncer_unittest.cc
+++ b/chrome/browser/sync/engine/syncer_unittest.cc
@@ -345,31 +345,30 @@ class SyncerTest : public testing::Test,
     // The expected order is "x", "b", "c", "e", truncated appropriately.
     for (size_t limit = expected_id_order.size() + 2; limit > 0; --limit) {
       StatusController* status = session_->status_controller();
-      status->ResetTransientState();
       WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__);
       ScopedSetSessionWriteTransaction set_trans(session_.get(), &wtrans);
       status->set_unsynced_handles(unsynced_handle_view);
 
-      GetCommitIdsCommand command(limit);
       ModelSafeRoutingInfo routes;
       GetModelSafeRoutingInfo(&routes);
+      GetCommitIdsCommand command(limit);
       command.BuildCommitIds(session_->status_controller()->unsynced_handles(),
           session_->write_transaction(), routes);
       vector<syncable::Id> output =
-          command.ordered_commit_set_.GetAllCommitIds();
+          command.ordered_commit_set_->GetAllCommitIds();
       size_t truncated_size = std::min(limit, expected_id_order.size());
       ASSERT_TRUE(truncated_size == output.size());
       for (size_t i = 0; i < truncated_size; ++i) {
         ASSERT_TRUE(expected_id_order[i] == output[i])
             << "At index " << i << " with batch size limited to " << limit;
       }
-      GetCommitIdsCommand::OrderedCommitSet::Projection proj;
-      proj = command.ordered_commit_set_.GetCommitIdProjection(GROUP_PASSIVE);
+      sessions::OrderedCommitSet::Projection proj;
+      proj = command.ordered_commit_set_->GetCommitIdProjection(GROUP_PASSIVE);
       ASSERT_EQ(truncated_size, proj.size());
       for (size_t i = 0; i < truncated_size; ++i) {
         SCOPED_TRACE(::testing::Message("Projection mismatch with i = ") << i);
         syncable::Id projected =
-            command.ordered_commit_set_.GetCommitIdAt(proj[i]);
+            command.ordered_commit_set_->GetCommitIdAt(proj[i]);
         ASSERT_TRUE(expected_id_order[proj[i]] == projected);
         // Since this projection is the identity, the following holds.
         ASSERT_TRUE(expected_id_order[i] == projected);
@@ -503,7 +502,7 @@ TEST_F(SyncerTest, TestCommitMetahandleIterator) {
     WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__);
     ScopedSetSessionWriteTransaction set_trans(session_.get(), &wtrans);
 
-    GetCommitIdsCommand::OrderedCommitSet commit_set;
+    sessions::OrderedCommitSet commit_set(session_->routing_info());
     GetCommitIdsCommand::CommitMetahandleIterator iterator(unsynced, &wtrans,
         &commit_set);
     EXPECT_FALSE(iterator.Valid());
@@ -519,7 +518,7 @@ TEST_F(SyncerTest, TestCommitMetahandleIterator) {
 
     WriteTransaction wtrans(dir, UNITTEST, __FILE__, __LINE__);
     ScopedSetSessionWriteTransaction set_trans(session_.get(), &wtrans);
-    GetCommitIdsCommand::OrderedCommitSet commit_set;
+    sessions::OrderedCommitSet commit_set(session_->routing_info());
     GetCommitIdsCommand::CommitMetahandleIterator iterator(unsynced, &wtrans,
         &commit_set);
 
@@ -539,75 +538,6 @@ TEST_F(SyncerTest, TestCommitMetahandleIterator) {
   }
 }
 
-TEST_F(SyncerTest, OrderedCommitSetProjections) {
-  vector<syncable::Id> expected;
-  for (int i = 0; i < 8; i++)
-    expected.push_back(ids_.NewLocalId());
-
-  GetCommitIdsCommand::OrderedCommitSet commit_set1, commit_set2;
-  commit_set1.AddCommitItem(0, expected[0], GROUP_UI);
-  commit_set1.AddCommitItem(1, expected[1], GROUP_UI);
-  commit_set1.AddCommitItem(2, expected[2], GROUP_UI);
-  commit_set1.AddCommitItem(3, expected[3], GROUP_PASSIVE);
-  commit_set1.AddCommitItem(4, expected[4], GROUP_PASSIVE);
-  commit_set2.AddCommitItem(7, expected[7], GROUP_DB);
-  commit_set2.AddCommitItem(6, expected[6], GROUP_DB);
-  commit_set2.AddCommitItem(5, expected[5], GROUP_DB);
-  commit_set1.AppendReverse(commit_set2);
-
-  // First, we should verify the projections are correct. Second, we want to
-  // do the same verification after truncating by 1. Next, try truncating
-  // the set to a size of 4, so that the DB projection is wiped out and
-  // PASSIVE has one element removed.  Finally, truncate to 1 so only UI is
-  // remaining.
-  int j = 0;
-  do {
-    SCOPED_TRACE(::testing::Message("Iteration j = ") << j);
-    vector<syncable::Id> all_ids = commit_set1.GetAllCommitIds();
-    EXPECT_EQ(expected.size(), all_ids.size());
-    for (size_t i = 0; i < expected.size(); i++) {
-      SCOPED_TRACE(::testing::Message("CommitSet mismatch at iteration i = ")
-                   << i);
-      EXPECT_TRUE(expected[i] == all_ids[i]);
-      EXPECT_TRUE(expected[i] == commit_set1.GetCommitIdAt(i));
-    }
-
-    GetCommitIdsCommand::OrderedCommitSet::Projection p1, p2, p3;
-    p1 = commit_set1.GetCommitIdProjection(GROUP_UI);
-    p2 = commit_set1.GetCommitIdProjection(GROUP_PASSIVE);
-    p3 = commit_set1.GetCommitIdProjection(GROUP_DB);
-    EXPECT_TRUE(p1.size() + p2.size() + p3.size() == expected.size()) << "Sum"
-        << "of sizes of projections should equal full expected size!";
-
-    for (size_t i = 0; i < p1.size(); i++) {
-      SCOPED_TRACE(::testing::Message("UI projection mismatch at i = ") << i);
-      EXPECT_TRUE(expected[p1[i]] == commit_set1.GetCommitIdAt(p1[i]))
-          << "expected[p1[i]] = " << expected[p1[i]]
-          << ", commit_set1[p1[i]] = " << commit_set1.GetCommitIdAt(p1[i]);
-    }
-    for (size_t i = 0; i < p2.size(); i++) {
-      SCOPED_TRACE(::testing::Message("PASSIVE projection mismatch at i = ")
-                   << i);
-      EXPECT_TRUE(expected[p2[i]] == commit_set1.GetCommitIdAt(p2[i]))
-          << "expected[p2[i]] = " << expected[p2[i]]
-          << ", commit_set1[p2[i]] = " << commit_set1.GetCommitIdAt(p2[i]);
-    }
-    for (size_t i = 0; i < p3.size(); i++) {
-      SCOPED_TRACE(::testing::Message("DB projection mismatch at i = ") << i);
-      EXPECT_TRUE(expected[p3[i]] == commit_set1.GetCommitIdAt(p3[i]))
-          << "expected[p3[i]] = " << expected[p3[i]]
-          << ", commit_set1[p3[i]] = " << commit_set1.GetCommitIdAt(p3[i]);
-    }
-
-    int cut_to_size = 7 - 3 * j++;
-    if (cut_to_size < 0)
-      break;
-
-    expected.resize(cut_to_size);
-    commit_set1.Truncate(cut_to_size);
-  } while (true);
-}
-
 TEST_F(SyncerTest, TestGetUnsyncedAndSimpleCommit) {
   ScopedDirLookup dir(syncdb_.manager(), syncdb_.name());
   ASSERT_TRUE(dir.good());
diff --git a/chrome/browser/sync/sessions/ordered_commit_set.cc b/chrome/browser/sync/sessions/ordered_commit_set.cc
new file mode 100644
index 0000000..5c46a35
--- /dev/null
+++ b/chrome/browser/sync/sessions/ordered_commit_set.cc
@@ -0,0 +1,94 @@
+// Copyright (c) 2010 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/sessions/ordered_commit_set.h"
+
+#include "base/logging.h"
+
+namespace browser_sync {
+namespace sessions {
+
+void OrderedCommitSet::AddCommitItem(const int64 metahandle,
+                                     const syncable::Id& commit_id,
+                                     syncable::ModelType type) {
+  if (!HaveCommitItem(metahandle)) {
+    inserted_metahandles_.insert(metahandle);
+    metahandle_order_.push_back(metahandle);
+    commit_ids_.push_back(commit_id);
+    projections_[GetGroupForModelType(type, routes_)].push_back(
+        commit_ids_.size() - 1);
+    types_.push_back(type);
+  }
+}
+
+void OrderedCommitSet::AppendReverse(const OrderedCommitSet& other) {
+  for (int i = other.Size() - 1; i >= 0; i--) {
+    CommitItem item = other.GetCommitItemAt(i);
+    AddCommitItem(item.meta, item.id, item.group);
+  }
+}
+
+void OrderedCommitSet::Truncate(size_t max_size) {
+  if (max_size < metahandle_order_.size()) {
+    for (size_t i = max_size; i < metahandle_order_.size(); ++i) {
+      inserted_metahandles_.erase(metahandle_order_[i]);
+    }
+
+    // Some projections may refer to indices that are getting chopped.
+    // Since projections are in increasing order, it's easy to fix. Except
+    // that you can't erase(..) using a reverse_iterator, so we use binary
+    // search to find the chop point.
+    Projections::iterator it = projections_.begin();
+    for (; it != projections_.end(); ++it) {
+      // For each projection, chop off any indices larger than or equal to
+      // max_size by looking for max_size using binary search.
+      Projection& p = it->second;
+      Projection::iterator element = std::lower_bound(p.begin(), p.end(),
+        max_size);
+      if (element != p.end())
+        p.erase(element, p.end());
+    }
+    commit_ids_.resize(max_size);
+    metahandle_order_.resize(max_size);
+    types_.resize(max_size);
+  }
+}
+
+OrderedCommitSet::CommitItem OrderedCommitSet::GetCommitItemAt(
+    const int position) const {
+  DCHECK(position < Size());
+  CommitItem return_item = {metahandle_order_[position],
+      commit_ids_[position],
+      types_[position]};
+  return return_item;
+}
+
+bool OrderedCommitSet::HasBookmarkCommitId() const {
+  ModelSafeRoutingInfo::const_iterator group
+      = routes_.find(syncable::BOOKMARKS);
+  if (group == routes_.end())
+    return false;
+  Projections::const_iterator proj = projections_.find(group->second);
+  if (proj == projections_.end())
+    return false;
+  DCHECK_LE(proj->second.size(), types_.size());
+  for (size_t i = 0; i < proj->second.size(); i++) {
+    if (types_[proj->second[i]] == syncable::BOOKMARKS)
+      return true;
+  }
+  return false;
+}
+
+void OrderedCommitSet::operator=(const OrderedCommitSet& other) {
+  inserted_metahandles_ = other.inserted_metahandles_;
+  commit_ids_ = other.commit_ids_;
+  metahandle_order_ = other.metahandle_order_;
+  projections_ = other.projections_;
+  types_ = other.types_;
+  routes_ = other.routes_;
+}
+
+}  // namespace sessions
+}  // namespace browser_sync
+
diff --git a/chrome/browser/sync/sessions/ordered_commit_set.h b/chrome/browser/sync/sessions/ordered_commit_set.h
new file mode 100644
index 0000000..144fba1
--- /dev/null
+++ b/chrome/browser/sync/sessions/ordered_commit_set.h
@@ -0,0 +1,119 @@
+// Copyright (c) 2010 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.
+
+#ifndef CHROME_BROWSER_SYNC_SESSIONS_ORDERED_COMMIT_SET_H_
+#define CHROME_BROWSER_SYNC_SESSIONS_ORDERED_COMMIT_SET_H_
+
+#include <map>
+#include <set>
+#include <vector>
+
+#include "chrome/browser/sync/engine/model_safe_worker.h"
+#include "chrome/browser/sync/syncable/model_type.h"
+#include "chrome/browser/sync/syncable/syncable_id.h"
+
+namespace browser_sync {
+namespace sessions {
+
+// TODO(ncarter): This code is more generic than just Commit and can
+// be reused elsewhere (e.g. ChangeReorderBuffer do similar things).  Merge
+// all these implementations.
+class OrderedCommitSet {
+ public:
+  // A list of indices into the full list of commit ids such that:
+  // 1 - each element is an index belonging to a particular ModelSafeGroup.
+  // 2 - the vector is in sorted (smallest to largest) order.
+  // 3 - each element is a valid index for GetCommitItemAt.
+  // See GetCommitIdProjection for usage.
+  typedef std::vector<size_t> Projection;
+
+  // TODO(chron): Reserve space according to batch size?
+  explicit OrderedCommitSet(const browser_sync::ModelSafeRoutingInfo& routes)
+      : routes_(routes) {}
+
+  ~OrderedCommitSet() {}
+
+  bool HaveCommitItem(const int64 metahandle) const {
+    return inserted_metahandles_.count(metahandle) > 0;
+  }
+
+  void AddCommitItem(const int64 metahandle, const syncable::Id& commit_id,
+                     syncable::ModelType type);
+
+  const std::vector<syncable::Id>& GetAllCommitIds() const {
+    return commit_ids_;
+  }
+
+  // Return the Id at index |position| in this OrderedCommitSet.  Note that
+  // the index uniquely identifies the same logical item in each of:
+  // 1) this OrderedCommitSet
+  // 2) the CommitRequest sent to the server
+  // 3) the list of EntryResponse objects in the CommitResponse.
+  // These together allow re-association of the pre-commit Id with the
+  // actual committed entry.
+  const syncable::Id& GetCommitIdAt(const size_t position) const {
+    return commit_ids_[position];
+  }
+
+  // Same as above, but for ModelType of the item.
+  const syncable::ModelType GetModelTypeAt(const size_t position) const {
+    return types_[position];
+  }
+
+  // Get the projection of commit ids onto the space of commit ids
+  // belonging to |group|.  This is useful when you need to process a commit
+  // response one ModelSafeGroup at a time. See GetCommitIdAt for how the
+  // indices contained in the returned Projection can be used.
+  const Projection& GetCommitIdProjection(browser_sync::ModelSafeGroup group) {
+    return projections_[group];
+  }
+
+  int Size() const {
+    return commit_ids_.size();
+  }
+
+  // Returns true iff any of the commit ids added to this set have model type
+  // BOOKMARKS.
+  bool HasBookmarkCommitId() const;
+
+  void AppendReverse(const OrderedCommitSet& other);
+  void Truncate(size_t max_size);
+
+  void operator=(const OrderedCommitSet& other);
+ private:
+  // A set of CommitIdProjections associated with particular ModelSafeGroups.
+  typedef std::map<browser_sync::ModelSafeGroup, Projection> Projections;
+
+  // Helper container for return value of GetCommitItemAt.
+  struct CommitItem {
+    int64 meta;
+    syncable::Id id;
+    syncable::ModelType group;
+  };
+
+  CommitItem GetCommitItemAt(const int position) const;
+
+  // These lists are different views of the same items; e.g they are
+  // isomorphic.
+  std::set<int64> inserted_metahandles_;
+  std::vector<syncable::Id> commit_ids_;
+  std::vector<int64> metahandle_order_;
+  Projections projections_;
+
+  // We need this because of operations like AppendReverse that take ids from
+  // one OrderedCommitSet and insert into another -- we need to know the
+  // group for each ID so that the insertion can update the appropriate
+  // projection.  We could store it in commit_ids_, but sometimes we want
+  // to just return the vector of Ids, so this is more straightforward
+  // and shouldn't take up too much extra space since commit lists are small.
+  std::vector<syncable::ModelType> types_;
+
+  browser_sync::ModelSafeRoutingInfo routes_;
+};
+
+}  // namespace sessions
+}  // namespace browser_sync
+
+#endif  // CHROME_BROWSER_SYNC_SESSIONS_ORDERED_COMMIT_SET_H_
+
diff --git a/chrome/browser/sync/sessions/ordered_commit_set_unittest.cc b/chrome/browser/sync/sessions/ordered_commit_set_unittest.cc
new file mode 100644
index 0000000..0c9db8e
--- /dev/null
+++ b/chrome/browser/sync/sessions/ordered_commit_set_unittest.cc
@@ -0,0 +1,120 @@
+// Copyright (c) 2010 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 "testing/gtest/include/gtest/gtest.h"
+#include "chrome/browser/sync/sessions/ordered_commit_set.h"
+#include "chrome/test/sync/engine/test_id_factory.h"
+
+using std::vector;
+
+class OrderedCommitSetTest : public testing::Test {
+ public:
+  OrderedCommitSetTest() {
+    routes_[syncable::BOOKMARKS] = browser_sync::GROUP_UI;
+    routes_[syncable::PREFERENCES] = browser_sync::GROUP_UI;
+    routes_[syncable::AUTOFILL] = browser_sync::GROUP_DB;
+    routes_[syncable::TOP_LEVEL_FOLDER] = browser_sync::GROUP_PASSIVE;
+  }
+ protected:
+  browser_sync::TestIdFactory ids_;
+  browser_sync::ModelSafeRoutingInfo routes_;
+};
+
+namespace browser_sync {
+namespace sessions {
+
+TEST_F(OrderedCommitSetTest, Projections) {
+  vector<syncable::Id> expected;
+  for (int i = 0; i < 8; i++)
+    expected.push_back(ids_.NewLocalId());
+
+  OrderedCommitSet commit_set1(routes_), commit_set2(routes_);
+  commit_set1.AddCommitItem(0, expected[0], syncable::BOOKMARKS);
+  commit_set1.AddCommitItem(1, expected[1], syncable::BOOKMARKS);
+  commit_set1.AddCommitItem(2, expected[2], syncable::PREFERENCES);
+  // Duplicates should be dropped.
+  commit_set1.AddCommitItem(2, expected[2], syncable::PREFERENCES);
+  commit_set1.AddCommitItem(3, expected[3], syncable::TOP_LEVEL_FOLDER);
+  commit_set1.AddCommitItem(4, expected[4], syncable::TOP_LEVEL_FOLDER);
+  commit_set2.AddCommitItem(7, expected[7], syncable::AUTOFILL);
+  commit_set2.AddCommitItem(6, expected[6], syncable::AUTOFILL);
+  commit_set2.AddCommitItem(5, expected[5], syncable::AUTOFILL);
+  // Add something in set1 to set2, which should get dropped by AppendReverse.
+  commit_set2.AddCommitItem(0, expected[0], syncable::BOOKMARKS);
+  commit_set1.AppendReverse(commit_set2);
+
+  // First, we should verify the projections are correct. Second, we want to
+  // do the same verification after truncating by 1. Next, try truncating
+  // the set to a size of 4, so that the DB projection is wiped out and
+  // PASSIVE has one element removed.  Finally, truncate to 1 so only UI is
+  // remaining.
+  int j = 0;
+  do {
+    SCOPED_TRACE(::testing::Message("Iteration j = ") << j);
+    vector<syncable::Id> all_ids = commit_set1.GetAllCommitIds();
+    EXPECT_EQ(expected.size(), all_ids.size());
+    for (size_t i = 0; i < expected.size(); i++) {
+      SCOPED_TRACE(::testing::Message("CommitSet mismatch at iteration i = ")
+                   << i);
+      EXPECT_TRUE(expected[i] == all_ids[i]);
+      EXPECT_TRUE(expected[i] == commit_set1.GetCommitIdAt(i));
+    }
+
+    OrderedCommitSet::Projection p1, p2, p3;
+    p1 = commit_set1.GetCommitIdProjection(GROUP_UI);
+    p2 = commit_set1.GetCommitIdProjection(GROUP_PASSIVE);
+    p3 = commit_set1.GetCommitIdProjection(GROUP_DB);
+    EXPECT_TRUE(p1.size() + p2.size() + p3.size() == expected.size()) << "Sum"
+        << "of sizes of projections should equal full expected size!";
+
+    for (size_t i = 0; i < p1.size(); i++) {
+      SCOPED_TRACE(::testing::Message("UI projection mismatch at i = ") << i);
+      EXPECT_TRUE(expected[p1[i]] == commit_set1.GetCommitIdAt(p1[i]))
+          << "expected[p1[i]] = " << expected[p1[i]]
+          << ", commit_set1[p1[i]] = " << commit_set1.GetCommitIdAt(p1[i]);
+    }
+    for (size_t i = 0; i < p2.size(); i++) {
+      SCOPED_TRACE(::testing::Message("PASSIVE projection mismatch at i = ")
+                   << i);
+      EXPECT_TRUE(expected[p2[i]] == commit_set1.GetCommitIdAt(p2[i]))
+          << "expected[p2[i]] = " << expected[p2[i]]
+          << ", commit_set1[p2[i]] = " << commit_set1.GetCommitIdAt(p2[i]);
+    }
+    for (size_t i = 0; i < p3.size(); i++) {
+      SCOPED_TRACE(::testing::Message("DB projection mismatch at i = ") << i);
+      EXPECT_TRUE(expected[p3[i]] == commit_set1.GetCommitIdAt(p3[i]))
+          << "expected[p3[i]] = " << expected[p3[i]]
+          << ", commit_set1[p3[i]] = " << commit_set1.GetCommitIdAt(p3[i]);
+    }
+
+    int cut_to_size = 7 - 3 * j++;
+    if (cut_to_size < 0)
+      break;
+
+    expected.resize(cut_to_size);
+    commit_set1.Truncate(cut_to_size);
+  } while (true);
+}
+
+TEST_F(OrderedCommitSetTest, HasBookmarkCommitId) {
+  OrderedCommitSet commit_set(routes_);
+
+  commit_set.AddCommitItem(0, ids_.NewLocalId(), syncable::AUTOFILL);
+  commit_set.AddCommitItem(1, ids_.NewLocalId(), syncable::TOP_LEVEL_FOLDER);
+  EXPECT_FALSE(commit_set.HasBookmarkCommitId());
+
+  commit_set.AddCommitItem(2, ids_.NewLocalId(), syncable::PREFERENCES);
+  commit_set.AddCommitItem(3, ids_.NewLocalId(), syncable::PREFERENCES);
+  EXPECT_FALSE(commit_set.HasBookmarkCommitId());
+
+  commit_set.AddCommitItem(4, ids_.NewLocalId(), syncable::BOOKMARKS);
+  EXPECT_TRUE(commit_set.HasBookmarkCommitId());
+
+  commit_set.Truncate(4);
+  EXPECT_FALSE(commit_set.HasBookmarkCommitId());
+}
+
+}  // namespace sessions
+}  // namespace browser_sync
+
diff --git a/chrome/browser/sync/sessions/session_state.h b/chrome/browser/sync/sessions/session_state.h
index 6079259..e4e479c 100644
--- a/chrome/browser/sync/sessions/session_state.h
+++ b/chrome/browser/sync/sessions/session_state.h
@@ -17,6 +17,7 @@
 #include "base/basictypes.h"
 #include "chrome/browser/sync/engine/syncer_types.h"
 #include "chrome/browser/sync/engine/syncproto.h"
+#include "chrome/browser/sync/sessions/ordered_commit_set.h"
 
 namespace syncable {
 class DirectoryManager;
@@ -47,23 +48,16 @@ struct SyncerStatus {
   bool syncer_stuck;
   bool syncing;
   int num_successful_commits;
+  // This is needed for monitoring extensions activity.
+  int num_successful_bookmark_commits;
 };
 
 // Counters for various errors that can occur repeatedly during a sync session.
 struct ErrorCounters {
   ErrorCounters() : num_conflicting_commits(0),
-                    consecutive_problem_get_updates(0),
-                    consecutive_problem_commits(0),
                     consecutive_transient_error_commits(0),
                     consecutive_errors(0) {}
   int num_conflicting_commits;
-  // Is reset when we get any updates (not on pings) and increments whenever
-  // the request fails.
-  int consecutive_problem_get_updates;
-
-  // Consecutive_problem_commits_ resets whenever we commit any number of items
-  // and increments whenever all commits fail for any reason.
-  int consecutive_problem_commits;
 
   // Number of commits hitting transient errors since the last successful
   // commit.
diff --git a/chrome/browser/sync/sessions/status_controller.cc b/chrome/browser/sync/sessions/status_controller.cc
index eefead2..921bde7 100644
--- a/chrome/browser/sync/sessions/status_controller.cc
+++ b/chrome/browser/sync/sessions/status_controller.cc
@@ -10,12 +10,10 @@ namespace browser_sync {
 namespace sessions {
 
 StatusController::StatusController()
-    : transient_(new Dirtyable<TransientState>()) {}
-
-void StatusController::ResetTransientState() {
-  transient_.reset(new Dirtyable<TransientState>());
-  syncer_status_.value()->over_quota = false;
-}
+    : commit_set_(ModelSafeRoutingInfo()),
+      transient_(new Dirtyable<TransientState>()),
+      group_restriction_in_effect_(false),
+      group_restriction_(GROUP_PASSIVE) {}
 
 // Helper to set the 'dirty' bit in the container of the field being
 // mutated.
@@ -43,21 +41,16 @@ bool StatusController::TestAndClearIsDirty() {
   conflict_progress_.reset_progress_changed();
   return is_dirty;
 }
-void StatusController::set_num_conflicting_commits(int value) {
+void StatusController::increment_num_conflicting_commits_by(int value) {
+  int new_value = error_counters_.value()->num_conflicting_commits + value;
   SetAndMarkDirtyIfChanged(&error_counters_.value()->num_conflicting_commits,
-                           value, &error_counters_);
-}
-
-void StatusController::set_num_consecutive_problem_get_updates(int value) {
-  SetAndMarkDirtyIfChanged(
-      &error_counters_.value()->consecutive_problem_get_updates, value,
+      new_value,
       &error_counters_);
 }
 
-void StatusController::set_num_consecutive_problem_commits(int value) {
-  SetAndMarkDirtyIfChanged(
-      &error_counters_.value()->consecutive_problem_commits, value,
-      &error_counters_);
+void StatusController::reset_num_conflicting_commits() {
+  SetAndMarkDirtyIfChanged(&error_counters_.value()->num_conflicting_commits,
+                           0, &error_counters_);
 }
 
 void StatusController::set_num_consecutive_transient_error_commits(int value) {
@@ -109,6 +102,12 @@ void StatusController::set_syncing(bool syncing) {
                            &syncer_status_);
 }
 
+void StatusController::set_num_successful_bookmark_commits(int value) {
+  SetAndMarkDirtyIfChanged(
+      &syncer_status_.value()->num_successful_bookmark_commits,
+      value, &syncer_status_);
+}
+
 void StatusController::set_num_successful_commits(int value) {
   SetAndMarkDirtyIfChanged(&syncer_status_.value()->num_successful_commits,
                            value, &syncer_status_);
@@ -120,16 +119,6 @@ void StatusController::set_unsynced_handles(
                            unsynced_handles, transient_.get());
 }
 
-void StatusController::increment_num_consecutive_problem_get_updates() {
-  set_num_consecutive_problem_get_updates(
-    error_counters_.value()->consecutive_problem_get_updates + 1);
-}
-
-void StatusController::increment_num_consecutive_problem_commits() {
-  set_num_consecutive_problem_commits(
-    error_counters_.value()->consecutive_problem_commits + 1);
-}
-
 void StatusController::increment_num_consecutive_errors() {
   set_num_consecutive_errors(error_counters_.value()->consecutive_errors + 1);
 }
@@ -140,15 +129,20 @@ void StatusController::increment_num_consecutive_errors_by(int value) {
                              value);
 }
 
+void StatusController::increment_num_successful_bookmark_commits() {
+  set_num_successful_bookmark_commits(
+      syncer_status_.value()->num_successful_bookmark_commits + 1);
+}
+
 void StatusController::increment_num_successful_commits() {
   set_num_successful_commits(
     syncer_status_.value()->num_successful_commits + 1);
 }
 
 // These setters don't affect the dirtiness of TransientState.
-void StatusController::set_commit_ids(
-    const std::vector<syncable::Id>& commit_ids) {
-  transient_->value()->commit_ids = commit_ids;
+void StatusController::set_commit_set(const OrderedCommitSet& commit_set) {
+  DCHECK(!group_restriction_in_effect_);
+  commit_set_ = commit_set;
 }
 
 void StatusController::set_conflict_sets_built(bool built) {
@@ -175,5 +169,11 @@ int64 StatusController::CountUpdates() const {
   }
 }
 
+bool StatusController::CurrentCommitIdProjectionHasIndex(size_t index) {
+  OrderedCommitSet::Projection proj =
+      commit_set_.GetCommitIdProjection(group_restriction_);
+  return std::binary_search(proj.begin(), proj.end(), index);
+}
+
 }  // namespace sessions
 }  // namespace browser_sync
diff --git a/chrome/browser/sync/sessions/status_controller.h b/chrome/browser/sync/sessions/status_controller.h
index aa177f8..40752f6 100644
--- a/chrome/browser/sync/sessions/status_controller.h
+++ b/chrome/browser/sync/sessions/status_controller.h
@@ -14,6 +14,7 @@
 #define CHROME_BROWSER_SYNC_SESSIONS_STATUS_CONTROLLER_H_
 
 #include "base/scoped_ptr.h"
+#include "chrome/browser/sync/sessions/ordered_commit_set.h"
 #include "chrome/browser/sync/sessions/session_state.h"
 
 namespace browser_sync {
@@ -27,12 +28,6 @@ class StatusController {
   // since it was created or was last reset.
   bool TestAndClearIsDirty();
 
-  // Discards the current transient state components that should not carry over
-  // to a subsequent sync cycle (a run between states in SyncShare), and keeps
-  // everything else intact. After the call, |this| is ready for use
-  // as part of a new sync cycle.
-  void ResetTransientState();
-
   ConflictProgress const* conflict_progress() const {
     return &conflict_progress_;
   }
@@ -70,7 +65,21 @@ class StatusController {
     return change_progress_.value();
   }
   const std::vector<syncable::Id>& commit_ids() const {
-    return transient_->value()->commit_ids;
+    DCHECK(!group_restriction_in_effect_) << "Group restriction in effect!";
+    return commit_set_.GetAllCommitIds();
+  }
+  const OrderedCommitSet::Projection& commit_id_projection() {
+    DCHECK(group_restriction_in_effect_)
+        << "No group restriction for projection.";
+    return commit_set_.GetCommitIdProjection(group_restriction_);
+  }
+  const syncable::Id& GetCommitIdAt(size_t index) {
+    DCHECK(CurrentCommitIdProjectionHasIndex(index));
+    return commit_set_.GetCommitIdAt(index);
+  }
+  const syncable::ModelType GetCommitIdModelTypeAt(size_t index) {
+    DCHECK(CurrentCommitIdProjectionHasIndex(index));
+    return commit_set_.GetModelTypeAt(index);
   }
   const std::vector<int64>& unsynced_handles() const {
     return transient_->value()->unsynced_handles;
@@ -91,14 +100,17 @@ class StatusController {
   // Returns the number of updates received from the sync server.
   int64 CountUpdates() const;
 
+  // Returns true iff any of the commit ids added during this session are
+  // bookmark related.
+  bool HasBookmarkCommitActivity() const {
+    return commit_set_.HasBookmarkCommitId();
+  }
+
   bool got_zero_updates() const { return CountUpdates() == 0; }
 
   // A toolbelt full of methods for updating counters and flags.
-  void set_num_conflicting_commits(int value);
-  void set_num_consecutive_problem_get_updates(int value);
-  void increment_num_consecutive_problem_get_updates();
-  void set_num_consecutive_problem_commits(int value);
-  void increment_num_consecutive_problem_commits();
+  void increment_num_conflicting_commits_by(int value);
+  void reset_num_conflicting_commits();
   void set_num_consecutive_transient_error_commits(int value);
   void increment_num_consecutive_transient_error_commits_by(int value);
   void set_num_consecutive_errors(int value);
@@ -111,16 +123,24 @@ class StatusController {
   void set_syncer_stuck(bool syncer_stuck);
   void set_syncing(bool syncing);
   void set_num_successful_commits(int value);
+  void set_num_successful_bookmark_commits(int value);
   void increment_num_successful_commits();
+  void increment_num_successful_bookmark_commits();
   void set_unsynced_handles(const std::vector<int64>& unsynced_handles);
 
-  void set_commit_ids(const std::vector<syncable::Id>& commit_ids);
+  void set_commit_set(const OrderedCommitSet& commit_set);
   void set_conflict_sets_built(bool built);
   void set_conflicts_resolved(bool resolved);
   void set_items_committed(bool items_committed);
   void set_timestamp_dirty(bool dirty);
 
  private:
+  friend class ScopedModelSafeGroupRestriction;
+
+  // Returns true iff the commit id projection for |group_restriction_|
+  // references position |index| into the full set of commit ids in play.
+  bool CurrentCommitIdProjectionHasIndex(size_t index);
+
   // Dirtyable keeps a dirty bit that can be set, cleared, and checked to
   // determine if a notification should be sent due to state change.
   // This is useful when applied to any session state object if you want to know
@@ -138,6 +158,8 @@ class StatusController {
     bool dirty_;
   };
 
+  OrderedCommitSet commit_set_;
+
   // Various pieces of state we track dirtiness of.
   Dirtyable<ChangelogProgress> change_progress_;
   Dirtyable<SyncerStatus> syncer_status_;
@@ -146,10 +168,17 @@ class StatusController {
   // The transient parts of a sync session that can be reset during the session.
   // For some parts of this state, we want to track whether changes occurred so
   // we allocate a Dirtyable version.
+  // TODO(tim): Get rid of transient state since it has no valid use case
+  // anymore.
   scoped_ptr<Dirtyable<TransientState> > transient_;
 
   ConflictProgress conflict_progress_;
 
+  // Used to fail read/write operations on state that don't obey the current
+  // active ModelSafeWorker contract.
+  bool group_restriction_in_effect_;
+  ModelSafeGroup group_restriction_;
+
   DISALLOW_COPY_AND_ASSIGN(StatusController);
 };
 
diff --git a/chrome/browser/sync/sessions/status_controller_unittest.cc b/chrome/browser/sync/sessions/status_controller_unittest.cc
index 139fa70..95ee36d 100644
--- a/chrome/browser/sync/sessions/status_controller_unittest.cc
+++ b/chrome/browser/sync/sessions/status_controller_unittest.cc
@@ -12,24 +12,12 @@ typedef testing::Test StatusControllerTest;
 
 TEST_F(StatusControllerTest, GetsDirty) {
   StatusController status;
-  status.set_num_conflicting_commits(1);
+  status.increment_num_conflicting_commits_by(1);
   EXPECT_TRUE(status.TestAndClearIsDirty());
   EXPECT_FALSE(status.TestAndClearIsDirty());  // Test that it actually resets.
-  status.set_num_conflicting_commits(1);
+  status.increment_num_conflicting_commits_by(0);
   EXPECT_FALSE(status.TestAndClearIsDirty());
-  status.set_num_conflicting_commits(0);
-  EXPECT_TRUE(status.TestAndClearIsDirty());
-
-  status.set_num_consecutive_problem_get_updates(1);
-  EXPECT_TRUE(status.TestAndClearIsDirty());
-
-  status.increment_num_consecutive_problem_get_updates();
-  EXPECT_TRUE(status.TestAndClearIsDirty());
-
-  status.set_num_consecutive_problem_commits(1);
-  EXPECT_TRUE(status.TestAndClearIsDirty());
-
-  status.increment_num_consecutive_problem_commits();
+  status.increment_num_conflicting_commits_by(1);
   EXPECT_TRUE(status.TestAndClearIsDirty());
 
   status.set_num_consecutive_transient_error_commits(1);
@@ -77,7 +65,7 @@ TEST_F(StatusControllerTest, GetsDirty) {
   status.set_syncing(false);
   EXPECT_TRUE(status.TestAndClearIsDirty());
 
-  status.set_num_successful_commits(1);
+  status.increment_num_successful_commits();
   EXPECT_TRUE(status.TestAndClearIsDirty());
   status.increment_num_successful_commits();
   EXPECT_TRUE(status.TestAndClearIsDirty());
@@ -103,9 +91,11 @@ TEST_F(StatusControllerTest, StaysClean) {
   status.set_items_committed(true);
   EXPECT_FALSE(status.TestAndClearIsDirty());
 
-  std::vector<syncable::Id> v;
-  v.push_back(syncable::Id());
-  status.set_commit_ids(v);
+  ModelSafeRoutingInfo routes;
+  routes[syncable::BOOKMARKS] = GROUP_UI;
+  OrderedCommitSet commits(routes);
+  commits.AddCommitItem(0, syncable::Id(), syncable::BOOKMARKS);
+  status.set_commit_set(commits);
   EXPECT_FALSE(status.TestAndClearIsDirty());
 }
 
@@ -114,19 +104,9 @@ TEST_F(StatusControllerTest, StaysClean) {
 // method was actually setting |bar_| instead!).
 TEST_F(StatusControllerTest, ReadYourWrites) {
   StatusController status;
-  status.set_num_conflicting_commits(1);
+  status.increment_num_conflicting_commits_by(1);
   EXPECT_EQ(1, status.error_counters().num_conflicting_commits);
 
-  status.set_num_consecutive_problem_get_updates(2);
-  EXPECT_EQ(2, status.error_counters().consecutive_problem_get_updates);
-  status.increment_num_consecutive_problem_get_updates();
-  EXPECT_EQ(3, status.error_counters().consecutive_problem_get_updates);
-
-  status.set_num_consecutive_problem_commits(4);
-  EXPECT_EQ(4, status.error_counters().consecutive_problem_commits);
-  status.increment_num_consecutive_problem_commits();
-  EXPECT_EQ(5, status.error_counters().consecutive_problem_commits);
-
   status.set_num_consecutive_transient_error_commits(6);
   EXPECT_EQ(6, status.error_counters().consecutive_transient_error_commits);
   status.increment_num_consecutive_transient_error_commits_by(1);
@@ -174,44 +154,6 @@ TEST_F(StatusControllerTest, ReadYourWrites) {
   EXPECT_EQ(16, v[0]);
 }
 
-TEST_F(StatusControllerTest, ResetTransientState) {
-  StatusController status;
-  status.set_conflict_sets_built(true);
-  EXPECT_TRUE(status.conflict_sets_built());
-  status.set_timestamp_dirty(true);
-  status.set_items_committed(true);
-  status.set_conflicts_resolved(true);
-  sync_pb::SyncEntity entity;
-  status.mutable_update_progress()->AddVerifyResult(VERIFY_SUCCESS, entity);
-  status.mutable_commit_message()->mutable_commit();  // Lazy initialization.
-  ASSERT_TRUE(status.mutable_commit_message()->has_commit());
-  status.mutable_commit_response()->mutable_commit();
-  ASSERT_TRUE(status.commit_response().has_commit());
-  status.mutable_updates_response()->mutable_get_updates();
-  ASSERT_TRUE(status.updates_response().has_get_updates());
-
-  std::vector<int64> v;
-  v.push_back(1);
-  status.set_unsynced_handles(v);
-
-  std::vector<syncable::Id> v2;
-  v2.push_back(syncable::Id());
-  status.set_commit_ids(v2);
-
-  EXPECT_TRUE(status.TestAndClearIsDirty());
-  status.ResetTransientState();
-  EXPECT_FALSE(status.TestAndClearIsDirty());
-
-  EXPECT_FALSE(status.conflict_sets_built());
-  EXPECT_FALSE(status.timestamp_dirty());
-  EXPECT_FALSE(status.did_commit_items());
-  EXPECT_FALSE(status.conflicts_resolved());
-  EXPECT_EQ(0, status.update_progress().VerifiedUpdatesSize());
-  EXPECT_FALSE(status.mutable_commit_message()->has_commit());
-  EXPECT_FALSE(status.commit_response().has_commit());
-  EXPECT_FALSE(status.updates_response().has_get_updates());
-}
-
 TEST_F(StatusControllerTest, HasConflictingUpdates) {
   StatusController status;
   EXPECT_FALSE(status.update_progress().HasConflictingUpdates());
diff --git a/chrome/browser/sync/sessions/sync_session.cc b/chrome/browser/sync/sessions/sync_session.cc
index 27e2235..a863a5e 100644
--- a/chrome/browser/sync/sessions/sync_session.cc
+++ b/chrome/browser/sync/sessions/sync_session.cc
@@ -12,8 +12,7 @@ SyncSession::SyncSession(SyncSessionContext* context, Delegate* delegate)
     : context_(context),
       source_(sync_pb::GetUpdatesCallerInfo::UNKNOWN),
       write_transaction_(NULL),
-      delegate_(delegate),
-      auth_failure_occurred_(false) {
+      delegate_(delegate) {
 
   context_->registrar()->GetWorkers(
       const_cast<std::vector<ModelSafeWorker*>*>(&workers_));
@@ -30,8 +29,6 @@ SyncSession::SyncSession(SyncSessionContext* context, Delegate* delegate)
   // synced) *continue* to be for this whole session, even though the
   // ModelSafeWorkerRegistrar may be configured to route syncable::AUTOFILL to
   // GROUP_DB now.
-  group_restriction_in_effect_ = false;
-  group_restriction_ = GROUP_PASSIVE;
 }
 
 SyncSessionSnapshot SyncSession::TakeSnapshot() const {
diff --git a/chrome/browser/sync/sessions/sync_session.h b/chrome/browser/sync/sessions/sync_session.h
index 196d256..5e49cec 100644
--- a/chrome/browser/sync/sessions/sync_session.h
+++ b/chrome/browser/sync/sessions/sync_session.h
@@ -20,6 +20,7 @@
 #include "base/basictypes.h"
 #include "base/scoped_ptr.h"
 #include "base/time.h"
+#include "chrome/browser/sync/sessions/ordered_commit_set.h"
 #include "chrome/browser/sync/sessions/session_state.h"
 #include "chrome/browser/sync/sessions/status_controller.h"
 #include "chrome/browser/sync/sessions/sync_session_context.h"
@@ -90,10 +91,6 @@ class SyncSession {
     return &extensions_activity_;
   }
 
-  bool auth_failure_occurred() const { return auth_failure_occurred_; }
-  void set_auth_failure_occurred() { auth_failure_occurred_ = true; }
-  void clear_auth_failure_occurred() { auth_failure_occurred_ = false; }
-
   // Volatile reader for the source member of the sync session object.  The
   // value is set to the SYNC_CYCLE_CONTINUATION value to signal that it has
   // been read.
@@ -110,7 +107,6 @@ class SyncSession {
   // assignments. This way, the scope of these actions is explicit, they can't
   // be overridden, and assigning is always accompanied by unassigning.
   friend class ScopedSetSessionWriteTransaction;
-  friend class ScopedModelSafeGroupRestriction;
 
   // The context for this session, guaranteed to outlive |this|.
   SyncSessionContext* const context_;
@@ -130,9 +126,6 @@ class SyncSession {
   // Our controller for various status and error counters.
   StatusController status_controller_;
 
-  // Used to determine if an auth error notification should be sent out.
-  bool auth_failure_occurred_;
-
   // The set of active ModelSafeWorkers for the duration of this session.
   const std::vector<ModelSafeWorker*> workers_;
 
@@ -141,11 +134,6 @@ class SyncSession {
   // on those datatypes.
   const ModelSafeRoutingInfo routing_info_;
 
-  // Used to fail read/write operations on this SyncSession that don't obey the
-  // currently active ModelSafeWorker contract.
-  bool group_restriction_in_effect_;
-  ModelSafeGroup group_restriction_;
-
   DISALLOW_COPY_AND_ASSIGN(SyncSession);
 };
 
@@ -175,12 +163,13 @@ class ScopedModelSafeGroupRestriction {
   ScopedModelSafeGroupRestriction(SyncSession* to_restrict,
                                ModelSafeGroup restriction)
       : session_(to_restrict) {
-    DCHECK(!session_->group_restriction_in_effect_);
-    session_->group_restriction_in_effect_ = true;
-    session_->group_restriction_ = restriction;
+    DCHECK(!session_->status_controller()->group_restriction_in_effect_);
+    session_->status_controller()->group_restriction_ = restriction;
+    session_->status_controller()->group_restriction_in_effect_ = true;
   }
   ~ScopedModelSafeGroupRestriction() {
-    session_->group_restriction_in_effect_ = false;
+    DCHECK(session_->status_controller()->group_restriction_in_effect_);
+    session_->status_controller()->group_restriction_in_effect_ = false;
   }
  private:
   SyncSession* session_;
diff --git a/chrome/browser/sync/sessions/sync_session_unittest.cc b/chrome/browser/sync/sessions/sync_session_unittest.cc
index 8c9cfe3..a50a06b 100644
--- a/chrome/browser/sync/sessions/sync_session_unittest.cc
+++ b/chrome/browser/sync/sessions/sync_session_unittest.cc
@@ -22,7 +22,9 @@ class SyncSessionTest : public testing::Test,
                         public SyncSession::Delegate,
                         public ModelSafeWorkerRegistrar {
  public:
-  SyncSessionTest() : controller_invocations_allowed_(false) {}
+  SyncSessionTest() : controller_invocations_allowed_(false) {
+    GetModelSafeRoutingInfo(&routes_);
+  }
   virtual void SetUp() {
     context_.reset(new SyncSessionContext(NULL, NULL, this));
     session_.reset(new SyncSession(context_.get(), this));
@@ -50,7 +52,9 @@ class SyncSessionTest : public testing::Test,
 
   // ModelSafeWorkerRegistrar implementation.
   virtual void GetWorkers(std::vector<ModelSafeWorker*>* out) {}
-  virtual void GetModelSafeRoutingInfo(ModelSafeRoutingInfo* out) {}
+  virtual void GetModelSafeRoutingInfo(ModelSafeRoutingInfo* out) {
+    (*out)[syncable::BOOKMARKS] = GROUP_UI;
+  }
 
   StatusController* status() { return session_->status_controller(); }
  protected:
@@ -61,6 +65,7 @@ class SyncSessionTest : public testing::Test,
   bool controller_invocations_allowed_;
   scoped_ptr<SyncSession> session_;
   scoped_ptr<SyncSessionContext> context_;
+  ModelSafeRoutingInfo routes_;
 };
 
 TEST_F(SyncSessionTest, ScopedContextHelpers) {
@@ -107,9 +112,9 @@ TEST_F(SyncSessionTest, MoreToSyncIfUnsyncedGreaterThanCommitted) {
   // more to sync. For example, this occurs if we had more commit ids
   // than could fit in a single commit batch.
   EXPECT_FALSE(session_->HasMoreToSync());
-  std::vector<syncable::Id> commit_ids;
-  commit_ids.push_back(syncable::Id());
-  status()->set_commit_ids(commit_ids);
+  OrderedCommitSet commit_set(routes_);
+  commit_set.AddCommitItem(0, syncable::Id(), syncable::BOOKMARKS);
+  status()->set_commit_set(commit_set);
   EXPECT_FALSE(session_->HasMoreToSync());
 
   std::vector<int64> unsynced_handles;
@@ -119,8 +124,6 @@ TEST_F(SyncSessionTest, MoreToSyncIfUnsyncedGreaterThanCommitted) {
   EXPECT_FALSE(session_->HasMoreToSync());
   status()->increment_num_successful_commits();
   EXPECT_TRUE(session_->HasMoreToSync());
-  status()->ResetTransientState();
-  EXPECT_FALSE(session_->HasMoreToSync());
 }
 
 TEST_F(SyncSessionTest, MoreToSyncIfConflictSetsBuilt) {
@@ -128,8 +131,6 @@ TEST_F(SyncSessionTest, MoreToSyncIfConflictSetsBuilt) {
   // to get updates & commit again.
   status()->set_conflict_sets_built(true);
   EXPECT_TRUE(session_->HasMoreToSync());
-  status()->ResetTransientState();
-  EXPECT_FALSE(session_->HasMoreToSync());
 }
 
 TEST_F(SyncSessionTest, MoreToSyncIfDidNotGetZeroUpdates) {
@@ -142,8 +143,6 @@ TEST_F(SyncSessionTest, MoreToSyncIfDidNotGetZeroUpdates) {
   EXPECT_FALSE(session_->HasMoreToSync());
   status()->mutable_updates_response()->CopyFrom(response);
   EXPECT_TRUE(session_->HasMoreToSync());
-  status()->ResetTransientState();
-  EXPECT_FALSE(session_->HasMoreToSync());
 }
 
 TEST_F(SyncSessionTest, MoreToSyncIfConflictsResolved) {
@@ -152,8 +151,6 @@ TEST_F(SyncSessionTest, MoreToSyncIfConflictsResolved) {
   // that we have made forward progress.
   status()->set_conflicts_resolved(true);
   EXPECT_TRUE(session_->HasMoreToSync());
-  status()->ResetTransientState();
-  EXPECT_FALSE(session_->HasMoreToSync());
 }
 
 TEST_F(SyncSessionTest, MoreToSyncIfTimestampDirty) {
@@ -163,8 +160,6 @@ TEST_F(SyncSessionTest, MoreToSyncIfTimestampDirty) {
   status()->set_timestamp_dirty(true);
   status()->set_conflicts_resolved(true);
   EXPECT_TRUE(session_->HasMoreToSync());
-  status()->ResetTransientState();
-  EXPECT_FALSE(session_->HasMoreToSync());
 }