Multi-pass ModelChangingSyncerCommands.

BUG=31911

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

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

28 files changed, 651 insertions, 341 deletions

diff --git a/chrome/browser/sync/engine/all_status.cc b/chrome/browser/sync/engine/all_status.cc
index 49ec3ce..1a7fa78 100644
--- a/chrome/browser/sync/engine/all_status.cc
+++ b/chrome/browser/sync/engine/all_status.cc
@@ -112,9 +112,8 @@ AllStatus::Status AllStatus::CalcSyncing(const SyncerEvent &event) const {
   if (errors.consecutive_transient_error_commits > 100)
     status.server_broken = true;
 
-  const sessions::ChangelogProgress& progress(snapshot->changelog_progress);
-  status.updates_available += progress.num_server_changes_remaining;
-  status.updates_received += progress.current_sync_timestamp;
+  status.updates_available += snapshot->num_server_changes_remaining;
+  status.updates_received += snapshot->max_local_timestamp;
   return status;
 }
 
diff --git a/chrome/browser/sync/engine/apply_updates_command.cc b/chrome/browser/sync/engine/apply_updates_command.cc
index dda4e64..abce4db 100644
--- a/chrome/browser/sync/engine/apply_updates_command.cc
+++ b/chrome/browser/sync/engine/apply_updates_command.cc
@@ -29,7 +29,8 @@ void ApplyUpdatesCommand::ModelChangingExecuteImpl(SyncSession* session) {
   dir->GetUnappliedUpdateMetaHandles(&trans, &handles);
 
   UpdateApplicator applicator(session->context()->resolver(), handles.begin(),
-                              handles.end());
+      handles.end(), session->routing_info(),
+      session->status_controller()->group_restriction());
   while (applicator.AttemptOneApplication(&trans)) {}
   applicator.SaveProgressIntoSessionState(
       session->status_controller()->mutable_conflict_progress(),
diff --git a/chrome/browser/sync/engine/apply_updates_command_unittest.cc b/chrome/browser/sync/engine/apply_updates_command_unittest.cc
index a6bd108..4ec7ee3 100755
--- a/chrome/browser/sync/engine/apply_updates_command_unittest.cc
+++ b/chrome/browser/sync/engine/apply_updates_command_unittest.cc
@@ -30,6 +30,14 @@ class ApplyUpdatesCommandTest : public SyncerCommandTest {
   ApplyUpdatesCommandTest() : next_revision_(1) {}
   virtual ~ApplyUpdatesCommandTest() {}
 
+  virtual void SetUp() {
+    workers()->clear();
+    mutable_routing_info()->clear();
+    workers()->push_back(new ModelSafeWorker());  // GROUP_PASSIVE worker.
+    (*mutable_routing_info())[syncable::BOOKMARKS] = GROUP_PASSIVE;
+    SyncerCommandTest::SetUp();
+  }
+
   // Create a new unapplied update.
   void CreateUnappliedNewItemWithParent(const string& item_id,
                                         const string& parent_id) {
@@ -62,12 +70,14 @@ TEST_F(ApplyUpdatesCommandTest, Simple) {
   CreateUnappliedNewItemWithParent("parent", root_server_id);
   CreateUnappliedNewItemWithParent("child", "parent");
 
-  apply_updates_command_.ModelChangingExecuteImpl(session());
+  apply_updates_command_.ExecuteImpl(session());
 
   sessions::StatusController* status = session()->status_controller();
+
+  sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE);
   EXPECT_EQ(2, status->update_progress().AppliedUpdatesSize())
       << "All updates should have been attempted";
-  EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize())
+  EXPECT_EQ(0, status->conflict_progress().ConflictingItemsSize())
       << "Simple update shouldn't result in conflicts";
   EXPECT_EQ(2, status->update_progress().SuccessfullyAppliedUpdateCount())
       << "All items should have been successfully applied";
@@ -83,12 +93,13 @@ TEST_F(ApplyUpdatesCommandTest, UpdateWithChildrenBeforeParents) {
   CreateUnappliedNewItemWithParent("a_child_created_second", "parent");
   CreateUnappliedNewItemWithParent("x_child_created_second", "parent");
 
-  apply_updates_command_.ModelChangingExecuteImpl(session());
+  apply_updates_command_.ExecuteImpl(session());
 
   sessions::StatusController* status = session()->status_controller();
+  sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE);
   EXPECT_EQ(5, status->update_progress().AppliedUpdatesSize())
       << "All updates should have been attempted";
-  EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize())
+  EXPECT_EQ(0, status->conflict_progress().ConflictingItemsSize())
       << "Simple update shouldn't result in conflicts, even if out-of-order";
   EXPECT_EQ(5, status->update_progress().SuccessfullyAppliedUpdateCount())
       << "All updates should have been successfully applied";
@@ -99,12 +110,13 @@ TEST_F(ApplyUpdatesCommandTest, NestedItemsWithUnknownParent) {
   CreateUnappliedNewItemWithParent("some_item", "unknown_parent");
   CreateUnappliedNewItemWithParent("some_other_item", "some_item");
 
-  apply_updates_command_.ModelChangingExecuteImpl(session());
+  apply_updates_command_.ExecuteImpl(session());
 
   sessions::StatusController* status = session()->status_controller();
+  sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE);
   EXPECT_EQ(2, status->update_progress().AppliedUpdatesSize())
       << "All updates should have been attempted";
-  EXPECT_EQ(2, status->conflict_progress()->ConflictingItemsSize())
+  EXPECT_EQ(2, status->conflict_progress().ConflictingItemsSize())
       << "All updates with an unknown ancestors should be in conflict";
   EXPECT_EQ(0, status->update_progress().SuccessfullyAppliedUpdateCount())
       << "No item with an unknown ancestor should be applied";
@@ -120,12 +132,13 @@ TEST_F(ApplyUpdatesCommandTest, ItemsBothKnownAndUnknown) {
   CreateUnappliedNewItemWithParent("third_known_item", "fourth_known_item");
   CreateUnappliedNewItemWithParent("fourth_known_item", root_server_id);
 
-  apply_updates_command_.ModelChangingExecuteImpl(session());
+  apply_updates_command_.ExecuteImpl(session());
 
   sessions::StatusController* status = session()->status_controller();
+  sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE);
   EXPECT_EQ(6, status->update_progress().AppliedUpdatesSize())
       << "All updates should have been attempted";
-  EXPECT_EQ(2, status->conflict_progress()->ConflictingItemsSize())
+  EXPECT_EQ(2, status->conflict_progress().ConflictingItemsSize())
       << "The updates with unknown ancestors should be in conflict";
   EXPECT_EQ(4, status->update_progress().SuccessfullyAppliedUpdateCount())
       << "The updates with known ancestors should be successfully applied";
diff --git a/chrome/browser/sync/engine/build_and_process_conflict_sets_command.cc b/chrome/browser/sync/engine/build_and_process_conflict_sets_command.cc
index 41988f5..be45cdd 100755
--- a/chrome/browser/sync/engine/build_and_process_conflict_sets_command.cc
+++ b/chrome/browser/sync/engine/build_and_process_conflict_sets_command.cc
@@ -31,7 +31,7 @@ BuildAndProcessConflictSetsCommand::~BuildAndProcessConflictSetsCommand() {}
 
 void BuildAndProcessConflictSetsCommand::ModelChangingExecuteImpl(
     SyncSession* session) {
-  session->status_controller()->set_conflict_sets_built(
+  session->status_controller()->update_conflict_sets_built(
       BuildAndProcessConflictSets(session));
 }
 
@@ -47,7 +47,8 @@ bool BuildAndProcessConflictSetsCommand::BuildAndProcessConflictSets(
     BuildConflictSets(&trans,
         session->status_controller()->mutable_conflict_progress());
     had_single_direction_sets = ProcessSingleDirectionConflictSets(&trans,
-        session->context()->resolver(), session->status_controller());
+        session->context()->resolver(), session->status_controller(),
+        session->routing_info());
     // We applied some updates transactionally, lets try syncing again.
     if (had_single_direction_sets)
       return true;
@@ -57,11 +58,11 @@ bool BuildAndProcessConflictSetsCommand::BuildAndProcessConflictSets(
 
 bool BuildAndProcessConflictSetsCommand::ProcessSingleDirectionConflictSets(
     syncable::WriteTransaction* trans, ConflictResolver* resolver,
-    StatusController* status) {
+    StatusController* status, const ModelSafeRoutingInfo& routes) {
   bool rv = false;
   set<ConflictSet*>::const_iterator all_sets_iterator;
-  for (all_sets_iterator = status->conflict_progress()->ConflictSetsBegin();
-       all_sets_iterator != status->conflict_progress()->ConflictSetsEnd();) {
+  for (all_sets_iterator = status->conflict_progress().ConflictSetsBegin();
+       all_sets_iterator != status->conflict_progress().ConflictSetsEnd();) {
     const ConflictSet* conflict_set = *all_sets_iterator;
     CHECK(conflict_set->size() >= 2);
     // We scan the set to see if it consists of changes of only one type.
@@ -78,7 +79,8 @@ bool BuildAndProcessConflictSetsCommand::ProcessSingleDirectionConflictSets(
     if (conflict_set->size() == unsynced_count && 0 == unapplied_count) {
       LOG(INFO) << "Skipped transactional commit attempt.";
     } else if (conflict_set->size() == unapplied_count && 0 == unsynced_count &&
-          ApplyUpdatesTransactionally(trans, conflict_set, resolver, status)) {
+          ApplyUpdatesTransactionally(trans, conflict_set, resolver, routes,
+                                      status)) {
       rv = true;
     }
     ++all_sets_iterator;
@@ -143,7 +145,9 @@ void PlaceEntriesAtRoot(syncable::WriteTransaction* trans,
 bool BuildAndProcessConflictSetsCommand::ApplyUpdatesTransactionally(
     syncable::WriteTransaction* trans,
     const vector<syncable::Id>* const update_set,
-    ConflictResolver* resolver, StatusController* status) {
+    ConflictResolver* resolver,
+    const ModelSafeRoutingInfo& routes,
+    StatusController* status) {
   // The handles in the |update_set| order.
   vector<int64> handles;
 
@@ -188,7 +192,8 @@ bool BuildAndProcessConflictSetsCommand::ApplyUpdatesTransactionally(
 
   // 5. Use the usual apply updates from the special start state we've just
   // prepared.
-  UpdateApplicator applicator(resolver, handles.begin(), handles.end());
+  UpdateApplicator applicator(resolver, handles.begin(), handles.end(),
+      routes, status->group_restriction());
   while (applicator.AttemptOneApplication(trans)) {
     // Keep going till all updates are applied.
   }
diff --git a/chrome/browser/sync/engine/build_and_process_conflict_sets_command.h b/chrome/browser/sync/engine/build_and_process_conflict_sets_command.h
index 4a408c7..00cca6d 100644
--- a/chrome/browser/sync/engine/build_and_process_conflict_sets_command.h
+++ b/chrome/browser/sync/engine/build_and_process_conflict_sets_command.h
@@ -9,6 +9,7 @@
 
 #include "base/basictypes.h"
 #include "chrome/browser/sync/engine/model_changing_syncer_command.h"
+#include "chrome/browser/sync/engine/model_safe_worker.h"
 
 namespace syncable {
 class BaseTransaction;
@@ -40,11 +41,12 @@ class BuildAndProcessConflictSetsCommand : public ModelChangingSyncerCommand {
 
   bool ProcessSingleDirectionConflictSets(
       syncable::WriteTransaction* trans, ConflictResolver* resolver,
-      sessions::StatusController* status);
+      sessions::StatusController* status, const ModelSafeRoutingInfo& routes);
   bool ApplyUpdatesTransactionally(
       syncable::WriteTransaction* trans,
       const std::vector<syncable::Id>* const update_set,
       ConflictResolver* resolver,
+      const ModelSafeRoutingInfo& routes,
       sessions::StatusController* status);
   void BuildConflictSets(syncable::BaseTransaction* trans,
                          sessions::ConflictProgress* conflict_progress);
diff --git a/chrome/browser/sync/engine/conflict_resolver.cc b/chrome/browser/sync/engine/conflict_resolver.cc
index 7914ab5..7bc9418 100755
--- a/chrome/browser/sync/engine/conflict_resolver.cc
+++ b/chrome/browser/sync/engine/conflict_resolver.cc
@@ -386,7 +386,7 @@ bool ConflictResolver::LogAndSignalIfConflictStuck(
   }
 
   // Don't signal stuck if we're not up to date.
-  if (status->change_progress().num_server_changes_remaining > 0) {
+  if (status->num_server_changes_remaining() > 0) {
     return false;
   }
 
@@ -412,16 +412,16 @@ bool ConflictResolver::ResolveSimpleConflicts(const ScopedDirLookup& dir,
                                               StatusController* status) {
   WriteTransaction trans(dir, syncable::SYNCER, __FILE__, __LINE__);
   bool forward_progress = false;
-  ConflictProgress const* progress = status->conflict_progress();
+  const ConflictProgress& progress = status->conflict_progress();
   // First iterate over simple conflict items (those that belong to no set).
   set<Id>::const_iterator conflicting_item_it;
-  for (conflicting_item_it = progress->ConflictingItemsBeginConst();
-       conflicting_item_it != progress->ConflictingItemsEnd();
+  for (conflicting_item_it = progress.ConflictingItemsBeginConst();
+       conflicting_item_it != progress.ConflictingItemsEnd();
        ++conflicting_item_it) {
     Id id = *conflicting_item_it;
     map<Id, ConflictSet*>::const_iterator item_set_it =
-        progress->IdToConflictSetFind(id);
-    if (item_set_it == progress->IdToConflictSetEnd() ||
+        progress.IdToConflictSetFind(id);
+    if (item_set_it == progress.IdToConflictSetEnd() ||
         0 == item_set_it->second) {
       // We have a simple conflict.
       switch (ProcessSimpleConflict(&trans, id)) {
@@ -451,7 +451,7 @@ bool ConflictResolver::ResolveSimpleConflicts(const ScopedDirLookup& dir,
 
 bool ConflictResolver::ResolveConflicts(const ScopedDirLookup& dir,
                                         StatusController* status) {
-  ConflictProgress const* progress = status->conflict_progress();
+  const ConflictProgress& progress = status->conflict_progress();
   bool rv = false;
   if (ResolveSimpleConflicts(dir, status))
     rv = true;
@@ -459,8 +459,8 @@ bool ConflictResolver::ResolveConflicts(const ScopedDirLookup& dir,
   set<Id> children_of_dirs_merged_last_round;
   std::swap(children_of_merged_dirs_, children_of_dirs_merged_last_round);
   set<ConflictSet*>::const_iterator set_it;
-  for (set_it = progress->ConflictSetsBegin();
-       set_it != progress->ConflictSetsEnd();
+  for (set_it = progress.ConflictSetsBegin();
+       set_it != progress.ConflictSetsEnd();
        set_it++) {
     ConflictSet* conflict_set = *set_it;
     ConflictSetCountMapKey key = GetSetKey(conflict_set);
diff --git a/chrome/browser/sync/engine/model_changing_syncer_command.cc b/chrome/browser/sync/engine/model_changing_syncer_command.cc
index 2a0097d..c9f2d4d 100644
--- a/chrome/browser/sync/engine/model_changing_syncer_command.cc
+++ b/chrome/browser/sync/engine/model_changing_syncer_command.cc
@@ -6,6 +6,7 @@
 
 #include "base/callback.h"
 #include "chrome/browser/sync/engine/model_safe_worker.h"
+#include "chrome/browser/sync/sessions/status_controller.h"
 #include "chrome/browser/sync/sessions/sync_session.h"
 #include "chrome/browser/sync/util/closure.h"
 
@@ -21,7 +22,8 @@ void ModelChangingSyncerCommand::ExecuteImpl(sessions::SyncSession* session) {
     ModelSafeWorker* worker = session->workers()[i];
     ModelSafeGroup group = worker->GetModelSafeGroup();
 
-    sessions::ScopedModelSafeGroupRestriction r(work_session_, group);
+    sessions::StatusController* status = work_session_->status_controller();
+    sessions::ScopedModelSafeGroupRestriction r(status, group);
     scoped_ptr<Closure> c(NewCallback(this,
         &ModelChangingSyncerCommand::StartChangingModel));
     worker->DoWorkAndWaitUntilDone(c.get());
diff --git a/chrome/browser/sync/engine/model_changing_syncer_command.h b/chrome/browser/sync/engine/model_changing_syncer_command.h
index f0ee028..c08ade2 100644
--- a/chrome/browser/sync/engine/model_changing_syncer_command.h
+++ b/chrome/browser/sync/engine/model_changing_syncer_command.h
@@ -40,6 +40,7 @@ class ModelChangingSyncerCommand : public SyncerCommand {
   // safe.  This will be called once, prior to ModelChangingExecuteImpl,
   // *without* a ModelSafeGroup restriction in place on the SyncSession.
   // Returns true on success, false on failure.
+  // TODO(tim): Remove this (bug 36594).
   virtual bool ModelNeutralExecuteImpl(sessions::SyncSession* session) {
     return true;
   }
diff --git a/chrome/browser/sync/engine/model_safe_worker.cc b/chrome/browser/sync/engine/model_safe_worker.cc
index 3d42164..6df073a 100644
--- a/chrome/browser/sync/engine/model_safe_worker.cc
+++ b/chrome/browser/sync/engine/model_safe_worker.cc
@@ -10,7 +10,11 @@ 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!";
+    // TODO(tim): We shouldn't end up here for TOP_LEVEL_FOLDER, but an issue
+    // with the server's PermanentItemPopulator is causing TLF updates in
+    // some cases.  See bug 36735.
+    if (type != syncable::UNSPECIFIED && type != syncable::TOP_LEVEL_FOLDER)
+      NOTREACHED() << "Entry does not belong to active ModelSafeGroup!";
     return GROUP_PASSIVE;
   }
   return it->second;
diff --git a/chrome/browser/sync/engine/post_commit_message_command.cc b/chrome/browser/sync/engine/post_commit_message_command.cc
index c0a7c98..eea5d0e 100644
--- a/chrome/browser/sync/engine/post_commit_message_command.cc
+++ b/chrome/browser/sync/engine/post_commit_message_command.cc
@@ -43,7 +43,7 @@ void PostCommitMessageCommand::ExecuteImpl(sessions::SyncSession* session) {
     }
     return;
   } else {
-    status->set_items_committed(true);
+    status->set_items_committed();
   }
   status->mutable_commit_response()->CopyFrom(response);
 }
diff --git a/chrome/browser/sync/engine/process_updates_command.cc b/chrome/browser/sync/engine/process_updates_command.cc
index f17a05d..03af303 100755
--- a/chrome/browser/sync/engine/process_updates_command.cc
+++ b/chrome/browser/sync/engine/process_updates_command.cc
@@ -54,7 +54,7 @@ void ProcessUpdatesCommand::ModelChangingExecuteImpl(SyncSession* session) {
     if (0 == update_count) {
       if (new_timestamp > dir->last_sync_timestamp()) {
         dir->set_last_sync_timestamp(new_timestamp);
-        status->set_timestamp_dirty(true);
+        status->set_got_new_timestamp();
       }
       return;
     }
@@ -104,11 +104,14 @@ void ProcessUpdatesCommand::ModelChangingExecuteImpl(SyncSession* session) {
 
   if (new_timestamp > dir->last_sync_timestamp()) {
     dir->set_last_sync_timestamp(new_timestamp);
-    status->set_timestamp_dirty(true);
+    status->set_got_new_timestamp();
   }
 
   status->set_num_consecutive_errors(0);
-  status->set_current_sync_timestamp(dir->last_sync_timestamp());
+  // TODO(tim): Related to bug 30665, the Directory needs last sync timestamp
+  // per data type.  Until then, use UNSPECIFIED.
+  status->set_current_sync_timestamp(syncable::UNSPECIFIED,
+                                     dir->last_sync_timestamp());
   status->set_syncing(true);
   return;
 }
diff --git a/chrome/browser/sync/engine/process_updates_command.h b/chrome/browser/sync/engine/process_updates_command.h
index 6eb4333..e1d1434 100644
--- a/chrome/browser/sync/engine/process_updates_command.h
+++ b/chrome/browser/sync/engine/process_updates_command.h
@@ -25,6 +25,7 @@ namespace browser_sync {
 //
 // Postconditions - All of the verified SyncEntity data will be copied to
 //                  the server fields of the corresponding syncable entries.
+// TODO(tim): This should not be ModelChanging (bug 36592).
 class ProcessUpdatesCommand : public ModelChangingSyncerCommand {
  public:
   ProcessUpdatesCommand();
diff --git a/chrome/browser/sync/engine/resolve_conflicts_command.cc b/chrome/browser/sync/engine/resolve_conflicts_command.cc
index df74d2bd..a8fcefb 100644
--- a/chrome/browser/sync/engine/resolve_conflicts_command.cc
+++ b/chrome/browser/sync/engine/resolve_conflicts_command.cc
@@ -25,7 +25,7 @@ void ResolveConflictsCommand::ModelChangingExecuteImpl(
   if (!dir.good())
     return;
   sessions::StatusController* status = session->status_controller();
-  status->set_conflicts_resolved(resolver->ResolveConflicts(dir, status));
+  status->update_conflicts_resolved(resolver->ResolveConflicts(dir, status));
 }
 
 }  // namespace browser_sync
diff --git a/chrome/browser/sync/engine/syncer.cc b/chrome/browser/sync/engine/syncer.cc
index 9170a46..59dbae4 100755
--- a/chrome/browser/sync/engine/syncer.cc
+++ b/chrome/browser/sync/engine/syncer.cc
@@ -221,10 +221,11 @@ void Syncer::SyncShare(sessions::SyncSession* session,
           pre_conflict_resolution_closure_->Run();
         }
 
+        StatusController* status = session->status_controller();
+        status->reset_conflicts_resolved();
         ResolveConflictsCommand resolve_conflicts_command;
         resolve_conflicts_command.Execute(session);
-        StatusController* status = session->status_controller();
-        if (status->update_progress().HasConflictingUpdates())
+        if (status->HasConflictingUpdates())
           next_step = APPLY_UPDATES_TO_RESOLVE_CONFLICTS;
         else
           next_step = SYNCER_END;
@@ -232,14 +233,13 @@ void Syncer::SyncShare(sessions::SyncSession* session,
       }
       case APPLY_UPDATES_TO_RESOLVE_CONFLICTS: {
         StatusController* status = session->status_controller();
-        const ConflictProgress* progress = status->conflict_progress();
         LOG(INFO) << "Applying updates to resolve conflicts";
         ApplyUpdatesCommand apply_updates;
-        int num_conflicting_updates = progress->ConflictingItemsSize();
+        int before_conflicting_updates = status->TotalNumConflictingItems();
         apply_updates.Execute(session);
-        int post_facto_conflicting_updates = progress->ConflictingItemsSize();
-        status->set_conflicts_resolved(status->conflicts_resolved() ||
-            num_conflicting_updates > post_facto_conflicting_updates);
+        int after_conflicting_updates = status->TotalNumConflictingItems();
+        status->update_conflicts_resolved(before_conflicting_updates >
+                                          after_conflicting_updates);
         if (status->conflicts_resolved())
           next_step = RESOLVE_CONFLICTS;
         else
diff --git a/chrome/browser/sync/engine/syncer_unittest.cc b/chrome/browser/sync/engine/syncer_unittest.cc
index 46969cc..c547106 100755
--- a/chrome/browser/sync/engine/syncer_unittest.cc
+++ b/chrome/browser/sync/engine/syncer_unittest.cc
@@ -1159,7 +1159,11 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) {
   StatusController* status = session_->status_controller();
 
   // Id 3 should be in conflict now.
-  EXPECT_TRUE(1 == status->conflict_progress()->ConflictingItemsSize());
+  EXPECT_EQ(1, status->TotalNumConflictingItems());
+  {
+    sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE);
+    EXPECT_EQ(1, status->conflict_progress().ConflictingItemsSize());
+  }
 
   // These entries will be used in the second set of updates.
   mock_server_->AddUpdateDirectory(4, 0, "newer_version", 20, 10);
@@ -1172,7 +1176,12 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) {
   syncer_->SyncShare(session_.get());
   // The three items with an unresolved parent should be unapplied (3, 9, 100).
   // The name clash should also still be in conflict.
-  EXPECT_TRUE(3 ==  status->conflict_progress()->ConflictingItemsSize());
+  EXPECT_EQ(3, status->TotalNumConflictingItems());
+  {
+    sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE);
+    EXPECT_EQ(3, status->conflict_progress().ConflictingItemsSize());
+  }
+
   {
     WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__);
     // Even though it has the same name, it should work.
@@ -1260,7 +1269,11 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) {
   }
 
   EXPECT_TRUE(0 == syncer_events_.size());
-  EXPECT_TRUE(4 == status->conflict_progress()->ConflictingItemsSize());
+  EXPECT_EQ(4, status->TotalNumConflictingItems());
+  {
+    sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE);
+    EXPECT_EQ(4, status->conflict_progress().ConflictingItemsSize());
+  }
 }
 
 TEST_F(SyncerTest, CommitTimeRename) {
@@ -1938,9 +1951,7 @@ TEST_F(SyncerTest, UnappliedUpdateDuringCommit) {
   }
   syncer_->SyncShare(session_.get());
   syncer_->SyncShare(session_.get());
-  const ConflictProgress* progress =
-      session_->status_controller()->conflict_progress();
-  EXPECT_TRUE(0 == progress->ConflictingItemsSize());
+  EXPECT_TRUE(0 == session_->status_controller()->TotalNumConflictingItems());
   syncer_events_.clear();
 }
 
@@ -3432,12 +3443,14 @@ TEST(SyncerSyncProcessState, MergeSetsTest) {
   for (int i = 1; i < 7; i++) {
     id[i] = id_factory.NewServerId();
   }
-  ConflictProgress c;
+  bool is_dirty = false;
+  ConflictProgress c(&is_dirty);
   c.MergeSets(id[1], id[2]);
   c.MergeSets(id[2], id[3]);
   c.MergeSets(id[4], id[5]);
   c.MergeSets(id[5], id[6]);
   EXPECT_TRUE(6 == c.IdToConflictSetSize());
+  EXPECT_FALSE(is_dirty);
   for (int i = 1; i < 7; i++) {
     EXPECT_TRUE(NULL != c.IdToConflictSetGet(id[i]));
     EXPECT_TRUE(c.IdToConflictSetGet(id[(i & ~3) + 1]) ==
@@ -3450,10 +3463,11 @@ TEST(SyncerSyncProcessState, MergeSetsTest) {
   }
 
   // Check dupes don't cause double sets.
-  ConflictProgress identical_set;
+  ConflictProgress identical_set(&is_dirty);
   identical_set.MergeSets(id[1], id[1]);
   EXPECT_TRUE(identical_set.IdToConflictSetSize() == 1);
   EXPECT_TRUE(identical_set.IdToConflictSetGet(id[1])->size() == 1);
+  EXPECT_FALSE(is_dirty);
 }
 
 // Bug Synopsis:
diff --git a/chrome/browser/sync/engine/update_applicator.cc b/chrome/browser/sync/engine/update_applicator.cc
index 1484d5f..9f2598c 100755
--- a/chrome/browser/sync/engine/update_applicator.cc
+++ b/chrome/browser/sync/engine/update_applicator.cc
@@ -18,12 +18,16 @@ namespace browser_sync {
 
 UpdateApplicator::UpdateApplicator(ConflictResolver* resolver,
                                    const UpdateIterator& begin,
-                                   const UpdateIterator& end)
+                                   const UpdateIterator& end,
+                                   const ModelSafeRoutingInfo& routes,
+                                   ModelSafeGroup group_filter)
     : resolver_(resolver),
       begin_(begin),
       end_(end),
       pointer_(begin),
-      progress_(false) {
+      group_filter_(group_filter),
+      progress_(false),
+      routing_info_(routes) {
     size_t item_count = end - begin;
     LOG(INFO) << "UpdateApplicator created for " << item_count << " items.";
     successful_ids_.reserve(item_count);
@@ -47,12 +51,17 @@ bool UpdateApplicator::AttemptOneApplication(
     conflicting_ids_.clear();
   }
   syncable::MutableEntry entry(trans, syncable::GET_BY_HANDLE, *pointer_);
+
+  if (SkipUpdate(entry)) {
+    Advance();
+    return true;
+  }
+
   UpdateAttemptResponse updateResponse =
       SyncerUtil::AttemptToUpdateEntry(trans, &entry, resolver_);
   switch (updateResponse) {
     case SUCCESS:
-      --end_;
-      *pointer_ = *end_;
+      Advance();
       progress_ = true;
       successful_ids_.push_back(entry.Get(syncable::ID));
       break;
@@ -70,6 +79,18 @@ bool UpdateApplicator::AttemptOneApplication(
   return true;
 }
 
+void UpdateApplicator::Advance() {
+  --end_;
+  *pointer_ = *end_;
+}
+
+bool UpdateApplicator::SkipUpdate(const syncable::MutableEntry& entry) {
+  ModelSafeGroup g = GetGroupForModelType(entry.GetModelType(), routing_info_);
+  if (g != group_filter_)
+    return true;
+  return false;
+}
+
 bool UpdateApplicator::AllUpdatesApplied() const {
   return conflicting_ids_.empty() && begin_ == end_;
 }
diff --git a/chrome/browser/sync/engine/update_applicator.h b/chrome/browser/sync/engine/update_applicator.h
index 6f4e02e..7a412e8 100755
--- a/chrome/browser/sync/engine/update_applicator.h
+++ b/chrome/browser/sync/engine/update_applicator.h
@@ -16,6 +16,7 @@
 
 #include "base/basictypes.h"
 #include "base/port.h"
+#include "chrome/browser/sync/engine/model_safe_worker.h"
 #include "chrome/browser/sync/syncable/syncable.h"
 
 namespace browser_sync {
@@ -34,7 +35,9 @@ class UpdateApplicator {
 
   UpdateApplicator(ConflictResolver* resolver,
                    const UpdateIterator& begin,
-                   const UpdateIterator& end);
+                   const UpdateIterator& end,
+                   const ModelSafeRoutingInfo& routes,
+                   ModelSafeGroup group_filter);
 
   // returns true if there's more we can do.
   bool AttemptOneApplication(syncable::WriteTransaction* trans);
@@ -50,14 +53,23 @@ class UpdateApplicator {
       sessions::UpdateProgress* update_progress);
 
  private:
+  // If true, AttemptOneApplication will skip over |entry| and return true.
+  bool SkipUpdate(const syncable::MutableEntry& entry);
+
+  // Adjusts the UpdateIterator members to move ahead by one update.
+  void Advance();
+
   // Used to resolve conflicts when trying to apply updates.
   ConflictResolver* const resolver_;
 
   UpdateIterator const begin_;
   UpdateIterator end_;
   UpdateIterator pointer_;
+  ModelSafeGroup group_filter_;
   bool progress_;
 
+  const ModelSafeRoutingInfo routing_info_;
+
   // Track the result of the various items.
   std::vector<syncable::Id> conflicting_ids_;
   std::vector<syncable::Id> successful_ids_;
diff --git a/chrome/browser/sync/engine/verify_updates_command.cc b/chrome/browser/sync/engine/verify_updates_command.cc
index d84c38c..7671722 100755
--- a/chrome/browser/sync/engine/verify_updates_command.cc
+++ b/chrome/browser/sync/engine/verify_updates_command.cc
@@ -52,8 +52,10 @@ void VerifyUpdatesCommand::ExecuteImpl(sessions::SyncSession* session) {
     // Affix IDs properly prior to inputting data into server entry.
     SyncerUtil::AttemptReuniteClientTag(&trans, entry);
 
-    VerifyResult result = VerifyUpdate(&trans, entry);
-    status->mutable_update_progress()->AddVerifyResult(result, entry);
+    VerifyUpdateResult result = VerifyUpdate(&trans, entry,
+                                             session->routing_info());
+    status->GetUnrestrictedUpdateProgress(
+        result.placement)->AddVerifyResult(result.value, entry);
   }
 }
 
@@ -73,9 +75,11 @@ VerifyResult VerifyTagConsistency(const SyncEntity& entry,
 }
 }  // namespace
 
-VerifyResult VerifyUpdatesCommand::VerifyUpdate(
-    syncable::WriteTransaction* trans, const SyncEntity& entry) {
+VerifyUpdatesCommand::VerifyUpdateResult VerifyUpdatesCommand::VerifyUpdate(
+    syncable::WriteTransaction* trans, const SyncEntity& entry,
+    const ModelSafeRoutingInfo& routes) {
   syncable::Id id = entry.id();
+  VerifyUpdateResult result = {VERIFY_FAIL, GROUP_PASSIVE};
 
   const bool deleted = entry.has_deleted() && entry.deleted();
   const bool is_directory = entry.IsFolder();
@@ -83,44 +87,47 @@ VerifyResult VerifyUpdatesCommand::VerifyUpdate(
 
   if (!id.ServerKnows()) {
     LOG(ERROR) << "Illegal negative id in received updates";
-    return VERIFY_FAIL;
+    return result;
   }
   if (!entry.parent_id().ServerKnows()) {
     LOG(ERROR) << "Illegal parent id in received updates";
-    return VERIFY_FAIL;
+    return result;
   }
   {
     const std::string name = SyncerProtoUtil::NameFromSyncEntity(entry);
     if (name.empty() && !deleted) {
       LOG(ERROR) << "Zero length name in non-deleted update";
-      return VERIFY_FAIL;
+      return result;
     }
   }
 
   syncable::MutableEntry same_id(trans, GET_BY_ID, id);
-  VerifyResult result = VERIFY_UNDECIDED;
-  result = SyncerUtil::VerifyNewEntry(entry, &same_id, deleted);
+  result.value = SyncerUtil::VerifyNewEntry(entry, &same_id, deleted);
 
-  if (VERIFY_UNDECIDED == result) {
-    result = VerifyTagConsistency(entry, same_id);
+  syncable::ModelType placement_type = !deleted ? entry.GetModelType()
+    : same_id.good() ? same_id.GetModelType() : syncable::UNSPECIFIED;
+  result.placement = GetGroupForModelType(placement_type, routes);
+
+  if (VERIFY_UNDECIDED == result.value) {
+    result.value = VerifyTagConsistency(entry, same_id);
   }
 
-  if (VERIFY_UNDECIDED == result) {
+  if (VERIFY_UNDECIDED == result.value) {
     if (deleted)
-      result = VERIFY_SUCCESS;
+      result.value = VERIFY_SUCCESS;
   }
 
   // If we have an existing entry, we check here for updates that break
   // consistency rules.
-  if (VERIFY_UNDECIDED == result) {
-    result = SyncerUtil::VerifyUpdateConsistency(trans, entry, &same_id,
+  if (VERIFY_UNDECIDED == result.value) {
+    result.value = SyncerUtil::VerifyUpdateConsistency(trans, entry, &same_id,
         deleted, is_directory, model_type);
   }
 
-  if (VERIFY_UNDECIDED == result)
-    return VERIFY_SUCCESS;  // No news is good news.
-  else
-    return result;  // This might be VERIFY_SUCCESS as well
+  if (VERIFY_UNDECIDED == result.value)
+    result.value = VERIFY_SUCCESS;  // No news is good news.
+
+  return result;  // This might be VERIFY_SUCCESS as well
 }
 
 }  // namespace browser_sync
diff --git a/chrome/browser/sync/engine/verify_updates_command.h b/chrome/browser/sync/engine/verify_updates_command.h
index f4ccf2b..4529680 100644
--- a/chrome/browser/sync/engine/verify_updates_command.h
+++ b/chrome/browser/sync/engine/verify_updates_command.h
@@ -7,6 +7,7 @@
 
 #include "base/basictypes.h"
 
+#include "chrome/browser/sync/engine/model_safe_worker.h"
 #include "chrome/browser/sync/engine/syncer_command.h"
 #include "chrome/browser/sync/engine/syncproto.h"
 #include "chrome/browser/sync/engine/syncer_types.h"
@@ -27,9 +28,14 @@ class VerifyUpdatesCommand : public SyncerCommand {
   // SyncerCommand implementation.
   virtual void ExecuteImpl(sessions::SyncSession* session);
 
-  VerifyResult VerifyUpdate(syncable::WriteTransaction* trans,
-                            const SyncEntity& entry);
  private:
+  struct VerifyUpdateResult {
+    VerifyResult value;
+    ModelSafeGroup placement;
+  };
+  VerifyUpdateResult VerifyUpdate(syncable::WriteTransaction* trans,
+                                  const SyncEntity& entry,
+                                  const ModelSafeRoutingInfo& routes);
   DISALLOW_COPY_AND_ASSIGN(VerifyUpdatesCommand);
 };
 
diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc
index 6d16dc30..31626cb 100644
--- a/chrome/browser/sync/glue/sync_backend_host.cc
+++ b/chrome/browser/sync/glue/sync_backend_host.cc
@@ -60,6 +60,7 @@ void SyncBackendHost::Initialize(
   // when a new type is synced as the worker may already exist and you just
   // need to update routing_info_.
   registrar_.workers[GROUP_UI] = new UIModelWorker(frontend_loop_);
+  registrar_.workers[GROUP_PASSIVE] = new ModelSafeWorker();
 
   // Any datatypes that we want the syncer to pull down must
   // be in the routing_info map.  We set them to group passive, meaning that
@@ -121,7 +122,9 @@ void SyncBackendHost::Shutdown(bool sync_disabled) {
 
   registrar_.routing_info.clear();
   registrar_.workers[GROUP_UI] = NULL;
+  registrar_.workers[GROUP_PASSIVE] = NULL;
   registrar_.workers.erase(GROUP_UI);
+  registrar_.workers.erase(GROUP_PASSIVE);
   frontend_ = NULL;
   core_ = NULL;  // Releases reference to core_.
 }
diff --git a/chrome/browser/sync/sessions/session_state.cc b/chrome/browser/sync/sessions/session_state.cc
index b9c8572..f28a7dc 100644
--- a/chrome/browser/sync/sessions/session_state.cc
+++ b/chrome/browser/sync/sessions/session_state.cc
@@ -68,12 +68,14 @@ ConflictProgress::ConflictingItemsEnd() const {
 void ConflictProgress::AddConflictingItemById(const syncable::Id& the_id) {
   std::pair<std::set<syncable::Id>::iterator, bool> ret =
     conflicting_item_ids_.insert(the_id);
-  progress_changed_ |= ret.second;
+  if (ret.second)
+    *dirty_ = true;
 }
 
 void ConflictProgress::EraseConflictingItemById(const syncable::Id& the_id) {
   int items_erased = conflicting_item_ids_.erase(the_id);
-  progress_changed_ = items_erased != 0;
+  if (items_erased != 0)
+    *dirty_ = true;
 }
 
 void ConflictProgress::MergeSets(const syncable::Id& id1,
diff --git a/chrome/browser/sync/sessions/session_state.h b/chrome/browser/sync/sessions/session_state.h
index e4e479c..66c01fb 100644
--- a/chrome/browser/sync/sessions/session_state.h
+++ b/chrome/browser/sync/sessions/session_state.h
@@ -13,6 +13,7 @@
 #define CHROME_BROWSER_SYNC_SESSIONS_SESSION_STATE_H_
 
 #include <set>
+#include <vector>
 
 #include "base/basictypes.h"
 #include "chrome/browser/sync/engine/syncer_types.h"
@@ -28,19 +29,12 @@ namespace sessions {
 
 class UpdateProgress;
 
-// Data describing the progress made relative to the changelog on the server.
-struct ChangelogProgress {
-  ChangelogProgress() : current_sync_timestamp(0),
-                        num_server_changes_remaining(0) {}
-  int64 current_sync_timestamp;
-  int64 num_server_changes_remaining;
-};
-
 // Data pertaining to the status of an active Syncer object.
 struct SyncerStatus {
   SyncerStatus()
       : over_quota(false), invalid_store(false), syncer_stuck(false),
         syncing(false), num_successful_commits(0) {}
+
   bool over_quota;
   // True when we get such an INVALID_STORE error from the server.
   bool invalid_store;
@@ -74,12 +68,18 @@ struct ErrorCounters {
 struct SyncSessionSnapshot {
   SyncSessionSnapshot(const SyncerStatus& syncer_status,
       const ErrorCounters& errors,
-      const ChangelogProgress& changelog_progress, bool is_share_usable,
-      bool more_to_sync, bool is_silenced, int64 unsynced_count,
-      int num_conflicting_updates, bool did_commit_items)
+      int64 num_server_changes_remaining,
+      int64 max_local_timestamp,
+      bool is_share_usable,
+      bool more_to_sync,
+      bool is_silenced,
+      int64 unsynced_count,
+      int num_conflicting_updates,
+      bool did_commit_items)
       : syncer_status(syncer_status),
         errors(errors),
-        changelog_progress(changelog_progress),
+        num_server_changes_remaining(num_server_changes_remaining),
+        max_local_timestamp(max_local_timestamp),
         is_share_usable(is_share_usable),
         has_more_to_sync(more_to_sync),
         is_silenced(is_silenced),
@@ -88,7 +88,8 @@ struct SyncSessionSnapshot {
         did_commit_items(did_commit_items) {}
   const SyncerStatus syncer_status;
   const ErrorCounters errors;
-  const ChangelogProgress changelog_progress;
+  const int64 num_server_changes_remaining;
+  const int64 max_local_timestamp;
   const bool is_share_usable;
   const bool has_more_to_sync;
   const bool is_silenced;
@@ -100,7 +101,7 @@ struct SyncSessionSnapshot {
 // Tracks progress of conflicts and their resolution using conflict sets.
 class ConflictProgress {
  public:
-  ConflictProgress() : progress_changed_(false) {}
+  explicit ConflictProgress(bool* dirty_flag) : dirty_(dirty_flag) {}
   ~ConflictProgress() { CleanupSets(); }
   // Various iterators, size, and retrieval functions for conflict sets.
   IdToConflictSetMap::const_iterator IdToConflictSetBegin() const;
@@ -124,8 +125,6 @@ class ConflictProgress {
   void MergeSets(const syncable::Id& set1, const syncable::Id& set2);
   void CleanupSets();
 
-  bool progress_changed() const { return progress_changed_; }
-  void reset_progress_changed() { progress_changed_ = false; }
  private:
   // TODO(sync): move away from sets if it makes more sense.
   std::set<syncable::Id> conflicting_item_ids_;
@@ -133,8 +132,9 @@ class ConflictProgress {
   std::set<ConflictSet*> conflict_sets_;
 
   // Whether a conflicting item was added or removed since
-  // the last call to reset_progress_changed(), if any.
-  bool progress_changed_;
+  // the last call to reset_progress_changed(), if any. In practice this
+  // points to StatusController::is_dirty_.
+  bool* dirty_;
 };
 
 typedef std::pair<VerifyResult, sync_pb::SyncEntity> VerifiedUpdate;
@@ -181,25 +181,87 @@ class UpdateProgress {
   std::vector<AppliedUpdate> applied_updates_;
 };
 
-// Transient state is a physical grouping of session state that can be reset
-// while that session is in flight.  This is useful when multiple
-// SyncShare operations take place during a session.
-struct TransientState {
-  TransientState()
-    : conflict_sets_built(false),
-      conflicts_resolved(false),
-      items_committed(false),
-      timestamp_dirty(false) {}
-  UpdateProgress update_progress;
-  ClientToServerMessage commit_message;
-  ClientToServerResponse commit_response;
-  ClientToServerResponse updates_response;
-  std::vector<int64> unsynced_handles;
-  std::vector<syncable::Id> commit_ids;
+struct SyncCycleControlParameters {
+  SyncCycleControlParameters() : conflict_sets_built(false),
+                                 conflicts_resolved(false),
+                                 items_committed(false),
+                                 got_new_timestamp(false) {}
+  // Set to true by BuildAndProcessConflictSetsCommand if the RESOLVE_CONFLICTS
+  // step is needed.
   bool conflict_sets_built;
+
+  // Set to true by ResolveConflictsCommand if any forward progress was made.
   bool conflicts_resolved;
+
+  // Set to true by PostCommitMessageCommand if any commits were successful.
   bool items_committed;
-  bool timestamp_dirty;
+
+  // The server sent us updates and a newer timestamp as part of the session.
+  bool got_new_timestamp;
+};
+
+// DirtyOnWrite wraps a value such that any write operation will update a
+// specified dirty bit, which can be used to determine if a notification should
+// be sent due to state change.
+template <typename T>
+class DirtyOnWrite {
+ public:
+  explicit DirtyOnWrite(bool* dirty) : dirty_(dirty) {}
+  DirtyOnWrite(bool* dirty, const T& t) : t_(t), dirty_(dirty) {}
+  T* mutate() {
+    *dirty_ = true;
+    return &t_;
+  }
+  const T& value() const { return t_; }
+ private:
+  T t_;
+  bool* dirty_;
+};
+
+// The next 3 structures declare how all the state involved in running a sync
+// cycle is divided between global scope (applies to all model types),
+// ModelSafeGroup scope (applies to all data types in a group), and single
+// model type scope.  Within this breakdown, each struct declares which bits
+// of state are dirty-on-write and should incur dirty bit updates if changed.
+
+// Grouping of all state that applies to all model types.  Note that some
+// components of the global grouping can internally implement finer grained
+// scope control (such as OrderedCommitSet), but the top level entity is still
+// a singleton with respect to model types.
+struct AllModelTypeState {
+  explicit AllModelTypeState(bool* dirty_flag)
+      : unsynced_handles(dirty_flag),
+        syncer_status(dirty_flag),
+        error_counters(dirty_flag),
+        num_server_changes_remaining(dirty_flag, 0),
+        commit_set(ModelSafeRoutingInfo()) {}
+  // Commits for all model types are bundled together into a single message.
+  ClientToServerMessage commit_message;
+  ClientToServerResponse commit_response;
+  // We GetUpdates for all desired types at once.
+  ClientToServerResponse updates_response;
+  // Used to build the shared commit message.
+  DirtyOnWrite<std::vector<int64> > unsynced_handles;
+  DirtyOnWrite<SyncerStatus> syncer_status;
+  DirtyOnWrite<ErrorCounters> error_counters;
+  SyncCycleControlParameters control_params;
+  DirtyOnWrite<int64> num_server_changes_remaining;
+  OrderedCommitSet commit_set;
+};
+
+// Grouping of all state that applies to a single ModelSafeGroup.
+struct PerModelSafeGroupState {
+  explicit PerModelSafeGroupState(bool* dirty_flag)
+      : conflict_progress(dirty_flag) {}
+  UpdateProgress update_progress;
+  ConflictProgress conflict_progress;
+};
+
+// Grouping of all state that applies to a single ModelType.
+struct PerModelTypeState {
+  explicit PerModelTypeState(bool* dirty_flag)
+      : current_sync_timestamp(dirty_flag, 0) {}
+  DirtyOnWrite<int64> current_sync_timestamp;
 };
 
 }  // namespace sessions
diff --git a/chrome/browser/sync/sessions/status_controller.cc b/chrome/browser/sync/sessions/status_controller.cc
index 921bde7..0d112b3 100644
--- a/chrome/browser/sync/sessions/status_controller.cc
+++ b/chrome/browser/sync/sessions/status_controller.cc
@@ -9,159 +9,167 @@
 namespace browser_sync {
 namespace sessions {
 
-StatusController::StatusController()
-    : commit_set_(ModelSafeRoutingInfo()),
-      transient_(new Dirtyable<TransientState>()),
+StatusController::StatusController(const ModelSafeRoutingInfo& routes)
+    : shared_(&is_dirty_),
+      per_model_group_deleter_(&per_model_group_),
+      per_model_type_deleter_(&per_model_type_),
+      is_dirty_(false),
       group_restriction_in_effect_(false),
-      group_restriction_(GROUP_PASSIVE) {}
+      group_restriction_(GROUP_PASSIVE),
+      routing_info_(routes) {
+}
 
-// Helper to set the 'dirty' bit in the container of the field being
-// mutated.
-template <typename FieldType, typename DirtyableContainer>
-void SetAndMarkDirtyIfChanged(FieldType* old_value,
-    const FieldType& new_value, DirtyableContainer* container) {
-  if (new_value != *old_value)
-    container->set_dirty();
-  *old_value = new_value;
+bool StatusController::TestAndClearIsDirty() {
+  bool is_dirty = is_dirty_;
+  is_dirty_ = false;
+  return is_dirty;
 }
 
-template <typename T>
-bool StatusController::Dirtyable<T>::TestAndClearIsDirty() {
-  bool dirty = dirty_;
-  dirty_ = false;
-  return dirty;
+PerModelSafeGroupState* StatusController::GetOrCreateModelSafeGroupState(
+    bool restrict, ModelSafeGroup group) {
+  DCHECK(restrict == group_restriction_in_effect_) << "Group violation!";
+  if (per_model_group_.find(group) == per_model_group_.end()) {
+    PerModelSafeGroupState* state = new PerModelSafeGroupState(&is_dirty_);
+    per_model_group_[group] = state;
+    return state;
+  }
+  return per_model_group_[group];
 }
 
-bool StatusController::TestAndClearIsDirty() {
-  bool is_dirty = change_progress_.TestAndClearIsDirty() ||
-      syncer_status_.TestAndClearIsDirty() ||
-      error_counters_.TestAndClearIsDirty() ||
-      transient_->TestAndClearIsDirty() ||
-      conflict_progress_.progress_changed();
-  conflict_progress_.reset_progress_changed();
-  return is_dirty;
+PerModelTypeState* StatusController::GetOrCreateModelTypeState(
+    bool restrict, syncable::ModelType model) {
+  if (restrict) {
+    DCHECK(group_restriction_in_effect_) << "No group restriction in effect!";
+    DCHECK_EQ(group_restriction_, GetGroupForModelType(model, routing_info_));
+  }
+  if (per_model_type_.find(model) == per_model_type_.end()) {
+    PerModelTypeState* state = new PerModelTypeState(&is_dirty_);
+    per_model_type_[model] = state;
+    return state;
+  }
+  return per_model_type_[model];
 }
+
 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,
-      new_value,
-      &error_counters_);
+  if (value == 0)
+    return;
+  shared_.error_counters.mutate()->num_conflicting_commits += value;
 }
 
 void StatusController::reset_num_conflicting_commits() {
-  SetAndMarkDirtyIfChanged(&error_counters_.value()->num_conflicting_commits,
-                           0, &error_counters_);
+  if (shared_.error_counters.value().num_conflicting_commits != 0)
+    shared_.error_counters.mutate()->num_conflicting_commits = 0;
 }
 
 void StatusController::set_num_consecutive_transient_error_commits(int value) {
-  SetAndMarkDirtyIfChanged(
-      &error_counters_.value()->consecutive_transient_error_commits, value,
-      &error_counters_);
+  if (shared_.error_counters.value().consecutive_transient_error_commits !=
+      value) {
+    shared_.error_counters.mutate()->consecutive_transient_error_commits =
+        value;
+  }
 }
 
 void StatusController::increment_num_consecutive_transient_error_commits_by(
     int value) {
   set_num_consecutive_transient_error_commits(
-      error_counters_.value()->consecutive_transient_error_commits + value);
+      shared_.error_counters.value().consecutive_transient_error_commits +
+      value);
 }
 
 void StatusController::set_num_consecutive_errors(int value) {
-  SetAndMarkDirtyIfChanged(&error_counters_.value()->consecutive_errors, value,
-                           &error_counters_);
+  if (shared_.error_counters.value().consecutive_errors != value)
+    shared_.error_counters.mutate()->consecutive_errors = value;
 }
 
-void StatusController::set_current_sync_timestamp(int64 current_timestamp) {
-  SetAndMarkDirtyIfChanged(&change_progress_.value()->current_sync_timestamp,
-                           current_timestamp, &change_progress_);
+void StatusController::set_got_new_timestamp() {
+  shared_.control_params.got_new_timestamp = true;
+}
+
+void StatusController::set_current_sync_timestamp(syncable::ModelType model,
+                                                  int64 current_timestamp) {
+  PerModelTypeState* state = GetOrCreateModelTypeState(false, model);
+  if (current_timestamp > state->current_sync_timestamp.value())
+    *(state->current_sync_timestamp.mutate()) = current_timestamp;
 }
 
 void StatusController::set_num_server_changes_remaining(
     int64 changes_remaining) {
-  SetAndMarkDirtyIfChanged(
-      &change_progress_.value()->num_server_changes_remaining,
-      changes_remaining, &change_progress_);
+  if (shared_.num_server_changes_remaining.value() != changes_remaining)
+    *(shared_.num_server_changes_remaining.mutate()) = changes_remaining;
 }
 
 void StatusController::set_over_quota(bool over_quota) {
-  SetAndMarkDirtyIfChanged(&syncer_status_.value()->over_quota, over_quota,
-                           &syncer_status_);
+  if (shared_.syncer_status.value().over_quota != over_quota)
+    shared_.syncer_status.mutate()->over_quota = over_quota;
 }
 
 void StatusController::set_invalid_store(bool invalid_store) {
-  SetAndMarkDirtyIfChanged(&syncer_status_.value()->invalid_store,
-                           invalid_store, &syncer_status_);
+  if (shared_.syncer_status.value().invalid_store != invalid_store)
+    shared_.syncer_status.mutate()->invalid_store = invalid_store;
 }
 
 void StatusController::set_syncer_stuck(bool syncer_stuck) {
-  SetAndMarkDirtyIfChanged(&syncer_status_.value()->syncer_stuck, syncer_stuck,
-                           &syncer_status_);
+  if (shared_.syncer_status.value().syncer_stuck != syncer_stuck)
+    shared_.syncer_status.mutate()->syncer_stuck = syncer_stuck;
 }
 
 void StatusController::set_syncing(bool syncing) {
-  SetAndMarkDirtyIfChanged(&syncer_status_.value()->syncing, syncing,
-                           &syncer_status_);
+  if (shared_.syncer_status.value().syncing != syncing)
+    shared_.syncer_status.mutate()->syncing = syncing;
 }
 
 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_);
+  if (shared_.syncer_status.value().num_successful_bookmark_commits != value)
+    shared_.syncer_status.mutate()->num_successful_bookmark_commits = value;
 }
 
 void StatusController::set_unsynced_handles(
     const std::vector<int64>& unsynced_handles) {
-  SetAndMarkDirtyIfChanged(&transient_->value()->unsynced_handles,
-                           unsynced_handles, transient_.get());
+  if (!operator==(unsynced_handles, shared_.unsynced_handles.value())) {
+    *(shared_.unsynced_handles.mutate()) = unsynced_handles;
+  }
 }
 
 void StatusController::increment_num_consecutive_errors() {
-  set_num_consecutive_errors(error_counters_.value()->consecutive_errors + 1);
+  set_num_consecutive_errors(
+      shared_.error_counters.value().consecutive_errors + 1);
 }
 
-
 void StatusController::increment_num_consecutive_errors_by(int value) {
-  set_num_consecutive_errors(error_counters_.value()->consecutive_errors +
-                             value);
+  set_num_consecutive_errors(
+      shared_.error_counters.value().consecutive_errors + value);
 }
 
 void StatusController::increment_num_successful_bookmark_commits() {
   set_num_successful_bookmark_commits(
-      syncer_status_.value()->num_successful_bookmark_commits + 1);
+      shared_.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);
+  shared_.syncer_status.mutate()->num_successful_commits++;
 }
 
-// These setters don't affect the dirtiness of TransientState.
 void StatusController::set_commit_set(const OrderedCommitSet& commit_set) {
   DCHECK(!group_restriction_in_effect_);
-  commit_set_ = commit_set;
+  shared_.commit_set = commit_set;
 }
 
-void StatusController::set_conflict_sets_built(bool built) {
-  transient_->value()->conflict_sets_built = built;
+void StatusController::update_conflict_sets_built(bool built) {
+  shared_.control_params.conflict_sets_built |= built;
 }
-void StatusController::set_conflicts_resolved(bool resolved) {
-  transient_->value()->conflicts_resolved = resolved;
+void StatusController::update_conflicts_resolved(bool resolved) {
+  shared_.control_params.conflict_sets_built |= resolved;
 }
-void StatusController::set_items_committed(bool items_committed) {
-  transient_->value()->items_committed = items_committed;
+void StatusController::reset_conflicts_resolved() {
+  shared_.control_params.conflicts_resolved = false;
 }
-void StatusController::set_timestamp_dirty(bool dirty) {
-  transient_->value()->timestamp_dirty = dirty;
+void StatusController::set_items_committed() {
+  shared_.control_params.items_committed = true;
 }
 
 // Returns the number of updates received from the sync server.
 int64 StatusController::CountUpdates() const {
-  const ClientToServerResponse& updates =
-      transient_->value()->updates_response;
+  const ClientToServerResponse& updates = shared_.updates_response;
   if (updates.has_get_updates()) {
     return updates.get_updates().entries().size();
   } else {
@@ -171,9 +179,44 @@ int64 StatusController::CountUpdates() const {
 
 bool StatusController::CurrentCommitIdProjectionHasIndex(size_t index) {
   OrderedCommitSet::Projection proj =
-      commit_set_.GetCommitIdProjection(group_restriction_);
+      shared_.commit_set.GetCommitIdProjection(group_restriction_);
   return std::binary_search(proj.begin(), proj.end(), index);
 }
 
+int64 StatusController::ComputeMaxLocalTimestamp() const {
+  std::map<syncable::ModelType, PerModelTypeState*>::const_iterator it =
+      per_model_type_.begin();
+  int64 max_timestamp = 0;
+  for (; it != per_model_type_.end(); ++it) {
+    if (it->second->current_sync_timestamp.value() > max_timestamp)
+      max_timestamp = it->second->current_sync_timestamp.value();
+  }
+  return max_timestamp;
+}
+
+bool StatusController::HasConflictingUpdates() const {
+  DCHECK(!group_restriction_in_effect_)
+      << "HasConflictingUpdates applies to all ModelSafeGroups";
+  std::map<ModelSafeGroup, PerModelSafeGroupState*>::const_iterator it =
+    per_model_group_.begin();
+  for (; it != per_model_group_.end(); ++it) {
+    if (it->second->update_progress.HasConflictingUpdates())
+      return true;
+  }
+  return false;
+}
+
+int StatusController::TotalNumConflictingItems() const {
+  DCHECK(!group_restriction_in_effect_)
+      << "TotalNumConflictingItems applies to all ModelSafeGroups";
+  std::map<ModelSafeGroup, PerModelSafeGroupState*>::const_iterator it =
+    per_model_group_.begin();
+  int sum = 0;
+  for (; it != per_model_group_.end(); ++it) {
+    sum += it->second->conflict_progress.ConflictingItemsSize();
+  }
+  return sum;
+}
+
 }  // namespace sessions
 }  // namespace browser_sync
diff --git a/chrome/browser/sync/sessions/status_controller.h b/chrome/browser/sync/sessions/status_controller.h
index 40752f6..1169b2e 100644
--- a/chrome/browser/sync/sessions/status_controller.h
+++ b/chrome/browser/sync/sessions/status_controller.h
@@ -4,16 +4,37 @@
 
 // StatusController handles all counter and status related number crunching and
 // state tracking on behalf of a SyncSession.  It 'controls' the model data
-// defined in session_state.h.  It can track if changes occur to certain parts
-// of state so that various parts of the sync engine can avoid broadcasting
-// notifications if no changes occurred.  It also separates transient state
-// from long-lived SyncSession state for explicitness and to facilitate
-// resetting transient state.
+// defined in session_state.h.  The most important feature of StatusController
+// is the ScopedModelSafetyRestriction. When one of these is active, the
+// underlying data set exposed via accessors is swapped out to the appropriate
+// set for the restricted ModelSafeGroup behind the scenes.  For example, if
+// GROUP_UI is set, then accessors such as conflict_progress() and commit_ids()
+// are implicitly restricted to returning only data pertaining to GROUP_UI.
+// You can see which parts of status fall into this "restricted" category, the
+// global "shared" category for all model types, or the single per-model type
+// category by looking at the struct declarations in session_state.h.
+// If these accessors are invoked without a restriction in place, this is a
+// violation and will cause debug assertions to surface improper use of the API
+// in development.  Likewise for invocation of "shared" accessors when a
+// restriction is in place; for safety's sake, an assertion will fire.
+//
+// NOTE: There is no concurrent access protection provided by this class. It
+// assumes one single thread is accessing this class for each unique
+// ModelSafeGroup, and also only one single thread (in practice, the
+// SyncerThread) responsible for all "shared" access when no restriction is in
+// place. Thus, every bit of data is to be accessed mutually exclusively with
+// respect to threads.
+//
+// StatusController can also track if changes occur to certain parts of state
+// so that various parts of the sync engine can avoid broadcasting
+// notifications if no changes occurred.
 
 #ifndef CHROME_BROWSER_SYNC_SESSIONS_STATUS_CONTROLLER_H_
 #define CHROME_BROWSER_SYNC_SESSIONS_STATUS_CONTROLLER_H_
 
-#include "base/scoped_ptr.h"
+#include <map>
+
+#include "base/stl_util-inl.h"
 #include "chrome/browser/sync/sessions/ordered_commit_set.h"
 #include "chrome/browser/sync/sessions/session_state.h"
 
@@ -22,92 +43,128 @@ namespace sessions {
 
 class StatusController {
  public:
-  StatusController();
+  explicit StatusController(const ModelSafeRoutingInfo& routes);
 
   // Returns true if some portion of the session state has changed (is dirty)
   // since it was created or was last reset.
   bool TestAndClearIsDirty();
 
-  ConflictProgress const* conflict_progress() const {
-    return &conflict_progress_;
+  // Progress counters.
+  const ConflictProgress& conflict_progress() {
+    return GetOrCreateModelSafeGroupState(true,
+        group_restriction_)->conflict_progress;
   }
   ConflictProgress* mutable_conflict_progress() {
-    return &conflict_progress_;
+    return &GetOrCreateModelSafeGroupState(true,
+        group_restriction_)->conflict_progress;
   }
   const UpdateProgress& update_progress() {
-    return transient_->value()->update_progress;
+    return GetOrCreateModelSafeGroupState(true,
+        group_restriction_)->update_progress;
   }
   UpdateProgress* mutable_update_progress() {
-    return &transient_->value()->update_progress;
+    return &GetOrCreateModelSafeGroupState(true,
+        group_restriction_)->update_progress;
+  }
+  // Some unrestricted, non-ModelChangingSyncerCommand commands need to store
+  // meta information about updates.
+  UpdateProgress* GetUnrestrictedUpdateProgress(ModelSafeGroup group) {
+    return &GetOrCreateModelSafeGroupState(false, group)->update_progress;
   }
+
+  // ClientToServer messages.
   ClientToServerMessage* mutable_commit_message() {
-    return &transient_->value()->commit_message;
+    return &shared_.commit_message;
   }
   const ClientToServerResponse& commit_response() const {
-    return transient_->value()->commit_response;
+    return shared_.commit_response;
   }
   ClientToServerResponse* mutable_commit_response() {
-    return &transient_->value()->commit_response;
+    return &shared_.commit_response;
   }
   const ClientToServerResponse& updates_response() {
-    return transient_->value()->updates_response;
+    return shared_.updates_response;
   }
   ClientToServerResponse* mutable_updates_response() {
-    return &transient_->value()->updates_response;
+    return &shared_.updates_response;
   }
+
+  // Errors and SyncerStatus.
   const ErrorCounters& error_counters() const {
-    return error_counters_.value();
+    return shared_.error_counters.value();
   }
   const SyncerStatus& syncer_status() const {
-    return syncer_status_.value();
+    return shared_.syncer_status.value();
   }
-  const ChangelogProgress& change_progress() const {
-    return change_progress_.value();
+
+  // Changelog related state.
+  int64 num_server_changes_remaining() const {
+    return shared_.num_server_changes_remaining.value();
   }
+  // Aggregate max over all data type timestamps, used for UI reporting.
+  int64 ComputeMaxLocalTimestamp() const;
+
+  // Commit path data.
   const std::vector<syncable::Id>& commit_ids() const {
     DCHECK(!group_restriction_in_effect_) << "Group restriction in effect!";
-    return commit_set_.GetAllCommitIds();
+    return shared_.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_);
+    return shared_.commit_set.GetCommitIdProjection(group_restriction_);
   }
   const syncable::Id& GetCommitIdAt(size_t index) {
     DCHECK(CurrentCommitIdProjectionHasIndex(index));
-    return commit_set_.GetCommitIdAt(index);
+    return shared_.commit_set.GetCommitIdAt(index);
   }
   const syncable::ModelType GetCommitIdModelTypeAt(size_t index) {
     DCHECK(CurrentCommitIdProjectionHasIndex(index));
-    return commit_set_.GetModelTypeAt(index);
+    return shared_.commit_set.GetModelTypeAt(index);
   }
   const std::vector<int64>& unsynced_handles() const {
-    return transient_->value()->unsynced_handles;
+    DCHECK(!group_restriction_in_effect_)
+        << "unsynced_handles is unrestricted.";
+    return shared_.unsynced_handles.value();
   }
+
+  // Control parameters for sync cycles.
   bool conflict_sets_built() const {
-    return transient_->value()->conflict_sets_built;
+    return shared_.control_params.conflict_sets_built;
   }
   bool conflicts_resolved() const {
-    return transient_->value()->conflicts_resolved;
+    return shared_.control_params.conflicts_resolved;
   }
-  bool timestamp_dirty() const {
-    return transient_->value()->timestamp_dirty;
+  bool got_new_timestamp() const {
+    return shared_.control_params.got_new_timestamp;
   }
   bool did_commit_items() const {
-    return transient_->value()->items_committed;
+    return shared_.control_params.items_committed;
   }
 
+  // If a GetUpdates for any data type resulted in downloading an update that
+  // is in conflict, this method returns true.
+  bool HasConflictingUpdates() const;
+
+  // Aggregate sum of ConflictingItemSize() over all ConflictProgress objects
+  // (one for each ModelSafeGroup currently in-use).
+  int TotalNumConflictingItems() const;
+
   // 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();
+    return shared_.commit_set.HasBookmarkCommitId();
   }
 
   bool got_zero_updates() const { return CountUpdates() == 0; }
 
+  ModelSafeGroup group_restriction() const {
+    return group_restriction_;
+  }
+
   // A toolbelt full of methods for updating counters and flags.
   void increment_num_conflicting_commits_by(int value);
   void reset_num_conflicting_commits();
@@ -116,23 +173,24 @@ class StatusController {
   void set_num_consecutive_errors(int value);
   void increment_num_consecutive_errors();
   void increment_num_consecutive_errors_by(int value);
-  void set_current_sync_timestamp(int64 current_timestamp);
+  void set_got_new_timestamp();
+  void set_current_sync_timestamp(syncable::ModelType model,
+                                  int64 current_timestamp);
   void set_num_server_changes_remaining(int64 changes_remaining);
   void set_over_quota(bool over_quota);
   void set_invalid_store(bool invalid_store);
   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_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);
+  void update_conflict_sets_built(bool built);
+  void update_conflicts_resolved(bool resolved);
+  void reset_conflicts_resolved();
+  void set_items_committed();
 
  private:
   friend class ScopedModelSafeGroupRestriction;
@@ -141,47 +199,56 @@ class StatusController {
   // 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
-  // that some part of that object changed.
-  template <typename T>
-  class Dirtyable {
-   public:
-    Dirtyable() : dirty_(false) {}
-    void set_dirty() { dirty_ = true; }
-    bool TestAndClearIsDirty();
-    T* value() { return &t_; }
-    const T& value() const { return t_; }
-   private:
-    T t_;
-    bool dirty_;
-  };
-
-  OrderedCommitSet commit_set_;
-
-  // Various pieces of state we track dirtiness of.
-  Dirtyable<ChangelogProgress> change_progress_;
-  Dirtyable<SyncerStatus> syncer_status_;
-  Dirtyable<ErrorCounters> error_counters_;
-
-  // 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_;
+  // Helper to lazily create objects for per-ModelSafeGroup state.
+  PerModelSafeGroupState* GetOrCreateModelSafeGroupState(bool restrict,
+                                                         ModelSafeGroup group);
+  // Helper to lazily create objects for per-model type state.
+  PerModelTypeState* GetOrCreateModelTypeState(bool restrict,
+                                               syncable::ModelType model);
+
+  AllModelTypeState shared_;
+  std::map<ModelSafeGroup, PerModelSafeGroupState*> per_model_group_;
+  std::map<syncable::ModelType, PerModelTypeState*> per_model_type_;
+
+  STLValueDeleter<std::map<ModelSafeGroup, PerModelSafeGroupState*> >
+      per_model_group_deleter_;
+  STLValueDeleter<std::map<syncable::ModelType, PerModelTypeState*> >
+      per_model_type_deleter_;
+
+  // Set to true if any DirtyOnWrite pieces of state we maintain are changed.
+  // Reset to false by TestAndClearIsDirty.
+  bool is_dirty_;
 
   // 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_;
 
+  const ModelSafeRoutingInfo routing_info_;
+
   DISALLOW_COPY_AND_ASSIGN(StatusController);
 };
 
+// A utility to restrict access to only those parts of the given
+// StatusController that pertain to the specified ModelSafeGroup.
+class ScopedModelSafeGroupRestriction {
+ public:
+  ScopedModelSafeGroupRestriction(StatusController* to_restrict,
+                                  ModelSafeGroup restriction)
+      : status_(to_restrict) {
+    DCHECK(!status_->group_restriction_in_effect_);
+    status_->group_restriction_ = restriction;
+    status_->group_restriction_in_effect_ = true;
+  }
+  ~ScopedModelSafeGroupRestriction() {
+    DCHECK(status_->group_restriction_in_effect_);
+    status_->group_restriction_in_effect_ = false;
+  }
+ private:
+  StatusController* status_;
+  DISALLOW_COPY_AND_ASSIGN(ScopedModelSafeGroupRestriction);
+};
+
 }
 }
 
diff --git a/chrome/browser/sync/sessions/status_controller_unittest.cc b/chrome/browser/sync/sessions/status_controller_unittest.cc
index 95ee36d..24c4025 100644
--- a/chrome/browser/sync/sessions/status_controller_unittest.cc
+++ b/chrome/browser/sync/sessions/status_controller_unittest.cc
@@ -3,15 +3,23 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/sync/sessions/sync_session.h"
+#include "chrome/test/sync/engine/test_id_factory.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace browser_sync {
 namespace sessions {
 
-typedef testing::Test StatusControllerTest;
+class StatusControllerTest : public testing::Test {
+ public:
+  virtual void SetUp() {
+    routes_[syncable::BOOKMARKS] = GROUP_UI;
+  }
+ protected:
+  ModelSafeRoutingInfo routes_;
+};
 
 TEST_F(StatusControllerTest, GetsDirty) {
-  StatusController status;
+  StatusController status(routes_);
   status.increment_num_conflicting_commits_by(1);
   EXPECT_TRUE(status.TestAndClearIsDirty());
   EXPECT_FALSE(status.TestAndClearIsDirty());  // Test that it actually resets.
@@ -39,8 +47,11 @@ TEST_F(StatusControllerTest, GetsDirty) {
   status.increment_num_consecutive_errors_by(0);
   EXPECT_FALSE(status.TestAndClearIsDirty());
 
-  status.set_current_sync_timestamp(100);
-  EXPECT_TRUE(status.TestAndClearIsDirty());
+  {
+    ScopedModelSafeGroupRestriction r(&status, GROUP_UI);
+    status.set_current_sync_timestamp(syncable::BOOKMARKS, 100);
+    EXPECT_TRUE(status.TestAndClearIsDirty());
+  }
 
   status.set_num_server_changes_remaining(30);
   EXPECT_TRUE(status.TestAndClearIsDirty());
@@ -70,6 +81,12 @@ TEST_F(StatusControllerTest, GetsDirty) {
   status.increment_num_successful_commits();
   EXPECT_TRUE(status.TestAndClearIsDirty());
 
+  {
+    ScopedModelSafeGroupRestriction r(&status, GROUP_UI);
+    status.mutable_conflict_progress()->AddConflictingItemById(syncable::Id());
+  }
+  EXPECT_TRUE(status.TestAndClearIsDirty());
+
   std::vector<int64> v;
   v.push_back(1);
   status.set_unsynced_handles(v);
@@ -81,19 +98,18 @@ TEST_F(StatusControllerTest, GetsDirty) {
 }
 
 TEST_F(StatusControllerTest, StaysClean) {
-  StatusController status;
-  status.set_conflict_sets_built(true);
+  StatusController status(routes_);
+  status.update_conflict_sets_built(true);
   EXPECT_FALSE(status.TestAndClearIsDirty());
-  status.set_conflicts_resolved(true);
+  status.update_conflicts_resolved(true);
   EXPECT_FALSE(status.TestAndClearIsDirty());
-  status.set_timestamp_dirty(true);
+  status.set_got_new_timestamp();
   EXPECT_FALSE(status.TestAndClearIsDirty());
-  status.set_items_committed(true);
+
+  status.set_items_committed();
   EXPECT_FALSE(status.TestAndClearIsDirty());
 
-  ModelSafeRoutingInfo routes;
-  routes[syncable::BOOKMARKS] = GROUP_UI;
-  OrderedCommitSet commits(routes);
+  OrderedCommitSet commits(routes_);
   commits.AddCommitItem(0, syncable::Id(), syncable::BOOKMARKS);
   status.set_commit_set(commits);
   EXPECT_FALSE(status.TestAndClearIsDirty());
@@ -103,7 +119,7 @@ TEST_F(StatusControllerTest, StaysClean) {
 // nature of status_controller.cc (we have had bugs in the past where a set_foo
 // method was actually setting |bar_| instead!).
 TEST_F(StatusControllerTest, ReadYourWrites) {
-  StatusController status;
+  StatusController status(routes_);
   status.increment_num_conflicting_commits_by(1);
   EXPECT_EQ(1, status.error_counters().num_conflicting_commits);
 
@@ -121,11 +137,14 @@ TEST_F(StatusControllerTest, ReadYourWrites) {
   status.increment_num_consecutive_errors_by(2);
   EXPECT_EQ(11, status.error_counters().consecutive_errors);
 
-  status.set_current_sync_timestamp(12);
-  EXPECT_EQ(12, status.change_progress().current_sync_timestamp);
+  {
+    ScopedModelSafeGroupRestriction r(&status, GROUP_UI);
+    status.set_current_sync_timestamp(syncable::BOOKMARKS, 12);
+    EXPECT_EQ(12, status.ComputeMaxLocalTimestamp());
+  }
 
   status.set_num_server_changes_remaining(13);
-  EXPECT_EQ(13, status.change_progress().num_server_changes_remaining);
+  EXPECT_EQ(13, status.num_server_changes_remaining());
 
   EXPECT_FALSE(status.syncer_status().over_quota);
   status.set_over_quota(true);
@@ -143,10 +162,9 @@ TEST_F(StatusControllerTest, ReadYourWrites) {
   status.set_syncing(true);
   EXPECT_TRUE(status.syncer_status().syncing);
 
-  status.set_num_successful_commits(14);
+  for (int i = 0; i < 14; i++)
+    status.increment_num_successful_commits();
   EXPECT_EQ(14, status.syncer_status().num_successful_commits);
-  status.increment_num_successful_commits();
-  EXPECT_EQ(15, status.syncer_status().num_successful_commits);
 
   std::vector<int64> v;
   v.push_back(16);
@@ -155,15 +173,28 @@ TEST_F(StatusControllerTest, ReadYourWrites) {
 }
 
 TEST_F(StatusControllerTest, HasConflictingUpdates) {
-  StatusController status;
-  EXPECT_FALSE(status.update_progress().HasConflictingUpdates());
-  status.mutable_update_progress()->AddAppliedUpdate(SUCCESS, syncable::Id());
-  status.mutable_update_progress()->AddAppliedUpdate(CONFLICT, syncable::Id());
-  EXPECT_TRUE(status.update_progress().HasConflictingUpdates());
+  StatusController status(routes_);
+  EXPECT_FALSE(status.HasConflictingUpdates());
+  {
+    ScopedModelSafeGroupRestriction r(&status, GROUP_UI);
+    EXPECT_FALSE(status.update_progress().HasConflictingUpdates());
+    status.mutable_update_progress()->AddAppliedUpdate(SUCCESS,
+        syncable::Id());
+    status.mutable_update_progress()->AddAppliedUpdate(CONFLICT,
+        syncable::Id());
+    EXPECT_TRUE(status.update_progress().HasConflictingUpdates());
+  }
+
+  EXPECT_TRUE(status.HasConflictingUpdates());
+
+  {
+    ScopedModelSafeGroupRestriction r(&status, GROUP_PASSIVE);
+    EXPECT_FALSE(status.update_progress().HasConflictingUpdates());
+  }
 }
 
 TEST_F(StatusControllerTest, CountUpdates) {
-  StatusController status;
+  StatusController status(routes_);
   EXPECT_EQ(0, status.CountUpdates());
   EXPECT_TRUE(status.got_zero_updates());
   ClientToServerResponse* response(status.mutable_updates_response());
@@ -174,5 +205,46 @@ TEST_F(StatusControllerTest, CountUpdates) {
   EXPECT_FALSE(status.got_zero_updates());
 }
 
+// Test TotalNumConflictingItems
+TEST_F(StatusControllerTest, TotalNumConflictingItems) {
+  StatusController status(routes_);
+  TestIdFactory f;
+  {
+    ScopedModelSafeGroupRestriction r(&status, GROUP_UI);
+    status.mutable_conflict_progress()->AddConflictingItemById(f.NewLocalId());
+    status.mutable_conflict_progress()->AddConflictingItemById(f.NewLocalId());
+    EXPECT_EQ(2, status.conflict_progress().ConflictingItemsSize());
+  }
+  EXPECT_EQ(2, status.TotalNumConflictingItems());
+  {
+    ScopedModelSafeGroupRestriction r(&status, GROUP_DB);
+    EXPECT_EQ(0, status.conflict_progress().ConflictingItemsSize());
+    status.mutable_conflict_progress()->AddConflictingItemById(f.NewLocalId());
+    status.mutable_conflict_progress()->AddConflictingItemById(f.NewLocalId());
+    EXPECT_EQ(2, status.conflict_progress().ConflictingItemsSize());
+  }
+  EXPECT_EQ(4, status.TotalNumConflictingItems());
+}
+
+// Basic test that non group-restricted state accessors don't cause violations.
+TEST_F(StatusControllerTest, Unrestricted) {
+  StatusController status(routes_);
+  status.GetUnrestrictedUpdateProgress(
+      GROUP_UI)->SuccessfullyAppliedUpdateCount();
+  status.mutable_commit_message();
+  status.commit_response();
+  status.mutable_commit_response();
+  status.updates_response();
+  status.mutable_updates_response();
+  status.error_counters();
+  status.syncer_status();
+  status.num_server_changes_remaining();
+  status.ComputeMaxLocalTimestamp();
+  status.commit_ids();
+  status.HasBookmarkCommitActivity();
+  status.got_zero_updates();
+  status.group_restriction();
+}
+
 }  // namespace sessions
 }  // namespace browser_sync
diff --git a/chrome/browser/sync/sessions/sync_session.cc b/chrome/browser/sync/sessions/sync_session.cc
index a863a5e..6453a4c 100644
--- a/chrome/browser/sync/sessions/sync_session.cc
+++ b/chrome/browser/sync/sessions/sync_session.cc
@@ -18,17 +18,7 @@ SyncSession::SyncSession(SyncSessionContext* context, Delegate* delegate)
       const_cast<std::vector<ModelSafeWorker*>*>(&workers_));
   context_->registrar()->GetModelSafeRoutingInfo(
       const_cast<ModelSafeRoutingInfo*>(&routing_info_));
-
-  // TODO(tim): Use ModelSafeRoutingInfo to silo parts of the session status by
-  // ModelSafeGroup;
-  // e.g. have a map<class, commit_ids>, map<class, ConflictProgress> etc.
-  // The routing will be used to map multiple model types into the right silo.
-  // The routing info can't change throughout a session, so we're assured that
-  // (for example) commit_ids for syncable::AUTOFILL items that were being
-  // processed as part of the GROUP_PASSIVE run (because they weren't being
-  // synced) *continue* to be for this whole session, even though the
-  // ModelSafeWorkerRegistrar may be configured to route syncable::AUTOFILL to
-  // GROUP_DB now.
+  status_controller_.reset(new StatusController(routing_info_));
 }
 
 SyncSessionSnapshot SyncSession::TakeSnapshot() const {
@@ -39,15 +29,16 @@ SyncSessionSnapshot SyncSession::TakeSnapshot() const {
 
   const bool is_share_usable = dir->initial_sync_ended();
   return SyncSessionSnapshot(
-      status_controller_.syncer_status(),
-      status_controller_.error_counters(),
-      status_controller_.change_progress(),
+      status_controller_->syncer_status(),
+      status_controller_->error_counters(),
+      status_controller_->num_server_changes_remaining(),
+      status_controller_->ComputeMaxLocalTimestamp(),
       is_share_usable,
       HasMoreToSync(),
       delegate_->IsSyncingCurrentlySilenced(),
-      status_controller_.unsynced_handles().size(),
-      status_controller_.conflict_progress()->ConflictingItemsSize(),
-      status_controller_.did_commit_items());
+      status_controller_->unsynced_handles().size(),
+      status_controller_->TotalNumConflictingItems(),
+      status_controller_->did_commit_items());
 }
 
 sync_pb::GetUpdatesCallerInfo::GetUpdatesSource
@@ -58,15 +49,15 @@ sync_pb::GetUpdatesCallerInfo::GetUpdatesSource
 }
 
 bool SyncSession::HasMoreToSync() const {
-  const StatusController& status = status_controller_;
-  return ((status.commit_ids().size() < status.unsynced_handles().size()) &&
-      status.syncer_status().num_successful_commits > 0) ||
-      status.conflict_sets_built() ||
-      status.conflicts_resolved() ||
+  const StatusController* status = status_controller_.get();
+  return ((status->commit_ids().size() < status->unsynced_handles().size()) &&
+      status->syncer_status().num_successful_commits > 0) ||
+      status->conflict_sets_built() ||
+      status->conflicts_resolved() ||
       // Or, we have conflicting updates, but we're making progress on
       // resolving them...
-      !status.got_zero_updates() ||
-      status.timestamp_dirty();
+      !status->got_zero_updates() ||
+      status->got_new_timestamp();
 }
 
 }  // namespace sessions
diff --git a/chrome/browser/sync/sessions/sync_session.h b/chrome/browser/sync/sessions/sync_session.h
index 5e49cec..6f29227 100644
--- a/chrome/browser/sync/sessions/sync_session.h
+++ b/chrome/browser/sync/sessions/sync_session.h
@@ -82,7 +82,7 @@ class SyncSession {
   SyncSessionContext* context() { return context_; }
   Delegate* delegate() { return delegate_; }
   syncable::WriteTransaction* write_transaction() { return write_transaction_; }
-  StatusController* status_controller() { return &status_controller_; }
+  StatusController* status_controller() { return status_controller_.get(); }
 
   const ExtensionsActivityMonitor::Records& extensions_activity() const {
     return extensions_activity_;
@@ -124,7 +124,7 @@ class SyncSession {
   Delegate* delegate_;
 
   // Our controller for various status and error counters.
-  StatusController status_controller_;
+  scoped_ptr<StatusController> status_controller_;
 
   // The set of active ModelSafeWorkers for the duration of this session.
   const std::vector<ModelSafeWorker*> workers_;
@@ -155,27 +155,6 @@ class ScopedSetSessionWriteTransaction {
   DISALLOW_COPY_AND_ASSIGN(ScopedSetSessionWriteTransaction);
 };
 
-// A utility to restrict access to only those parts of the given SyncSession
-// that pertain to the specified ModelSafeGroup.  See
-// SyncSession::ModelSafetyRestriction.
-class ScopedModelSafeGroupRestriction {
- public:
-  ScopedModelSafeGroupRestriction(SyncSession* to_restrict,
-                               ModelSafeGroup restriction)
-      : session_(to_restrict) {
-    DCHECK(!session_->status_controller()->group_restriction_in_effect_);
-    session_->status_controller()->group_restriction_ = restriction;
-    session_->status_controller()->group_restriction_in_effect_ = true;
-  }
-  ~ScopedModelSafeGroupRestriction() {
-    DCHECK(session_->status_controller()->group_restriction_in_effect_);
-    session_->status_controller()->group_restriction_in_effect_ = false;
-  }
- private:
-  SyncSession* session_;
-  DISALLOW_COPY_AND_ASSIGN(ScopedModelSafeGroupRestriction);
-};
-
 }  // namespace sessions
 }  // namespace browser_sync
 
diff --git a/chrome/browser/sync/sessions/sync_session_unittest.cc b/chrome/browser/sync/sessions/sync_session_unittest.cc
index 7e82f83..85c19cd 100755
--- a/chrome/browser/sync/sessions/sync_session_unittest.cc
+++ b/chrome/browser/sync/sessions/sync_session_unittest.cc
@@ -129,7 +129,7 @@ TEST_F(SyncSessionTest, MoreToSyncIfUnsyncedGreaterThanCommitted) {
 TEST_F(SyncSessionTest, MoreToSyncIfConflictSetsBuilt) {
   // If we built conflict sets, then we need to loop back and try
   // to get updates & commit again.
-  status()->set_conflict_sets_built(true);
+  status()->update_conflict_sets_built(true);
   EXPECT_TRUE(session_->HasMoreToSync());
 }
 
@@ -149,7 +149,7 @@ TEST_F(SyncSessionTest, MoreToSyncIfConflictsResolved) {
   // Conflict resolution happens after get updates and commit,
   // so we need to loop back and get updates / commit again now
   // that we have made forward progress.
-  status()->set_conflicts_resolved(true);
+  status()->update_conflicts_resolved(true);
   EXPECT_TRUE(session_->HasMoreToSync());
 }
 
@@ -157,8 +157,8 @@ TEST_F(SyncSessionTest, MoreToSyncIfTimestampDirty) {
   // If there are more changes on the server that weren't processed during this
   // GetUpdates request, the client should send another GetUpdates request and
   // use new_timestamp as the from_timestamp value within GetUpdatesMessage.
-  status()->set_timestamp_dirty(true);
-  status()->set_conflicts_resolved(true);
+  status()->set_got_new_timestamp();
+  status()->update_conflicts_resolved(true);
   EXPECT_TRUE(session_->HasMoreToSync());
 }