sync: Per-type update application

This change moves the update application functionality from the
ApplyUpdatesAndResolveConflictsCommand into the
SyncDirectoryUpdateHandler class.  This change will allow us to
implement update application differently for different types.

Because update application happens on the model threads, the
ApplyUpdatesAndResolveConflictsCommand had to be aware of
ModelSafeRoutingInfo, ModelSafeWorkers, and other concepts intended to
hide threading details.  The new code takes a different approach.  It
hides the threading details specific to each type inside its
SyncDirectoryUpateHandler by initializing it with a scoped_refptr to its
associated ModelSafeWorker.

The ApplyUpdatesAndResolveConflictsCommand was the last SyncerCommand.
With its removal, we can also remove the definitions of SyncerCommand,
ModelChangingSyncerCommand and SyncerCommandTest.

BUG=278484

Review URL: https://codereview.chromium.org/72403003

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

34 files changed, 833 insertions, 1367 deletions

diff --git a/sync/engine/apply_updates_and_resolve_conflicts_command.cc b/sync/engine/apply_updates_and_resolve_conflicts_command.cc
deleted file mode 100644
index 54b0b38..0000000
--- a/sync/engine/apply_updates_and_resolve_conflicts_command.cc
+++ /dev/null
@@ -1,134 +0,0 @@
-// Copyright 2012 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 "sync/engine/apply_updates_and_resolve_conflicts_command.h"
-
-#include "base/location.h"
-#include "sync/engine/conflict_resolver.h"
-#include "sync/engine/update_applicator.h"
-#include "sync/sessions/sync_session.h"
-#include "sync/syncable/directory.h"
-#include "sync/syncable/syncable_read_transaction.h"
-#include "sync/syncable/syncable_write_transaction.h"
-
-namespace syncer {
-
-using sessions::SyncSession;
-
-ApplyUpdatesAndResolveConflictsCommand::
-    ApplyUpdatesAndResolveConflictsCommand() {}
-ApplyUpdatesAndResolveConflictsCommand::
-    ~ApplyUpdatesAndResolveConflictsCommand() {}
-
-std::set<ModelSafeGroup>
-ApplyUpdatesAndResolveConflictsCommand::GetGroupsToChange(
-    const sessions::SyncSession& session) const {
-  std::set<ModelSafeGroup> groups_with_unapplied_updates;
-
-  FullModelTypeSet server_types_with_unapplied_updates;
-  {
-    syncable::Directory* dir = session.context()->directory();
-    syncable::ReadTransaction trans(FROM_HERE, dir);
-    server_types_with_unapplied_updates =
-        dir->GetServerTypesWithUnappliedUpdates(&trans);
-  }
-
-  for (FullModelTypeSet::Iterator it =
-           server_types_with_unapplied_updates.First(); it.Good(); it.Inc()) {
-    groups_with_unapplied_updates.insert(
-        GetGroupForModelType(it.Get(), session.context()->routing_info()));
-  }
-
-  return groups_with_unapplied_updates;
-}
-
-SyncerError ApplyUpdatesAndResolveConflictsCommand::ModelChangingExecuteImpl(
-    SyncSession* session) {
-  sessions::StatusController* status = session->mutable_status_controller();
-  syncable::Directory* dir = session->context()->directory();
-  syncable::WriteTransaction trans(FROM_HERE, syncable::SYNCER, dir);
-
-  // Compute server types with unapplied updates that fall under our
-  // group restriction.
-  const FullModelTypeSet server_types_with_unapplied_updates =
-      dir->GetServerTypesWithUnappliedUpdates(&trans);
-  FullModelTypeSet server_type_restriction;
-  for (FullModelTypeSet::Iterator it =
-           server_types_with_unapplied_updates.First(); it.Good(); it.Inc()) {
-    if (GetGroupForModelType(it.Get(), session->context()->routing_info()) ==
-        status->group_restriction()) {
-      server_type_restriction.Put(it.Get());
-    }
-  }
-
-  // Don't process control type updates here.  They will be handled elsewhere.
-  FullModelTypeSet control_types = ToFullModelTypeSet(ControlTypes());
-  server_type_restriction.RemoveAll(control_types);
-
-  std::vector<int64> handles;
-  dir->GetUnappliedUpdateMetaHandles(
-      &trans, server_type_restriction, &handles);
-
-  // First set of update application passes.
-  UpdateApplicator applicator(
-      dir->GetCryptographer(&trans),
-      session->context()->routing_info(),
-      status->group_restriction());
-  applicator.AttemptApplications(&trans, handles);
-  status->increment_num_updates_applied_by(applicator.updates_applied());
-  status->increment_num_hierarchy_conflicts_by(
-      applicator.hierarchy_conflicts());
-  status->increment_num_encryption_conflicts_by(
-      applicator.encryption_conflicts());
-
-  if (applicator.simple_conflict_ids().size() != 0) {
-    // Resolve the simple conflicts we just detected.
-    ConflictResolver resolver;
-    resolver.ResolveConflicts(&trans,
-                              dir->GetCryptographer(&trans),
-                              applicator.simple_conflict_ids(),
-                              status);
-
-    // Conflict resolution sometimes results in more updates to apply.
-    handles.clear();
-    dir->GetUnappliedUpdateMetaHandles(
-        &trans, server_type_restriction, &handles);
-
-    UpdateApplicator conflict_applicator(
-        dir->GetCryptographer(&trans),
-        session->context()->routing_info(),
-        status->group_restriction());
-    conflict_applicator.AttemptApplications(&trans, handles);
-
-    // We count the number of updates from both applicator passes.
-    status->increment_num_updates_applied_by(
-        conflict_applicator.updates_applied());
-
-    // Encryption conflicts should remain unchanged by the resolution of simple
-    // conflicts.  Those can only be solved by updating our nigori key bag.
-    DCHECK_EQ(conflict_applicator.encryption_conflicts(),
-              applicator.encryption_conflicts());
-
-    // Hierarchy conflicts should also remain unchanged, for reasons that are
-    // more subtle.  Hierarchy conflicts exist when the application of a pending
-    // update from the server would make the local folder hierarchy
-    // inconsistent.  The resolution of simple conflicts could never affect the
-    // hierarchy conflicting item directly, because hierarchy conflicts are not
-    // processed by the conflict resolver.  It could, in theory, modify the
-    // local hierarchy on which hierarchy conflict detection depends.  However,
-    // the conflict resolution algorithm currently in use does not allow this.
-    DCHECK_EQ(conflict_applicator.hierarchy_conflicts(),
-              applicator.hierarchy_conflicts());
-
-    // There should be no simple conflicts remaining.  We know this because the
-    // resolver should have resolved all the conflicts we detected last time
-    // and, by the two previous assertions, that no conflicts have been
-    // downgraded from encryption or hierarchy down to simple.
-    DCHECK(conflict_applicator.simple_conflict_ids().empty());
-  }
-
-  return SYNCER_OK;
-}
-
-}  // namespace syncer
diff --git a/sync/engine/apply_updates_and_resolve_conflicts_command.h b/sync/engine/apply_updates_and_resolve_conflicts_command.h
deleted file mode 100644
index 5072f80..0000000
--- a/sync/engine/apply_updates_and_resolve_conflicts_command.h
+++ /dev/null
@@ -1,33 +0,0 @@
-// Copyright 2012 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 SYNC_ENGINE_APPLY_UPDATES_AND_RESOLVE_CONFLICTS_COMMAND_H_
-#define SYNC_ENGINE_APPLY_UPDATES_AND_RESOLVE_CONFLICTS_COMMAND_H_
-
-#include "base/compiler_specific.h"
-#include "sync/base/sync_export.h"
-#include "sync/engine/model_changing_syncer_command.h"
-
-namespace syncer {
-
-class SYNC_EXPORT_PRIVATE ApplyUpdatesAndResolveConflictsCommand
-    : public ModelChangingSyncerCommand {
- public:
-  ApplyUpdatesAndResolveConflictsCommand();
-  virtual ~ApplyUpdatesAndResolveConflictsCommand();
-
- protected:
-  // ModelChangingSyncerCommand implementation.
-  virtual std::set<ModelSafeGroup> GetGroupsToChange(
-      const sessions::SyncSession& session) const OVERRIDE;
-  virtual SyncerError ModelChangingExecuteImpl(
-      sessions::SyncSession* session) OVERRIDE;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(ApplyUpdatesAndResolveConflictsCommand);
-};
-
-}  // namespace syncer
-
-#endif  // SYNC_ENGINE_APPLY_UPDATES_AND_RESOLVE_CONFLICTS_COMMAND_H_
diff --git a/sync/engine/apply_updates_and_resolve_conflicts_command_unittest.cc b/sync/engine/apply_updates_and_resolve_conflicts_command_unittest.cc
deleted file mode 100644
index d021789..0000000
--- a/sync/engine/apply_updates_and_resolve_conflicts_command_unittest.cc
+++ /dev/null
@@ -1,415 +0,0 @@
-// Copyright 2012 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 <string>
-
-#include "base/location.h"
-#include "base/memory/scoped_ptr.h"
-#include "base/strings/stringprintf.h"
-#include "sync/engine/apply_updates_and_resolve_conflicts_command.h"
-#include "sync/engine/syncer.h"
-#include "sync/internal_api/public/test/test_entry_factory.h"
-#include "sync/protocol/bookmark_specifics.pb.h"
-#include "sync/protocol/password_specifics.pb.h"
-#include "sync/syncable/mutable_entry.h"
-#include "sync/syncable/syncable_id.h"
-#include "sync/syncable/syncable_read_transaction.h"
-#include "sync/syncable/syncable_util.h"
-#include "sync/syncable/syncable_write_transaction.h"
-#include "sync/test/engine/fake_model_worker.h"
-#include "sync/test/engine/syncer_command_test.h"
-#include "sync/test/engine/test_id_factory.h"
-#include "sync/test/fake_sync_encryption_handler.h"
-#include "sync/util/cryptographer.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace syncer {
-
-using std::string;
-using syncable::Id;
-using syncable::MutableEntry;
-using syncable::UNITTEST;
-using syncable::WriteTransaction;
-
-namespace {
-sync_pb::EntitySpecifics DefaultBookmarkSpecifics() {
-  sync_pb::EntitySpecifics result;
-  AddDefaultFieldValue(BOOKMARKS, &result);
-  return result;
-}
-} // namespace
-
-// A test fixture for tests exercising ApplyUpdatesAndResolveConflictsCommand.
-class ApplyUpdatesAndResolveConflictsCommandTest : public SyncerCommandTest {
- public:
- protected:
-  ApplyUpdatesAndResolveConflictsCommandTest() {}
-  virtual ~ApplyUpdatesAndResolveConflictsCommandTest() {}
-
-  virtual void SetUp() {
-    workers()->clear();
-    mutable_routing_info()->clear();
-    workers()->push_back(
-        make_scoped_refptr(new FakeModelWorker(GROUP_UI)));
-    workers()->push_back(
-        make_scoped_refptr(new FakeModelWorker(GROUP_PASSWORD)));
-    (*mutable_routing_info())[BOOKMARKS] = GROUP_UI;
-    (*mutable_routing_info())[PASSWORDS] = GROUP_PASSWORD;
-    (*mutable_routing_info())[NIGORI] = GROUP_PASSIVE;
-    SyncerCommandTest::SetUp();
-    entry_factory_.reset(new TestEntryFactory(directory()));
-    ExpectNoGroupsToChange(apply_updates_command_);
-  }
-
- protected:
-  DISALLOW_COPY_AND_ASSIGN(ApplyUpdatesAndResolveConflictsCommandTest);
-
-  ApplyUpdatesAndResolveConflictsCommand apply_updates_command_;
-  scoped_ptr<TestEntryFactory> entry_factory_;
-};
-
-TEST_F(ApplyUpdatesAndResolveConflictsCommandTest, Simple) {
-  string root_server_id = syncable::GetNullId().GetServerId();
-  entry_factory_->CreateUnappliedNewBookmarkItemWithParent(
-      "parent", DefaultBookmarkSpecifics(), root_server_id);
-  entry_factory_->CreateUnappliedNewBookmarkItemWithParent(
-      "child", DefaultBookmarkSpecifics(), "parent");
-
-  ExpectGroupToChange(apply_updates_command_, GROUP_UI);
-  apply_updates_command_.ExecuteImpl(session());
-
-  const sessions::StatusController& status = session()->status_controller();
-  EXPECT_EQ(0, status.num_encryption_conflicts())
-      << "Simple update shouldn't result in conflicts";
-  EXPECT_EQ(0, status.num_hierarchy_conflicts())
-      << "Simple update shouldn't result in conflicts";
-  EXPECT_EQ(2, status.num_updates_applied())
-      << "All items should have been successfully applied";
-}
-
-TEST_F(ApplyUpdatesAndResolveConflictsCommandTest,
-       UpdateWithChildrenBeforeParents) {
-  // Set a bunch of updates which are difficult to apply in the order
-  // they're received due to dependencies on other unseen items.
-  string root_server_id = syncable::GetNullId().GetServerId();
-  entry_factory_->CreateUnappliedNewBookmarkItemWithParent(
-      "a_child_created_first", DefaultBookmarkSpecifics(), "parent");
-  entry_factory_->CreateUnappliedNewBookmarkItemWithParent(
-      "x_child_created_first", DefaultBookmarkSpecifics(), "parent");
-  entry_factory_->CreateUnappliedNewBookmarkItemWithParent(
-      "parent", DefaultBookmarkSpecifics(), root_server_id);
-  entry_factory_->CreateUnappliedNewBookmarkItemWithParent(
-      "a_child_created_second", DefaultBookmarkSpecifics(), "parent");
-  entry_factory_->CreateUnappliedNewBookmarkItemWithParent(
-      "x_child_created_second", DefaultBookmarkSpecifics(), "parent");
-
-  ExpectGroupToChange(apply_updates_command_, GROUP_UI);
-  apply_updates_command_.ExecuteImpl(session());
-
-  const sessions::StatusController& status = session()->status_controller();
-  EXPECT_EQ(5, status.num_updates_applied())
-      << "All updates should have been successfully applied";
-}
-
-// Runs the ApplyUpdatesAndResolveConflictsCommand on an item that has both
-// local and remote modifications (IS_UNSYNCED and IS_UNAPPLIED_UPDATE).  We
-// expect the command to detect that this update can't be applied because it is
-// in a CONFLICT state.
-TEST_F(ApplyUpdatesAndResolveConflictsCommandTest, SimpleConflict) {
-  entry_factory_->CreateUnappliedAndUnsyncedBookmarkItem("item");
-
-  ExpectGroupToChange(apply_updates_command_, GROUP_UI);
-  apply_updates_command_.ExecuteImpl(session());
-
-  const sessions::StatusController& status = session()->status_controller();
-  EXPECT_EQ(1, status.num_server_overwrites())
-      << "Unsynced and unapplied item conflict should be resolved";
-  EXPECT_EQ(0, status.num_updates_applied())
-      << "Update should not be applied; we should override the server.";
-}
-
-// Runs the ApplyUpdatesAndResolveConflictsCommand on an item that has both
-// local and remote modifications *and* the remote modification cannot be
-// applied without violating the tree constraints.  We expect the command to
-// detect that this update can't be applied and that this situation can't be
-// resolved with the simple conflict processing logic; it is in a
-// CONFLICT_HIERARCHY state.
-TEST_F(ApplyUpdatesAndResolveConflictsCommandTest, HierarchyAndSimpleConflict) {
-  // Create a simply-conflicting item.  It will start with valid parent ids.
-  int64 handle = entry_factory_->CreateUnappliedAndUnsyncedBookmarkItem(
-      "orphaned_by_server");
-  {
-    // Manually set the SERVER_PARENT_ID to bad value.
-    // A bad parent indicates a hierarchy conflict.
-    WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry entry(&trans, syncable::GET_BY_HANDLE, handle);
-    ASSERT_TRUE(entry.good());
-
-    entry.PutServerParentId(TestIdFactory::MakeServer("bogus_parent"));
-  }
-
-  ExpectGroupToChange(apply_updates_command_, GROUP_UI);
-  apply_updates_command_.ExecuteImpl(session());
-
-  const sessions::StatusController& status = session()->status_controller();
-  EXPECT_EQ(1, status.num_hierarchy_conflicts());
-}
-
-
-// Runs the ApplyUpdatesAndResolveConflictsCommand on an item with remote
-// modifications that would create a directory loop if the update were applied.
-// We expect the command to detect that this update can't be applied because it
-// is in a CONFLICT_HIERARCHY state.
-TEST_F(ApplyUpdatesAndResolveConflictsCommandTest,
-       HierarchyConflictDirectoryLoop) {
-  // Item 'X' locally has parent of 'root'.  Server is updating it to have
-  // parent of 'Y'.
-  {
-    // Create it as a child of root node.
-    int64 handle = entry_factory_->CreateSyncedItem("X", BOOKMARKS, true);
-
-    WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry entry(&trans, syncable::GET_BY_HANDLE, handle);
-    ASSERT_TRUE(entry.good());
-
-    // Re-parent from root to "Y"
-    entry.PutServerVersion(entry_factory_->GetNextRevision());
-    entry.PutIsUnappliedUpdate(true);
-    entry.PutServerParentId(TestIdFactory::MakeServer("Y"));
-  }
-
-  // Item 'Y' is child of 'X'.
-  entry_factory_->CreateUnsyncedItem(
-      TestIdFactory::MakeServer("Y"), TestIdFactory::MakeServer("X"), "Y", true,
-      BOOKMARKS, NULL);
-
-  // If the server's update were applied, we would have X be a child of Y, and Y
-  // as a child of X.  That's a directory loop.  The UpdateApplicator should
-  // prevent the update from being applied and note that this is a hierarchy
-  // conflict.
-
-  ExpectGroupToChange(apply_updates_command_, GROUP_UI);
-  apply_updates_command_.ExecuteImpl(session());
-
-  const sessions::StatusController& status = session()->status_controller();
-
-  // This should count as a hierarchy conflict.
-  EXPECT_EQ(1, status.num_hierarchy_conflicts());
-}
-
-// Runs the ApplyUpdatesAndResolveConflictsCommand on a directory where the
-// server sent us an update to add a child to a locally deleted (and unsynced)
-// parent.  We expect the command to not apply the update and to indicate the
-// update is in a CONFLICT_HIERARCHY state.
-TEST_F(ApplyUpdatesAndResolveConflictsCommandTest,
-       HierarchyConflictDeletedParent) {
-  // Create a locally deleted parent item.
-  int64 parent_handle;
-  entry_factory_->CreateUnsyncedItem(
-      Id::CreateFromServerId("parent"), TestIdFactory::root(),
-      "parent", true, BOOKMARKS, &parent_handle);
-  {
-    WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry entry(&trans, syncable::GET_BY_HANDLE, parent_handle);
-    entry.PutIsDel(true);
-  }
-
-  // Create an incoming child from the server.
-  entry_factory_->CreateUnappliedNewItemWithParent(
-      "child", DefaultBookmarkSpecifics(), "parent");
-
-  // The server's update may seem valid to some other client, but on this client
-  // that new item's parent no longer exists.  The update should not be applied
-  // and the update applicator should indicate this is a hierarchy conflict.
-
-  ExpectGroupToChange(apply_updates_command_, GROUP_UI);
-  apply_updates_command_.ExecuteImpl(session());
-
-  const sessions::StatusController& status = session()->status_controller();
-  EXPECT_EQ(1, status.num_hierarchy_conflicts());
-}
-
-// Runs the ApplyUpdatesAndResolveConflictsCommand on a directory where the
-// server is trying to delete a folder that has a recently added (and unsynced)
-// child.  We expect the command to not apply the update because it is in a
-// CONFLICT_HIERARCHY state.
-TEST_F(ApplyUpdatesAndResolveConflictsCommandTest,
-       HierarchyConflictDeleteNonEmptyDirectory) {
-  // Create a server-deleted directory.
-  {
-    // Create it as a child of root node.
-    int64 handle = entry_factory_->CreateSyncedItem("parent", BOOKMARKS, true);
-
-    WriteTransaction trans(FROM_HERE, UNITTEST, directory());
-    MutableEntry entry(&trans, syncable::GET_BY_HANDLE, handle);
-    ASSERT_TRUE(entry.good());
-
-    // Delete it on the server.
-    entry.PutServerVersion(entry_factory_->GetNextRevision());
-    entry.PutIsUnappliedUpdate(true);
-    entry.PutServerParentId(TestIdFactory::root());
-    entry.PutServerIsDel(true);
-  }
-
-  // Create a local child of the server-deleted directory.
-  entry_factory_->CreateUnsyncedItem(
-      TestIdFactory::MakeServer("child"), TestIdFactory::MakeServer("parent"),
-      "child", false, BOOKMARKS, NULL);
-
-  // The server's request to delete the directory must be ignored, otherwise our
-  // unsynced new child would be orphaned.  This is a hierarchy conflict.
-
-  ExpectGroupToChange(apply_updates_command_, GROUP_UI);
-  apply_updates_command_.ExecuteImpl(session());
-
-  const sessions::StatusController& status = session()->status_controller();
-  // This should count as a hierarchy conflict.
-  EXPECT_EQ(1, status.num_hierarchy_conflicts());
-}
-
-// Runs the ApplyUpdatesAndResolveConflictsCommand on a server-created item that
-// has a locally unknown parent.  We expect the command to not apply the update
-// because the item is in a CONFLICT_HIERARCHY state.
-TEST_F(ApplyUpdatesAndResolveConflictsCommandTest,
-       HierarchyConflictUnknownParent) {
-  // We shouldn't be able to do anything with either of these items.
-  entry_factory_->CreateUnappliedNewItemWithParent(
-      "some_item", DefaultBookmarkSpecifics(), "unknown_parent");
-  entry_factory_->CreateUnappliedNewItemWithParent(
-      "some_other_item", DefaultBookmarkSpecifics(), "some_item");
-
-  ExpectGroupToChange(apply_updates_command_, GROUP_UI);
-  apply_updates_command_.ExecuteImpl(session());
-
-  const sessions::StatusController& status = session()->status_controller();
-  EXPECT_EQ(2, status.num_hierarchy_conflicts())
-      << "All updates with an unknown ancestors should be in conflict";
-  EXPECT_EQ(0, status.num_updates_applied())
-      << "No item with an unknown ancestor should be applied";
-}
-
-TEST_F(ApplyUpdatesAndResolveConflictsCommandTest, ItemsBothKnownAndUnknown) {
-  // See what happens when there's a mixture of good and bad updates.
-  string root_server_id = syncable::GetNullId().GetServerId();
-  entry_factory_->CreateUnappliedNewItemWithParent(
-      "first_unknown_item", DefaultBookmarkSpecifics(), "unknown_parent");
-  entry_factory_->CreateUnappliedNewItemWithParent(
-      "first_known_item", DefaultBookmarkSpecifics(), root_server_id);
-  entry_factory_->CreateUnappliedNewItemWithParent(
-      "second_unknown_item", DefaultBookmarkSpecifics(), "unknown_parent");
-  entry_factory_->CreateUnappliedNewItemWithParent(
-      "second_known_item", DefaultBookmarkSpecifics(), "first_known_item");
-  entry_factory_->CreateUnappliedNewItemWithParent(
-      "third_known_item", DefaultBookmarkSpecifics(), "fourth_known_item");
-  entry_factory_->CreateUnappliedNewItemWithParent(
-      "fourth_known_item", DefaultBookmarkSpecifics(), root_server_id);
-
-  ExpectGroupToChange(apply_updates_command_, GROUP_UI);
-  apply_updates_command_.ExecuteImpl(session());
-
-  const sessions::StatusController& status = session()->status_controller();
-  EXPECT_EQ(2, status.num_hierarchy_conflicts())
-      << "The updates with unknown ancestors should be in conflict";
-  EXPECT_EQ(4, status.num_updates_applied())
-      << "The updates with known ancestors should be successfully applied";
-}
-
-TEST_F(ApplyUpdatesAndResolveConflictsCommandTest, DecryptablePassword) {
-  // Decryptable password updates should be applied.
-  Cryptographer* cryptographer;
-  {
-    // Storing the cryptographer separately is bad, but for this test we
-    // know it's safe.
-    syncable::ReadTransaction trans(FROM_HERE, directory());
-    cryptographer = directory()->GetCryptographer(&trans);
-  }
-
-  KeyParams params = {"localhost", "dummy", "foobar"};
-  cryptographer->AddKey(params);
-
-  sync_pb::EntitySpecifics specifics;
-  sync_pb::PasswordSpecificsData data;
-  data.set_origin("http://example.com");
-
-  cryptographer->Encrypt(data,
-                         specifics.mutable_password()->mutable_encrypted());
-  entry_factory_->CreateUnappliedNewItem("item", specifics, false);
-
-  ExpectGroupToChange(apply_updates_command_, GROUP_PASSWORD);
-  apply_updates_command_.ExecuteImpl(session());
-
-  const sessions::StatusController& status = session()->status_controller();
-  EXPECT_EQ(1, status.num_updates_applied())
-      << "The updates that can be decrypted should be applied";
-}
-
-TEST_F(ApplyUpdatesAndResolveConflictsCommandTest, UndecryptableData) {
-  // Undecryptable updates should not be applied.
-  sync_pb::EntitySpecifics encrypted_bookmark;
-  encrypted_bookmark.mutable_encrypted();
-  AddDefaultFieldValue(BOOKMARKS, &encrypted_bookmark);
-  string root_server_id = syncable::GetNullId().GetServerId();
-  entry_factory_->CreateUnappliedNewItemWithParent(
-      "folder", encrypted_bookmark, root_server_id);
-  entry_factory_->CreateUnappliedNewItem("item2", encrypted_bookmark, false);
-  sync_pb::EntitySpecifics encrypted_password;
-  encrypted_password.mutable_password();
-  entry_factory_->CreateUnappliedNewItem("item3", encrypted_password, false);
-
-  ExpectGroupsToChange(apply_updates_command_, GROUP_UI, GROUP_PASSWORD);
-  apply_updates_command_.ExecuteImpl(session());
-
-  const sessions::StatusController& status = session()->status_controller();
-  EXPECT_EQ(3, status.num_encryption_conflicts())
-      << "Updates that can't be decrypted should be in encryption conflict";
-  EXPECT_EQ(0, status.num_updates_applied())
-      << "No update that can't be decrypted should be applied";
-}
-
-TEST_F(ApplyUpdatesAndResolveConflictsCommandTest, SomeUndecryptablePassword) {
-  Cryptographer* cryptographer;
-  // Only decryptable password updates should be applied.
-  {
-    sync_pb::EntitySpecifics specifics;
-    sync_pb::PasswordSpecificsData data;
-    data.set_origin("http://example.com/1");
-    {
-      syncable::ReadTransaction trans(FROM_HERE, directory());
-      cryptographer = directory()->GetCryptographer(&trans);
-
-      KeyParams params = {"localhost", "dummy", "foobar"};
-      cryptographer->AddKey(params);
-
-      cryptographer->Encrypt(data,
-          specifics.mutable_password()->mutable_encrypted());
-    }
-    entry_factory_->CreateUnappliedNewItem("item1", specifics, false);
-  }
-  {
-    // Create a new cryptographer, independent of the one in the session.
-    Cryptographer other_cryptographer(cryptographer->encryptor());
-    KeyParams params = {"localhost", "dummy", "bazqux"};
-    other_cryptographer.AddKey(params);
-
-    sync_pb::EntitySpecifics specifics;
-    sync_pb::PasswordSpecificsData data;
-    data.set_origin("http://example.com/2");
-
-    other_cryptographer.Encrypt(data,
-        specifics.mutable_password()->mutable_encrypted());
-    entry_factory_->CreateUnappliedNewItem("item2", specifics, false);
-  }
-
-  ExpectGroupToChange(apply_updates_command_, GROUP_PASSWORD);
-  apply_updates_command_.ExecuteImpl(session());
-
-  const sessions::StatusController& status = session()->status_controller();
-  EXPECT_EQ(1, status.num_encryption_conflicts())
-      << "The updates that can't be decrypted should be in encryption "
-      << "conflict";
-  EXPECT_EQ(1, status.num_updates_applied())
-      << "The undecryptable password update shouldn't be applied";
-}
-
-}  // namespace syncer
diff --git a/sync/engine/download_unittest.cc b/sync/engine/download_unittest.cc
index 9336893..8fd08d1 100644
--- a/sync/engine/download_unittest.cc
+++ b/sync/engine/download_unittest.cc
@@ -13,6 +13,7 @@
 #include "sync/sessions/nudge_tracker.h"
 #include "sync/sessions/status_controller.h"
 #include "sync/syncable/directory.h"
+#include "sync/test/engine/fake_model_worker.h"
 #include "sync/test/engine/test_directory_setter_upper.h"
 #include "sync/test/sessions/mock_debug_info_getter.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -31,9 +32,9 @@ class DownloadUpdatesTest : public ::testing::Test {
   virtual void SetUp() {
     dir_maker_.SetUp();
 
-    AddUpdateHandler(AUTOFILL);
-    AddUpdateHandler(BOOKMARKS);
-    AddUpdateHandler(PREFERENCES);
+    AddUpdateHandler(AUTOFILL, GROUP_DB);
+    AddUpdateHandler(BOOKMARKS, GROUP_UI);
+    AddUpdateHandler(PREFERENCES, GROUP_UI);
   }
 
   virtual void TearDown() {
@@ -58,11 +59,12 @@ class DownloadUpdatesTest : public ::testing::Test {
   }
 
  private:
-  void AddUpdateHandler(ModelType type) {
+  void AddUpdateHandler(ModelType type, ModelSafeGroup group) {
     DCHECK(directory());
-    update_handler_map_.insert(
-        std::make_pair(type,
-                       new SyncDirectoryUpdateHandler(directory(), type)));
+    scoped_refptr<ModelSafeWorker> worker = new FakeModelWorker(group);
+    SyncDirectoryUpdateHandler* handler =
+        new SyncDirectoryUpdateHandler(directory(), type, worker);
+    update_handler_map_.insert(std::make_pair(type, handler));
   }
 
   base::MessageLoop loop_;  // Needed for directory init.
diff --git a/sync/engine/get_commit_ids.cc b/sync/engine/get_commit_ids.cc
index 3a87383..42faf21 100644
--- a/sync/engine/get_commit_ids.cc
+++ b/sync/engine/get_commit_ids.cc
@@ -73,9 +73,8 @@ void GetCommitIdsForType(
 
   // We filter out all unready entries from the set of unsynced handles. This
   // new set of ready and unsynced items is then what we use to determine what
-  // is a candidate for commit.  The caller of this SyncerCommand is responsible
-  // for ensuring that no throttled types are included among the
-  // requested_types.
+  // is a candidate for commit.  The caller is responsible for ensuring that no
+  // throttled types are included among the requested_types.
   FilterUnreadyEntries(trans,
                        ModelTypeSet(type),
                        encrypted_types,
diff --git a/sync/engine/model_changing_syncer_command.cc b/sync/engine/model_changing_syncer_command.cc
deleted file mode 100644
index a793626..0000000
--- a/sync/engine/model_changing_syncer_command.cc
+++ /dev/null
@@ -1,51 +0,0 @@
-// Copyright (c) 2012 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 "sync/engine/model_changing_syncer_command.h"
-
-#include "base/basictypes.h"
-#include "base/bind.h"
-#include "base/bind_helpers.h"
-#include "sync/sessions/status_controller.h"
-#include "sync/sessions/sync_session.h"
-
-namespace syncer {
-
-SyncerError ModelChangingSyncerCommand::ExecuteImpl(
-    sessions::SyncSession* session) {
-  work_session_ = session;
-  SyncerError result = SYNCER_OK;
-
-  const std::set<ModelSafeGroup>& groups_to_change =
-      GetGroupsToChange(*work_session_);
-  for (size_t i = 0; i < session->context()->workers().size(); ++i) {
-    ModelSafeWorker* worker = session->context()->workers()[i].get();
-    ModelSafeGroup group = worker->GetModelSafeGroup();
-    // Skip workers whose group isn't active.
-    if (groups_to_change.count(group) == 0u) {
-      DVLOG(2) << "Skipping worker for group "
-               << ModelSafeGroupToString(group);
-      continue;
-    }
-
-    sessions::StatusController* status =
-        work_session_->mutable_status_controller();
-    sessions::ScopedModelSafeGroupRestriction r(status, group);
-    WorkCallback c = base::Bind(
-        &ModelChangingSyncerCommand::StartChangingModel,
-        // We wait until the callback is executed. So it is safe to use
-        // unretained.
-        base::Unretained(this));
-
-    SyncerError this_worker_result = worker->DoWorkAndWaitUntilDone(c);
-    // TODO(rlarocque): Figure out a better way to deal with errors from
-    // multiple models at once.  See also: crbug.com/109422.
-    if (this_worker_result != SYNCER_OK)
-      result = this_worker_result;
-  }
-
-  return result;
-}
-
-}  // namespace syncer
diff --git a/sync/engine/model_changing_syncer_command.h b/sync/engine/model_changing_syncer_command.h
deleted file mode 100644
index f9ffe37..0000000
--- a/sync/engine/model_changing_syncer_command.h
+++ /dev/null
@@ -1,76 +0,0 @@
-// Copyright 2012 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 SYNC_ENGINE_MODEL_CHANGING_SYNCER_COMMAND_H_
-#define SYNC_ENGINE_MODEL_CHANGING_SYNCER_COMMAND_H_
-
-#include "base/compiler_specific.h"
-#include "sync/base/sync_export.h"
-#include "sync/engine/syncer_command.h"
-#include "sync/internal_api/public/engine/model_safe_worker.h"
-
-namespace syncer {
-namespace sessions {
-class SyncSession;
-}
-
-// An abstract SyncerCommand which dispatches its Execute step to the
-// model-safe worker thread.  Classes derived from ModelChangingSyncerCommand
-// instead of SyncerCommand must implement ModelChangingExecuteImpl instead of
-// ExecuteImpl, but otherwise, the contract is the same.
-//
-// A command should derive from ModelChangingSyncerCommand instead of
-// SyncerCommand whenever the operation might change any client-visible
-// fields on any syncable::Entry.  If the operation involves creating a
-// WriteTransaction, this is a sign that ModelChangingSyncerCommand is likely
-// necessary.
-class SYNC_EXPORT_PRIVATE ModelChangingSyncerCommand : public SyncerCommand {
- public:
-  ModelChangingSyncerCommand() : work_session_(NULL) { }
-  virtual ~ModelChangingSyncerCommand() { }
-
-  // SyncerCommand implementation. Sets work_session to session.
-  virtual SyncerError ExecuteImpl(sessions::SyncSession* session) OVERRIDE;
-
-  // Wrapper so implementations don't worry about storing work_session.
-  SyncerError StartChangingModel() {
-    return ModelChangingExecuteImpl(work_session_);
-  }
-
-  std::set<ModelSafeGroup> GetGroupsToChangeForTest(
-      const sessions::SyncSession& session) const {
-    return GetGroupsToChange(session);
-  }
-
- protected:
-  // This should return the set of groups in |session| that need to be
-  // changed.  The returned set should be a subset of
-  // session.GetEnabledGroups().  Subclasses can guarantee this either
-  // by calling one of the session.GetEnabledGroups*() functions and
-  // filtering that, or using GetGroupForModelType() (which handles
-  // top-level/unspecified nodes) to project from model types to
-  // groups.
-  virtual std::set<ModelSafeGroup> GetGroupsToChange(
-      const sessions::SyncSession& session) const = 0;
-
-  // 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 SyncerError ModelChangingExecuteImpl(
-      sessions::SyncSession* session) = 0;
-
- private:
-  // ExecuteImpl is expected to be run by SyncerCommand to set work_session.
-  // StartChangingModel is called to start this command running.
-  // Implementations will implement ModelChangingExecuteImpl and not
-  // worry about storing the session or setting it. They are given work_session.
-  sessions::SyncSession* work_session_;
-
-  DISALLOW_COPY_AND_ASSIGN(ModelChangingSyncerCommand);
-};
-
-}  // namespace syncer
-
-#endif  // SYNC_ENGINE_MODEL_CHANGING_SYNCER_COMMAND_H_
diff --git a/sync/engine/model_changing_syncer_command_unittest.cc b/sync/engine/model_changing_syncer_command_unittest.cc
deleted file mode 100644
index 71345ef..0000000
--- a/sync/engine/model_changing_syncer_command_unittest.cc
+++ /dev/null
@@ -1,88 +0,0 @@
-// Copyright (c) 2012 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 "base/basictypes.h"
-#include "base/compiler_specific.h"
-#include "base/memory/ref_counted.h"
-#include "sync/engine/model_changing_syncer_command.h"
-#include "sync/internal_api/public/base/model_type.h"
-#include "sync/sessions/sync_session.h"
-#include "sync/test/engine/fake_model_worker.h"
-#include "sync/test/engine/syncer_command_test.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace syncer {
-
-namespace {
-
-class FakeModelChangingSyncerCommand : public ModelChangingSyncerCommand {
- public:
-  FakeModelChangingSyncerCommand() {}
-  virtual ~FakeModelChangingSyncerCommand() {}
-
-  const std::set<ModelSafeGroup>& changed_groups() const {
-    return changed_groups_;
-  }
-
- protected:
-  virtual std::set<ModelSafeGroup> GetGroupsToChange(
-      const sessions::SyncSession& session) const OVERRIDE {
-    // This command doesn't actually make changes, so the empty set might be an
-    // appropriate response.  That would not be very interesting, so instead we
-    // return the set of groups with active types.
-    std::set<ModelSafeGroup> enabled_groups;
-    const ModelSafeRoutingInfo& routing_info =
-        session.context()->routing_info();
-    for (ModelSafeRoutingInfo::const_iterator it = routing_info.begin();
-         it != routing_info.end(); ++it) {
-      enabled_groups.insert(it->second);
-    }
-    enabled_groups.insert(GROUP_PASSIVE);
-    return enabled_groups;
-  }
-
-  virtual SyncerError ModelChangingExecuteImpl(
-      sessions::SyncSession* session) OVERRIDE {
-    changed_groups_.insert(session->status_controller().group_restriction());
-    return SYNCER_OK;
-  }
-
- private:
-  std::set<ModelSafeGroup> changed_groups_;
-
-  DISALLOW_COPY_AND_ASSIGN(FakeModelChangingSyncerCommand);
-};
-
-class ModelChangingSyncerCommandTest : public SyncerCommandTest {
- protected:
-  ModelChangingSyncerCommandTest() {}
-  virtual ~ModelChangingSyncerCommandTest() {}
-
-  virtual void SetUp() {
-    workers()->push_back(
-        make_scoped_refptr(new FakeModelWorker(GROUP_UI)));
-    workers()->push_back(
-        make_scoped_refptr(new FakeModelWorker(GROUP_PASSWORD)));
-    (*mutable_routing_info())[BOOKMARKS] = GROUP_UI;
-    (*mutable_routing_info())[PASSWORDS] = GROUP_PASSWORD;
-    SyncerCommandTest::SetUp();
-  }
-
-  FakeModelChangingSyncerCommand command_;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(ModelChangingSyncerCommandTest);
-};
-
-TEST_F(ModelChangingSyncerCommandTest, Basic) {
-  ExpectGroupsToChange(command_, GROUP_UI, GROUP_PASSWORD, GROUP_PASSIVE);
-  EXPECT_TRUE(command_.changed_groups().empty());
-  command_.ExecuteImpl(session());
-  EXPECT_EQ(command_.GetGroupsToChangeForTest(*session()),
-            command_.changed_groups());
-}
-
-}  // namespace
-
-}  // namespace syncer
diff --git a/sync/engine/sync_directory_update_handler.cc b/sync/engine/sync_directory_update_handler.cc
index 0d7953b..1a9bd1e 100644
--- a/sync/engine/sync_directory_update_handler.cc
+++ b/sync/engine/sync_directory_update_handler.cc
@@ -4,19 +4,25 @@
 
 #include "sync/engine/sync_directory_update_handler.h"
 
+#include "sync/engine/conflict_resolver.h"
 #include "sync/engine/process_updates_util.h"
+#include "sync/engine/update_applicator.h"
 #include "sync/sessions/status_controller.h"
 #include "sync/syncable/directory.h"
 #include "sync/syncable/syncable_model_neutral_write_transaction.h"
+#include "sync/syncable/syncable_write_transaction.h"
 
 namespace syncer {
 
 using syncable::SYNCER;
 
 SyncDirectoryUpdateHandler::SyncDirectoryUpdateHandler(
-    syncable::Directory* dir, ModelType type)
+    syncable::Directory* dir,
+    ModelType type,
+    scoped_refptr<ModelSafeWorker> worker)
   : dir_(dir),
-    type_(type) {}
+    type_(type),
+    worker_(worker) {}
 
 SyncDirectoryUpdateHandler::~SyncDirectoryUpdateHandler() {}
 
@@ -34,6 +40,91 @@ void SyncDirectoryUpdateHandler::ProcessGetUpdatesResponse(
   UpdateProgressMarker(progress_marker);
 }
 
+void SyncDirectoryUpdateHandler::ApplyUpdates(
+    sessions::StatusController* status) {
+  if (IsControlType(type_)) {
+    return;  // We don't process control types here.
+  }
+
+  if (!dir_->TypeHasUnappliedUpdates(type_)) {
+    return;  // No work to do.  Skip this type.
+  }
+
+  WorkCallback c = base::Bind(
+      &SyncDirectoryUpdateHandler::ApplyUpdatesImpl,
+      // We wait until the callback is executed.  We can safely use Unretained.
+      base::Unretained(this),
+      base::Unretained(status));
+  worker_->DoWorkAndWaitUntilDone(c);
+}
+
+SyncerError SyncDirectoryUpdateHandler::ApplyUpdatesImpl(
+    sessions::StatusController* status) {
+  syncable::WriteTransaction trans(FROM_HERE, syncable::SYNCER, dir_);
+
+  std::vector<int64> handles;
+  dir_->GetUnappliedUpdateMetaHandles(
+      &trans,
+      FullModelTypeSet(type_),
+      &handles);
+
+  // First set of update application passes.
+  UpdateApplicator applicator(dir_->GetCryptographer(&trans));
+  applicator.AttemptApplications(&trans, handles);
+  status->increment_num_updates_applied_by(applicator.updates_applied());
+  status->increment_num_hierarchy_conflicts_by(
+      applicator.hierarchy_conflicts());
+  status->increment_num_encryption_conflicts_by(
+      applicator.encryption_conflicts());
+
+  if (applicator.simple_conflict_ids().size() != 0) {
+    // Resolve the simple conflicts we just detected.
+    ConflictResolver resolver;
+    resolver.ResolveConflicts(&trans,
+                              dir_->GetCryptographer(&trans),
+                              applicator.simple_conflict_ids(),
+                              status);
+
+    // Conflict resolution sometimes results in more updates to apply.
+    handles.clear();
+    dir_->GetUnappliedUpdateMetaHandles(
+        &trans,
+        FullModelTypeSet(type_),
+        &handles);
+
+    UpdateApplicator conflict_applicator(dir_->GetCryptographer(&trans));
+    conflict_applicator.AttemptApplications(&trans, handles);
+
+    // We count the number of updates from both applicator passes.
+    status->increment_num_updates_applied_by(
+        conflict_applicator.updates_applied());
+
+    // Encryption conflicts should remain unchanged by the resolution of simple
+    // conflicts.  Those can only be solved by updating our nigori key bag.
+    DCHECK_EQ(conflict_applicator.encryption_conflicts(),
+              applicator.encryption_conflicts());
+
+    // Hierarchy conflicts should also remain unchanged, for reasons that are
+    // more subtle.  Hierarchy conflicts exist when the application of a pending
+    // update from the server would make the local folder hierarchy
+    // inconsistent.  The resolution of simple conflicts could never affect the
+    // hierarchy conflicting item directly, because hierarchy conflicts are not
+    // processed by the conflict resolver.  It could, in theory, modify the
+    // local hierarchy on which hierarchy conflict detection depends.  However,
+    // the conflict resolution algorithm currently in use does not allow this.
+    DCHECK_EQ(conflict_applicator.hierarchy_conflicts(),
+              applicator.hierarchy_conflicts());
+
+    // There should be no simple conflicts remaining.  We know this because the
+    // resolver should have resolved all the conflicts we detected last time
+    // and, by the two previous assertions, that no conflicts have been
+    // downgraded from encryption or hierarchy down to simple.
+    DCHECK(conflict_applicator.simple_conflict_ids().empty());
+  }
+
+  return SYNCER_OK;
+}
+
 void SyncDirectoryUpdateHandler::UpdateSyncEntities(
     syncable::ModelNeutralWriteTransaction* trans,
     const SyncEntityList& applicable_updates,
diff --git a/sync/engine/sync_directory_update_handler.h b/sync/engine/sync_directory_update_handler.h
index 38256a4..ea4d7914 100644
--- a/sync/engine/sync_directory_update_handler.h
+++ b/sync/engine/sync_directory_update_handler.h
@@ -8,9 +8,11 @@
 #include <map>
 
 #include "base/basictypes.h"
+#include "base/memory/ref_counted.h"
 #include "sync/base/sync_export.h"
 #include "sync/engine/process_updates_util.h"
 #include "sync/internal_api/public/base/model_type.h"
+#include "sync/internal_api/public/util/syncer_error.h"
 
 namespace sync_pb {
 class DataTypeProgressMarker;
@@ -27,6 +29,8 @@ namespace syncable {
 class Directory;
 }
 
+class ModelSafeWorker;
+
 // This class represents the syncable::Directory's processes for requesting and
 // processing updates from the sync server.
 //
@@ -35,7 +39,9 @@ class Directory;
 // It can also process a set of received SyncEntities and store their data.
 class SYNC_EXPORT_PRIVATE SyncDirectoryUpdateHandler {
  public:
-  SyncDirectoryUpdateHandler(syncable::Directory* dir, ModelType type);
+  SyncDirectoryUpdateHandler(syncable::Directory* dir,
+                             ModelType type,
+                             scoped_refptr<ModelSafeWorker> worker);
   ~SyncDirectoryUpdateHandler();
 
   // Fills the given parameter with the stored progress marker for this type.
@@ -53,8 +59,13 @@ class SYNC_EXPORT_PRIVATE SyncDirectoryUpdateHandler {
       const SyncEntityList& applicable_updates,
       sessions::StatusController* status);
 
+  // If there are updates to apply, apply them on the proper thread.
+  // Delegates to ApplyUpdatesImpl().
+  void ApplyUpdates(sessions::StatusController* status);
+
  private:
-  friend class SyncDirectoryUpdateHandlerTest;
+  friend class SyncDirectoryUpdateHandlerApplyUpdateTest;
+  friend class SyncDirectoryUpdateHandlerProcessUpdateTest;
 
   // Processes the given SyncEntities and stores their data in the directory.
   // Their types must match this update handler's type.
@@ -68,8 +79,12 @@ class SYNC_EXPORT_PRIVATE SyncDirectoryUpdateHandler {
   void UpdateProgressMarker(
       const sync_pb::DataTypeProgressMarker& progress_marker);
 
+  // Skips all checks and goes straight to applying the updates.
+  SyncerError ApplyUpdatesImpl(sessions::StatusController* status);
+
   syncable::Directory* dir_;
   ModelType type_;
+  scoped_refptr<ModelSafeWorker> worker_;
 
   DISALLOW_COPY_AND_ASSIGN(SyncDirectoryUpdateHandler);
 };
diff --git a/sync/engine/sync_directory_update_handler_unittest.cc b/sync/engine/sync_directory_update_handler_unittest.cc
index 53589b8..86d447e 100644
--- a/sync/engine/sync_directory_update_handler_unittest.cc
+++ b/sync/engine/sync_directory_update_handler_unittest.cc
@@ -6,16 +6,22 @@
 
 #include "base/compiler_specific.h"
 #include "base/message_loop/message_loop.h"
+#include "base/stl_util.h"
 #include "sync/engine/syncer_proto_util.h"
 #include "sync/internal_api/public/base/model_type.h"
+#include "sync/internal_api/public/test/test_entry_factory.h"
 #include "sync/protocol/sync.pb.h"
 #include "sync/sessions/status_controller.h"
 #include "sync/syncable/directory.h"
 #include "sync/syncable/entry.h"
+#include "sync/syncable/mutable_entry.h"
 #include "sync/syncable/syncable_model_neutral_write_transaction.h"
 #include "sync/syncable/syncable_proto_util.h"
 #include "sync/syncable/syncable_read_transaction.h"
+#include "sync/syncable/syncable_write_transaction.h"
+#include "sync/test/engine/fake_model_worker.h"
 #include "sync/test/engine/test_directory_setter_upper.h"
+#include "sync/test/engine/test_id_factory.h"
 #include "sync/test/engine/test_syncable_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -23,8 +29,19 @@ namespace syncer {
 
 using syncable::UNITTEST;
 
-class SyncDirectoryUpdateHandlerTest : public ::testing::Test {
+// A test harness for tests that focus on processing updates.
+//
+// Update processing is what occurs when we first download updates.  It converts
+// the received protobuf message into information in the syncable::Directory.
+// Any invalid or redundant updates will be dropped at this point.
+class SyncDirectoryUpdateHandlerProcessUpdateTest : public ::testing::Test {
  public:
+  SyncDirectoryUpdateHandlerProcessUpdateTest()
+      : ui_worker_(new FakeModelWorker(GROUP_UI)) {
+  }
+
+  virtual ~SyncDirectoryUpdateHandlerProcessUpdateTest() {}
+
   virtual void SetUp() OVERRIDE {
     dir_maker_.SetUp();
   }
@@ -54,12 +71,18 @@ class SyncDirectoryUpdateHandlerTest : public ::testing::Test {
       SyncDirectoryUpdateHandler* handler,
       const sync_pb::DataTypeProgressMarker& progress);
 
+  scoped_refptr<FakeModelWorker> ui_worker() {
+    return ui_worker_;
+  }
+
  private:
   base::MessageLoop loop_;  // Needed to initialize the directory.
   TestDirectorySetterUpper dir_maker_;
+  scoped_refptr<FakeModelWorker> ui_worker_;
 };
 
-scoped_ptr<sync_pb::SyncEntity> SyncDirectoryUpdateHandlerTest::CreateUpdate(
+scoped_ptr<sync_pb::SyncEntity>
+SyncDirectoryUpdateHandlerProcessUpdateTest::CreateUpdate(
     const std::string& id,
     const std::string& parent,
     const ModelType& type) {
@@ -73,7 +96,7 @@ scoped_ptr<sync_pb::SyncEntity> SyncDirectoryUpdateHandlerTest::CreateUpdate(
   return e.Pass();
 }
 
-void SyncDirectoryUpdateHandlerTest::UpdateSyncEntities(
+void SyncDirectoryUpdateHandlerProcessUpdateTest::UpdateSyncEntities(
     SyncDirectoryUpdateHandler* handler,
     const SyncEntityList& applicable_updates,
     sessions::StatusController* status) {
@@ -81,7 +104,7 @@ void SyncDirectoryUpdateHandlerTest::UpdateSyncEntities(
   handler->UpdateSyncEntities(&trans, applicable_updates, status);
 }
 
-void SyncDirectoryUpdateHandlerTest::UpdateProgressMarkers(
+void SyncDirectoryUpdateHandlerProcessUpdateTest::UpdateProgressMarkers(
     SyncDirectoryUpdateHandler* handler,
     const sync_pb::DataTypeProgressMarker& progress) {
   handler->UpdateProgressMarker(progress);
@@ -90,8 +113,8 @@ void SyncDirectoryUpdateHandlerTest::UpdateProgressMarkers(
 static const char kCacheGuid[] = "IrcjZ2jyzHDV9Io4+zKcXQ==";
 
 // Test that the bookmark tag is set on newly downloaded items.
-TEST_F(SyncDirectoryUpdateHandlerTest, NewBookmarkTag) {
-  SyncDirectoryUpdateHandler handler(dir(), BOOKMARKS);
+TEST_F(SyncDirectoryUpdateHandlerProcessUpdateTest, NewBookmarkTag) {
+  SyncDirectoryUpdateHandler handler(dir(), BOOKMARKS, ui_worker());
   sync_pb::GetUpdatesResponse gu_response;
   sessions::StatusController status;
 
@@ -127,8 +150,9 @@ TEST_F(SyncDirectoryUpdateHandlerTest, NewBookmarkTag) {
 }
 
 // Test the receipt of a type root node.
-TEST_F(SyncDirectoryUpdateHandlerTest, ReceiveServerCreatedBookmarkFolders) {
-  SyncDirectoryUpdateHandler handler(dir(), BOOKMARKS);
+TEST_F(SyncDirectoryUpdateHandlerProcessUpdateTest,
+       ReceiveServerCreatedBookmarkFolders) {
+  SyncDirectoryUpdateHandler handler(dir(), BOOKMARKS, ui_worker());
   sync_pb::GetUpdatesResponse gu_response;
   sessions::StatusController status;
 
@@ -161,15 +185,15 @@ TEST_F(SyncDirectoryUpdateHandlerTest, ReceiveServerCreatedBookmarkFolders) {
 }
 
 // Test the receipt of a non-bookmark item.
-TEST_F(SyncDirectoryUpdateHandlerTest, ReceiveNonBookmarkItem) {
-  SyncDirectoryUpdateHandler handler(dir(), AUTOFILL);
+TEST_F(SyncDirectoryUpdateHandlerProcessUpdateTest, ReceiveNonBookmarkItem) {
+  SyncDirectoryUpdateHandler handler(dir(), PREFERENCES, ui_worker());
   sync_pb::GetUpdatesResponse gu_response;
   sessions::StatusController status;
 
   std::string root = syncable::GetNullId().GetServerId();
   syncable::Id server_id = syncable::Id::CreateFromServerId("xyz");
   scoped_ptr<sync_pb::SyncEntity> e =
-      CreateUpdate(SyncableIdToProto(server_id), root, AUTOFILL);
+      CreateUpdate(SyncableIdToProto(server_id), root, PREFERENCES);
   e->set_server_defined_unique_tag("9PGRuKdX5sHyGMB17CvYTXuC43I=");
 
   // Add it to the applicable updates list.
@@ -192,8 +216,8 @@ TEST_F(SyncDirectoryUpdateHandlerTest, ReceiveNonBookmarkItem) {
 }
 
 // Tests the setting of progress markers.
-TEST_F(SyncDirectoryUpdateHandlerTest, ProcessNewProgressMarkers) {
-  SyncDirectoryUpdateHandler handler(dir(), BOOKMARKS);
+TEST_F(SyncDirectoryUpdateHandlerProcessUpdateTest, ProcessNewProgressMarkers) {
+  SyncDirectoryUpdateHandler handler(dir(), BOOKMARKS, ui_worker());
 
   sync_pb::DataTypeProgressMarker progress;
   progress.set_data_type_id(GetSpecificsFieldNumberFromModelType(BOOKMARKS));
@@ -208,4 +232,595 @@ TEST_F(SyncDirectoryUpdateHandlerTest, ProcessNewProgressMarkers) {
   EXPECT_EQ(progress.data_type_id(), saved.data_type_id());
 }
 
+// A test harness for tests that focus on applying updates.
+//
+// Update application is performed when we want to take updates that were
+// previously downloaded, processed, and stored in our syncable::Directory
+// and use them to update our local state (both the Directory's local state
+// and the model's local state, though these tests focus only on the Directory's
+// local state).
+//
+// This is kept separate from the update processing test in part for historical
+// reasons, and in part because these tests may require a bit more infrastrcture
+// in the future.  Update application should happen on a different thread a lot
+// of the time so these tests may end up requiring more infrastructure than the
+// update processing tests.  Currently, we're bypassing most of those issues by
+// using FakeModelWorkers, so there's not much difference between the two test
+// harnesses.
+class SyncDirectoryUpdateHandlerApplyUpdateTest : public ::testing::Test {
+ public:
+  SyncDirectoryUpdateHandlerApplyUpdateTest()
+      : ui_worker_(new FakeModelWorker(GROUP_UI)),
+        password_worker_(new FakeModelWorker(GROUP_PASSWORD)),
+        passive_worker_(new FakeModelWorker(GROUP_PASSIVE)),
+        update_handler_map_deleter_(&update_handler_map_) {}
+
+  virtual void SetUp() OVERRIDE {
+    dir_maker_.SetUp();
+    entry_factory_.reset(new TestEntryFactory(directory()));
+
+    update_handler_map_.insert(std::make_pair(
+        BOOKMARKS,
+        new SyncDirectoryUpdateHandler(directory(), BOOKMARKS, ui_worker_)));
+    update_handler_map_.insert(std::make_pair(
+        PASSWORDS,
+        new SyncDirectoryUpdateHandler(directory(),
+                                       PASSWORDS,
+                                       password_worker_)));
+  }
+
+  virtual void TearDown() OVERRIDE {
+    dir_maker_.TearDown();
+  }
+
+ protected:
+  void ApplyBookmarkUpdates(sessions::StatusController* status) {
+    update_handler_map_[BOOKMARKS]->ApplyUpdates(status);
+  }
+
+  void ApplyPasswordUpdates(sessions::StatusController* status) {
+    update_handler_map_[PASSWORDS]->ApplyUpdates(status);
+  }
+
+  TestEntryFactory* entry_factory() {
+    return entry_factory_.get();
+  }
+
+  syncable::Directory* directory() {
+    return dir_maker_.directory();
+  }
+
+ private:
+  base::MessageLoop loop_;  // Needed to initialize the directory.
+  TestDirectorySetterUpper dir_maker_;
+  scoped_ptr<TestEntryFactory> entry_factory_;
+
+  scoped_refptr<FakeModelWorker> ui_worker_;
+  scoped_refptr<FakeModelWorker> password_worker_;
+  scoped_refptr<FakeModelWorker> passive_worker_;
+
+  UpdateHandlerMap update_handler_map_;
+  STLValueDeleter<UpdateHandlerMap> update_handler_map_deleter_;
+};
+
+namespace {
+sync_pb::EntitySpecifics DefaultBookmarkSpecifics() {
+  sync_pb::EntitySpecifics result;
+  AddDefaultFieldValue(BOOKMARKS, &result);
+  return result;
+}
+} // namespace
+
+// Test update application for a few bookmark items.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest, SimpleBookmark) {
+  sessions::StatusController status;
+
+  std::string root_server_id = syncable::GetNullId().GetServerId();
+  int64 parent_handle =
+      entry_factory()->CreateUnappliedNewBookmarkItemWithParent(
+          "parent", DefaultBookmarkSpecifics(), root_server_id);
+  int64 child_handle =
+      entry_factory()->CreateUnappliedNewBookmarkItemWithParent(
+          "child", DefaultBookmarkSpecifics(), "parent");
+
+  ApplyBookmarkUpdates(&status);
+
+  EXPECT_EQ(0, status.num_encryption_conflicts())
+      << "Simple update shouldn't result in conflicts";
+  EXPECT_EQ(0, status.num_hierarchy_conflicts())
+      << "Simple update shouldn't result in conflicts";
+  EXPECT_EQ(2, status.num_updates_applied())
+      << "All items should have been successfully applied";
+
+  {
+    syncable::ReadTransaction trans(FROM_HERE, directory());
+
+    syncable::Entry parent(&trans, syncable::GET_BY_HANDLE, parent_handle);
+    syncable::Entry child(&trans, syncable::GET_BY_HANDLE, child_handle);
+
+    ASSERT_TRUE(parent.good());
+    ASSERT_TRUE(child.good());
+
+    EXPECT_FALSE(parent.GetIsUnsynced());
+    EXPECT_FALSE(parent.GetIsUnappliedUpdate());
+    EXPECT_FALSE(child.GetIsUnsynced());
+    EXPECT_FALSE(child.GetIsUnappliedUpdate());
+  }
+}
+
+// Test that the applicator can handle updates delivered out of order.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest,
+       BookmarkChildrenBeforeParent) {
+  // Start with some bookmarks whose parents are unknown.
+  std::string root_server_id = syncable::GetNullId().GetServerId();
+  int64 a_handle = entry_factory()->CreateUnappliedNewBookmarkItemWithParent(
+      "a_child_created_first", DefaultBookmarkSpecifics(), "parent");
+  int64 x_handle = entry_factory()->CreateUnappliedNewBookmarkItemWithParent(
+      "x_child_created_first", DefaultBookmarkSpecifics(), "parent");
+
+  // Update application will fail.
+  sessions::StatusController status1;
+  ApplyBookmarkUpdates(&status1);
+  EXPECT_EQ(0, status1.num_updates_applied());
+  EXPECT_EQ(2, status1.num_hierarchy_conflicts());
+
+  {
+    syncable::ReadTransaction trans(FROM_HERE, directory());
+
+    syncable::Entry a(&trans, syncable::GET_BY_HANDLE, a_handle);
+    syncable::Entry x(&trans, syncable::GET_BY_HANDLE, x_handle);
+
+    ASSERT_TRUE(a.good());
+    ASSERT_TRUE(x.good());
+
+    EXPECT_TRUE(a.GetIsUnappliedUpdate());
+    EXPECT_TRUE(x.GetIsUnappliedUpdate());
+  }
+
+  // Now add their parent and a few siblings.
+  entry_factory()->CreateUnappliedNewBookmarkItemWithParent(
+      "parent", DefaultBookmarkSpecifics(), root_server_id);
+  entry_factory()->CreateUnappliedNewBookmarkItemWithParent(
+      "a_child_created_second", DefaultBookmarkSpecifics(), "parent");
+  entry_factory()->CreateUnappliedNewBookmarkItemWithParent(
+      "x_child_created_second", DefaultBookmarkSpecifics(), "parent");
+
+  // Update application will succeed.
+  sessions::StatusController status2;
+  ApplyBookmarkUpdates(&status2);
+  EXPECT_EQ(5, status2.num_updates_applied())
+      << "All updates should have been successfully applied";
+
+  {
+    syncable::ReadTransaction trans(FROM_HERE, directory());
+
+    syncable::Entry a(&trans, syncable::GET_BY_HANDLE, a_handle);
+    syncable::Entry x(&trans, syncable::GET_BY_HANDLE, x_handle);
+
+    ASSERT_TRUE(a.good());
+    ASSERT_TRUE(x.good());
+
+    EXPECT_FALSE(a.GetIsUnappliedUpdate());
+    EXPECT_FALSE(x.GetIsUnappliedUpdate());
+  }
+}
+
+// Try to apply changes on an item that is both IS_UNSYNCED and
+// IS_UNAPPLIED_UPDATE.  Conflict resolution should be performed.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest, SimpleBookmarkConflict) {
+  int64 handle = entry_factory()->CreateUnappliedAndUnsyncedBookmarkItem("x");
+
+  int original_server_version = -10;
+  {
+    syncable::ReadTransaction trans(FROM_HERE, directory());
+    syncable::Entry e(&trans, syncable::GET_BY_HANDLE, handle);
+    original_server_version = e.GetServerVersion();
+    ASSERT_NE(original_server_version, e.GetBaseVersion());
+    EXPECT_TRUE(e.GetIsUnsynced());
+  }
+
+  sessions::StatusController status;
+  ApplyBookmarkUpdates(&status);
+  EXPECT_EQ(1, status.num_server_overwrites())
+      << "Unsynced and unapplied item conflict should be resolved";
+  EXPECT_EQ(0, status.num_updates_applied())
+      << "Update should not be applied; we should override the server.";
+
+  {
+    syncable::ReadTransaction trans(FROM_HERE, directory());
+    syncable::Entry e(&trans, syncable::GET_BY_HANDLE, handle);
+    ASSERT_TRUE(e.good());
+    EXPECT_EQ(original_server_version, e.GetServerVersion());
+    EXPECT_EQ(original_server_version, e.GetBaseVersion());
+    EXPECT_FALSE(e.GetIsUnappliedUpdate());
+
+    // The unsynced flag will remain set until we successfully commit the item.
+    EXPECT_TRUE(e.GetIsUnsynced());
+  }
+}
+
+// Create a simple conflict that is also a hierarchy conflict.  If we were to
+// follow the normal "server wins" logic, we'd end up violating hierarchy
+// constraints.  The hierarchy conflict must take precedence.  We can not allow
+// the update to be applied.  The item must remain in the conflict state.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest, HierarchyAndSimpleConflict) {
+  // Create a simply-conflicting item.  It will start with valid parent ids.
+  int64 handle = entry_factory()->CreateUnappliedAndUnsyncedBookmarkItem(
+      "orphaned_by_server");
+  {
+    // Manually set the SERVER_PARENT_ID to bad value.
+    // A bad parent indicates a hierarchy conflict.
+    syncable::WriteTransaction trans(FROM_HERE, UNITTEST, directory());
+    syncable::MutableEntry entry(&trans, syncable::GET_BY_HANDLE, handle);
+    ASSERT_TRUE(entry.good());
+
+    entry.PutServerParentId(TestIdFactory::MakeServer("bogus_parent"));
+  }
+
+  sessions::StatusController status;
+  ApplyBookmarkUpdates(&status);
+  EXPECT_EQ(0, status.num_updates_applied());
+  EXPECT_EQ(0, status.num_server_overwrites());
+  EXPECT_EQ(1, status.num_hierarchy_conflicts());
+
+  {
+    syncable::ReadTransaction trans(FROM_HERE, directory());
+    syncable::Entry e(&trans, syncable::GET_BY_HANDLE, handle);
+    ASSERT_TRUE(e.good());
+    EXPECT_TRUE(e.GetIsUnappliedUpdate());
+    EXPECT_TRUE(e.GetIsUnsynced());
+  }
+}
+
+// Attempt to apply an udpate that would create a bookmark folder loop.  This
+// application should fail.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest, BookmarkFolderLoop) {
+  // Item 'X' locally has parent of 'root'.  Server is updating it to have
+  // parent of 'Y'.
+
+  // Create it as a child of root node.
+  int64 handle = entry_factory()->CreateSyncedItem("X", BOOKMARKS, true);
+
+  {
+    syncable::WriteTransaction trans(FROM_HERE, UNITTEST, directory());
+    syncable::MutableEntry entry(&trans, syncable::GET_BY_HANDLE, handle);
+    ASSERT_TRUE(entry.good());
+
+    // Re-parent from root to "Y"
+    entry.PutServerVersion(entry_factory()->GetNextRevision());
+    entry.PutIsUnappliedUpdate(true);
+    entry.PutServerParentId(TestIdFactory::MakeServer("Y"));
+  }
+
+  // Item 'Y' is child of 'X'.
+  entry_factory()->CreateUnsyncedItem(
+      TestIdFactory::MakeServer("Y"), TestIdFactory::MakeServer("X"), "Y", true,
+      BOOKMARKS, NULL);
+
+  // If the server's update were applied, we would have X be a child of Y, and Y
+  // as a child of X.  That's a directory loop.  The UpdateApplicator should
+  // prevent the update from being applied and note that this is a hierarchy
+  // conflict.
+
+  sessions::StatusController status;
+  ApplyBookmarkUpdates(&status);
+
+  // This should count as a hierarchy conflict.
+  EXPECT_EQ(1, status.num_hierarchy_conflicts());
+
+  {
+    syncable::ReadTransaction trans(FROM_HERE, directory());
+    syncable::Entry e(&trans, syncable::GET_BY_HANDLE, handle);
+    ASSERT_TRUE(e.good());
+    EXPECT_TRUE(e.GetIsUnappliedUpdate());
+    EXPECT_FALSE(e.GetIsUnsynced());
+  }
+}
+
+// Test update application where the update has been orphaned by a local folder
+// deletion.  The update application attempt should fail.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest,
+       HierarchyConflictDeletedParent) {
+  // Create a locally deleted parent item.
+  int64 parent_handle;
+  entry_factory()->CreateUnsyncedItem(
+      syncable::Id::CreateFromServerId("parent"), TestIdFactory::root(),
+      "parent", true, BOOKMARKS, &parent_handle);
+  {
+    syncable::WriteTransaction trans(FROM_HERE, UNITTEST, directory());
+    syncable::MutableEntry entry(&trans,
+                                 syncable::GET_BY_HANDLE,
+                                 parent_handle);
+    entry.PutIsDel(true);
+  }
+
+  // Create an incoming child from the server.
+  int64 child_handle = entry_factory()->CreateUnappliedNewItemWithParent(
+      "child", DefaultBookmarkSpecifics(), "parent");
+
+  // The server's update may seem valid to some other client, but on this client
+  // that new item's parent no longer exists.  The update should not be applied
+  // and the update applicator should indicate this is a hierarchy conflict.
+
+  sessions::StatusController status;
+  ApplyBookmarkUpdates(&status);
+  EXPECT_EQ(1, status.num_hierarchy_conflicts());
+
+  {
+    syncable::ReadTransaction trans(FROM_HERE, directory());
+    syncable::Entry child(&trans, syncable::GET_BY_HANDLE, child_handle);
+    ASSERT_TRUE(child.good());
+    EXPECT_TRUE(child.GetIsUnappliedUpdate());
+    EXPECT_FALSE(child.GetIsUnsynced());
+  }
+}
+
+// Attempt to apply an update that deletes a folder where the folder has
+// locally-created children.  The update application should fail.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest,
+       HierarchyConflictDeleteNonEmptyDirectory) {
+  // Create a server-deleted folder as a child of root node.
+  int64 parent_handle =
+      entry_factory()->CreateSyncedItem("parent", BOOKMARKS, true);
+  {
+    syncable::WriteTransaction trans(FROM_HERE, UNITTEST, directory());
+    syncable::MutableEntry entry(&trans,
+                                 syncable::GET_BY_HANDLE,
+                                 parent_handle);
+    ASSERT_TRUE(entry.good());
+
+    // Delete it on the server.
+    entry.PutServerVersion(entry_factory()->GetNextRevision());
+    entry.PutIsUnappliedUpdate(true);
+    entry.PutServerParentId(TestIdFactory::root());
+    entry.PutServerIsDel(true);
+  }
+
+  // Create a local child of the server-deleted directory.
+  entry_factory()->CreateUnsyncedItem(
+      TestIdFactory::MakeServer("child"), TestIdFactory::MakeServer("parent"),
+      "child", false, BOOKMARKS, NULL);
+
+  // The server's request to delete the directory must be ignored, otherwise our
+  // unsynced new child would be orphaned.  This is a hierarchy conflict.
+
+  sessions::StatusController status;
+  ApplyBookmarkUpdates(&status);
+
+  // This should count as a hierarchy conflict.
+  EXPECT_EQ(1, status.num_hierarchy_conflicts());
+
+  {
+    syncable::ReadTransaction trans(FROM_HERE, directory());
+    syncable::Entry parent(&trans, syncable::GET_BY_HANDLE, parent_handle);
+    ASSERT_TRUE(parent.good());
+    EXPECT_TRUE(parent.GetIsUnappliedUpdate());
+    EXPECT_FALSE(parent.GetIsUnsynced());
+  }
+}
+
+// Attempt to apply updates where the updated item's parent is not known to this
+// client.  The update application attempt should fail.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest,
+       HierarchyConflictUnknownParent) {
+  // We shouldn't be able to do anything with either of these items.
+  int64 x_handle = entry_factory()->CreateUnappliedNewItemWithParent(
+      "some_item", DefaultBookmarkSpecifics(), "unknown_parent");
+  int64 y_handle = entry_factory()->CreateUnappliedNewItemWithParent(
+      "some_other_item", DefaultBookmarkSpecifics(), "some_item");
+
+  sessions::StatusController status;
+  ApplyBookmarkUpdates(&status);
+
+  EXPECT_EQ(2, status.num_hierarchy_conflicts())
+      << "All updates with an unknown ancestors should be in conflict";
+  EXPECT_EQ(0, status.num_updates_applied())
+      << "No item with an unknown ancestor should be applied";
+
+  {
+    syncable::ReadTransaction trans(FROM_HERE, directory());
+    syncable::Entry x(&trans, syncable::GET_BY_HANDLE, x_handle);
+    syncable::Entry y(&trans, syncable::GET_BY_HANDLE, y_handle);
+    ASSERT_TRUE(x.good());
+    ASSERT_TRUE(y.good());
+    EXPECT_TRUE(x.GetIsUnappliedUpdate());
+    EXPECT_TRUE(y.GetIsUnappliedUpdate());
+    EXPECT_FALSE(x.GetIsUnsynced());
+    EXPECT_FALSE(y.GetIsUnsynced());
+  }
+}
+
+// Attempt application of a mix of items.  Some update application attempts will
+// fail due to hierarchy conflicts.  Others should succeed.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest, ItemsBothKnownAndUnknown) {
+  // See what happens when there's a mixture of good and bad updates.
+  std::string root_server_id = syncable::GetNullId().GetServerId();
+  int64 u1_handle = entry_factory()->CreateUnappliedNewItemWithParent(
+      "first_unknown_item", DefaultBookmarkSpecifics(), "unknown_parent");
+  int64 k1_handle = entry_factory()->CreateUnappliedNewItemWithParent(
+      "first_known_item", DefaultBookmarkSpecifics(), root_server_id);
+  int64 u2_handle = entry_factory()->CreateUnappliedNewItemWithParent(
+      "second_unknown_item", DefaultBookmarkSpecifics(), "unknown_parent");
+  int64 k2_handle = entry_factory()->CreateUnappliedNewItemWithParent(
+      "second_known_item", DefaultBookmarkSpecifics(), "first_known_item");
+  int64 k3_handle = entry_factory()->CreateUnappliedNewItemWithParent(
+      "third_known_item", DefaultBookmarkSpecifics(), "fourth_known_item");
+  int64 k4_handle = entry_factory()->CreateUnappliedNewItemWithParent(
+      "fourth_known_item", DefaultBookmarkSpecifics(), root_server_id);
+
+  sessions::StatusController status;
+  ApplyBookmarkUpdates(&status);
+
+  EXPECT_EQ(2, status.num_hierarchy_conflicts())
+      << "The updates with unknown ancestors should be in conflict";
+  EXPECT_EQ(4, status.num_updates_applied())
+      << "The updates with known ancestors should be successfully applied";
+
+  {
+    syncable::ReadTransaction trans(FROM_HERE, directory());
+    syncable::Entry u1(&trans, syncable::GET_BY_HANDLE, u1_handle);
+    syncable::Entry u2(&trans, syncable::GET_BY_HANDLE, u2_handle);
+    syncable::Entry k1(&trans, syncable::GET_BY_HANDLE, k1_handle);
+    syncable::Entry k2(&trans, syncable::GET_BY_HANDLE, k2_handle);
+    syncable::Entry k3(&trans, syncable::GET_BY_HANDLE, k3_handle);
+    syncable::Entry k4(&trans, syncable::GET_BY_HANDLE, k4_handle);
+    ASSERT_TRUE(u1.good());
+    ASSERT_TRUE(u2.good());
+    ASSERT_TRUE(k1.good());
+    ASSERT_TRUE(k2.good());
+    ASSERT_TRUE(k3.good());
+    ASSERT_TRUE(k4.good());
+    EXPECT_TRUE(u1.GetIsUnappliedUpdate());
+    EXPECT_TRUE(u2.GetIsUnappliedUpdate());
+    EXPECT_FALSE(k1.GetIsUnappliedUpdate());
+    EXPECT_FALSE(k2.GetIsUnappliedUpdate());
+    EXPECT_FALSE(k3.GetIsUnappliedUpdate());
+    EXPECT_FALSE(k4.GetIsUnappliedUpdate());
+  }
+}
+
+// Attempt application of password upates where the passphrase is known.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest, DecryptablePassword) {
+  // Decryptable password updates should be applied.
+  Cryptographer* cryptographer;
+  {
+    // Storing the cryptographer separately is bad, but for this test we
+    // know it's safe.
+    syncable::ReadTransaction trans(FROM_HERE, directory());
+    cryptographer = directory()->GetCryptographer(&trans);
+  }
+
+  KeyParams params = {"localhost", "dummy", "foobar"};
+  cryptographer->AddKey(params);
+
+  sync_pb::EntitySpecifics specifics;
+  sync_pb::PasswordSpecificsData data;
+  data.set_origin("http://example.com");
+
+  cryptographer->Encrypt(data,
+                         specifics.mutable_password()->mutable_encrypted());
+  int64 handle =
+      entry_factory()->CreateUnappliedNewItem("item", specifics, false);
+
+  sessions::StatusController status;
+  ApplyPasswordUpdates(&status);
+
+  EXPECT_EQ(1, status.num_updates_applied())
+      << "The updates that can be decrypted should be applied";
+
+  {
+    syncable::ReadTransaction trans(FROM_HERE, directory());
+    syncable::Entry e(&trans, syncable::GET_BY_HANDLE, handle);
+    ASSERT_TRUE(e.good());
+    EXPECT_FALSE(e.GetIsUnappliedUpdate());
+    EXPECT_FALSE(e.GetIsUnsynced());
+  }
+}
+
+// Attempt application of encrypted items when the passphrase is not known.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest, UndecryptableData) {
+  // Undecryptable updates should not be applied.
+  sync_pb::EntitySpecifics encrypted_bookmark;
+  encrypted_bookmark.mutable_encrypted();
+  AddDefaultFieldValue(BOOKMARKS, &encrypted_bookmark);
+  std::string root_server_id = syncable::GetNullId().GetServerId();
+  int64 folder_handle = entry_factory()->CreateUnappliedNewItemWithParent(
+      "folder",
+      encrypted_bookmark,
+      root_server_id);
+  int64 bookmark_handle = entry_factory()->CreateUnappliedNewItem(
+      "item2",
+      encrypted_bookmark,
+      false);
+  sync_pb::EntitySpecifics encrypted_password;
+  encrypted_password.mutable_password();
+  int64 password_handle = entry_factory()->CreateUnappliedNewItem(
+      "item3",
+      encrypted_password,
+      false);
+
+  sessions::StatusController status;
+  ApplyBookmarkUpdates(&status);
+  ApplyPasswordUpdates(&status);
+
+  EXPECT_EQ(3, status.num_encryption_conflicts())
+      << "Updates that can't be decrypted should be in encryption conflict";
+  EXPECT_EQ(0, status.num_updates_applied())
+      << "No update that can't be decrypted should be applied";
+
+  {
+    syncable::ReadTransaction trans(FROM_HERE, directory());
+    syncable::Entry folder(&trans, syncable::GET_BY_HANDLE, folder_handle);
+    syncable::Entry bm(&trans, syncable::GET_BY_HANDLE, bookmark_handle);
+    syncable::Entry pw(&trans, syncable::GET_BY_HANDLE, password_handle);
+    ASSERT_TRUE(folder.good());
+    ASSERT_TRUE(bm.good());
+    ASSERT_TRUE(pw.good());
+    EXPECT_TRUE(folder.GetIsUnappliedUpdate());
+    EXPECT_TRUE(bm.GetIsUnappliedUpdate());
+    EXPECT_TRUE(pw.GetIsUnappliedUpdate());
+  }
+}
+
+// Test a mix of decryptable and undecryptable updates.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest, SomeUndecryptablePassword) {
+  Cryptographer* cryptographer;
+
+  int64 decryptable_handle = -1;
+  int64 undecryptable_handle = -1;
+
+  // Only decryptable password updates should be applied.
+  {
+    sync_pb::EntitySpecifics specifics;
+    sync_pb::PasswordSpecificsData data;
+    data.set_origin("http://example.com/1");
+    {
+      syncable::ReadTransaction trans(FROM_HERE, directory());
+      cryptographer = directory()->GetCryptographer(&trans);
+
+      KeyParams params = {"localhost", "dummy", "foobar"};
+      cryptographer->AddKey(params);
+
+      cryptographer->Encrypt(data,
+          specifics.mutable_password()->mutable_encrypted());
+    }
+    decryptable_handle =
+        entry_factory()->CreateUnappliedNewItem("item1", specifics, false);
+  }
+  {
+    // Create a new cryptographer, independent of the one in the session.
+    Cryptographer other_cryptographer(cryptographer->encryptor());
+    KeyParams params = {"localhost", "dummy", "bazqux"};
+    other_cryptographer.AddKey(params);
+
+    sync_pb::EntitySpecifics specifics;
+    sync_pb::PasswordSpecificsData data;
+    data.set_origin("http://example.com/2");
+
+    other_cryptographer.Encrypt(data,
+        specifics.mutable_password()->mutable_encrypted());
+    undecryptable_handle =
+        entry_factory()->CreateUnappliedNewItem("item2", specifics, false);
+  }
+
+  sessions::StatusController status;
+  ApplyPasswordUpdates(&status);
+
+  EXPECT_EQ(1, status.num_encryption_conflicts())
+      << "The updates that can't be decrypted should be in encryption "
+      << "conflict";
+  EXPECT_EQ(1, status.num_updates_applied())
+      << "The undecryptable password update shouldn't be applied";
+
+  {
+    syncable::ReadTransaction trans(FROM_HERE, directory());
+    syncable::Entry e1(&trans, syncable::GET_BY_HANDLE, decryptable_handle);
+    syncable::Entry e2(&trans, syncable::GET_BY_HANDLE, undecryptable_handle);
+    ASSERT_TRUE(e1.good());
+    ASSERT_TRUE(e2.good());
+    EXPECT_FALSE(e1.GetIsUnappliedUpdate());
+    EXPECT_TRUE(e2.GetIsUnappliedUpdate());
+  }
+}
+
 }  // namespace syncer
diff --git a/sync/engine/sync_engine_event.h b/sync/engine/sync_engine_event.h
index 25f9692..026d329 100644
--- a/sync/engine/sync_engine_event.h
+++ b/sync/engine/sync_engine_event.h
@@ -23,7 +23,7 @@ struct SYNC_EXPORT_PRIVATE SyncEngineEvent {
     // Sent on entry of Syncer state machine
     SYNC_CYCLE_BEGIN,
 
-    // SyncerCommand generated events.
+    // Sent any time progress is made during a sync cycle.
     STATUS_CHANGED,
 
     // We have reached the SYNCER_END state in the main sync loop.
diff --git a/sync/engine/sync_scheduler_impl.cc b/sync/engine/sync_scheduler_impl.cc
index 85e1b67..f3b96e4 100644
--- a/sync/engine/sync_scheduler_impl.cc
+++ b/sync/engine/sync_scheduler_impl.cc
@@ -242,10 +242,8 @@ void SyncSchedulerImpl::Start(Mode mode) {
 }
 
 ModelTypeSet SyncSchedulerImpl::GetEnabledAndUnthrottledTypes() {
-  ModelTypeSet enabled_types =
-      GetRoutingInfoTypes(session_context_->routing_info());
-  ModelTypeSet throttled_types =
-      nudge_tracker_.GetThrottledTypes();
+  ModelTypeSet enabled_types = session_context_->enabled_types();
+  ModelTypeSet throttled_types = nudge_tracker_.GetThrottledTypes();
   return Difference(enabled_types, throttled_types);
 }
 
@@ -336,8 +334,7 @@ bool SyncSchedulerImpl::CanRunNudgeJobNow(JobPriority priority) {
     return false;
   }
 
-  const ModelTypeSet enabled_types =
-      GetRoutingInfoTypes(session_context_->routing_info());
+  const ModelTypeSet enabled_types = session_context_->enabled_types();
   if (nudge_tracker_.GetThrottledTypes().HasAll(enabled_types)) {
     SDVLOG(1) << "Not running a nudge because we're fully type throttled.";
     return false;
@@ -459,8 +456,8 @@ void SyncSchedulerImpl::DoNudgeSyncSessionJob(JobPriority priority) {
   DCHECK(CalledOnValidThread());
   DCHECK(CanRunNudgeJobNow(priority));
 
-  DVLOG(2) << "Will run normal mode sync cycle with routing info "
-           << ModelSafeRoutingInfoToString(session_context_->routing_info());
+  DVLOG(2) << "Will run normal mode sync cycle with types "
+           << ModelTypeSetToString(session_context_->enabled_types());
   scoped_ptr<SyncSession> session(SyncSession::Build(session_context_, this));
   bool premature_exit = !syncer_->NormalSyncShare(
       GetEnabledAndUnthrottledTypes(),
@@ -503,11 +500,11 @@ void SyncSchedulerImpl::DoConfigurationSyncSessionJob(JobPriority priority) {
     return;
   }
 
-  SDVLOG(2) << "Will run configure SyncShare with routes "
-           << ModelSafeRoutingInfoToString(session_context_->routing_info());
+  SDVLOG(2) << "Will run configure SyncShare with types "
+            << ModelTypeSetToString(session_context_->enabled_types());
   scoped_ptr<SyncSession> session(SyncSession::Build(session_context_, this));
   bool premature_exit = !syncer_->ConfigureSyncShare(
-      GetRoutingInfoTypes(session_context_->routing_info()),
+      session_context_->enabled_types(),
       pending_configure_params_->source,
       session.get());
   AdjustPolling(FORCE_RESET);
@@ -567,8 +564,8 @@ void SyncSchedulerImpl::DoPollSyncSessionJob() {
     return;
   }
 
-  SDVLOG(2) << "Polling with routes "
-           << ModelSafeRoutingInfoToString(session_context_->routing_info());
+  SDVLOG(2) << "Polling with types "
+            << ModelTypeSetToString(session_context_->enabled_types());
   scoped_ptr<SyncSession> session(SyncSession::Build(session_context_, this));
   syncer_->PollSyncShare(
       GetEnabledAndUnthrottledTypes(),
diff --git a/sync/engine/syncer.cc b/sync/engine/syncer.cc
index 0c451ee..39b068b 100644
--- a/sync/engine/syncer.cc
+++ b/sync/engine/syncer.cc
@@ -11,7 +11,6 @@
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "sync/engine/apply_control_data_updates.h"
-#include "sync/engine/apply_updates_and_resolve_conflicts_command.h"
 #include "sync/engine/commit.h"
 #include "sync/engine/conflict_resolver.h"
 #include "sync/engine/download.h"
@@ -115,8 +114,11 @@ void Syncer::ApplyUpdates(SyncSession* session) {
 
   ApplyControlDataUpdates(session->context()->directory());
 
-  ApplyUpdatesAndResolveConflictsCommand apply_updates;
-  apply_updates.Execute(session);
+  UpdateHandlerMap* handler_map = session->context()->update_handler_map();
+  for (UpdateHandlerMap::iterator it = handler_map->begin();
+       it != handler_map->end(); ++it) {
+    it->second->ApplyUpdates(session->mutable_status_controller());
+  }
 
   session->context()->set_hierarchy_conflict_detected(
       session->status_controller().num_hierarchy_conflicts() > 0);
diff --git a/sync/engine/syncer.h b/sync/engine/syncer.h
index 5661fff..6154f91 100644
--- a/sync/engine/syncer.h
+++ b/sync/engine/syncer.h
@@ -23,16 +23,15 @@ namespace syncer {
 
 class CancelationSignal;
 
-// A Syncer provides a control interface for driving the individual steps
-// of the sync cycle.  Each cycle (hopefully) moves the client into closer
-// synchronization with the server.  The individual steps are modeled
-// as SyncerCommands, and the ordering of the steps is expressed using
-// the SyncerStep enum.
+// A Syncer provides a control interface for driving the sync cycle.  These
+// cycles consist of downloading updates, parsing the response (aka. process
+// updates), applying updates while resolving conflicts, and committing local
+// changes.  Some of these steps may be skipped if they're deemed to be
+// unnecessary.
 //
-// A Syncer instance expects to run on a dedicated thread.  Calls
-// to SyncShare() may take an unbounded amount of time, as SyncerCommands
-// may block on network i/o, on lock contention, or on tasks posted to
-// other threads.
+// A Syncer instance expects to run on a dedicated thread.  Calls to SyncShare()
+// may take an unbounded amount of time because it may block on network I/O, on
+// lock contention, or on tasks posted to other threads.
 class SYNC_EXPORT_PRIVATE Syncer {
  public:
   typedef std::vector<int64> UnsyncedMetaHandles;
diff --git a/sync/engine/syncer_command.cc b/sync/engine/syncer_command.cc
deleted file mode 100644
index 5dfd821..0000000
--- a/sync/engine/syncer_command.cc
+++ /dev/null
@@ -1,17 +0,0 @@
-// Copyright (c) 2012 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 "sync/engine/syncer_command.h"
-
-namespace syncer {
-
-SyncerCommand::SyncerCommand() {}
-SyncerCommand::~SyncerCommand() {}
-
-SyncerError SyncerCommand::Execute(sessions::SyncSession* session) {
-  SyncerError result = ExecuteImpl(session);
-  return result;
-}
-
-}  // namespace syncer
diff --git a/sync/engine/syncer_command.h b/sync/engine/syncer_command.h
deleted file mode 100644
index 303aad5..0000000
--- a/sync/engine/syncer_command.h
+++ /dev/null
@@ -1,47 +0,0 @@
-// Copyright 2012 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 SYNC_ENGINE_SYNCER_COMMAND_H_
-#define SYNC_ENGINE_SYNCER_COMMAND_H_
-
-#include "base/basictypes.h"
-
-#include "sync/base/sync_export.h"
-#include "sync/internal_api/public/util/syncer_error.h"
-
-namespace syncer {
-
-namespace sessions {
-class SyncSession;
-}
-
-// Implementation of a simple command pattern intended to be driven by the
-// Syncer. SyncerCommand is abstract and all subclasses must implement
-// ExecuteImpl(). This is done so that chunks of syncer operation can be unit
-// tested.
-//
-// Example Usage:
-//
-//   SyncSession session = ...;
-//   SyncerCommand *cmd = SomeCommandFactory.createCommand(...);
-//   cmd->Execute(session);
-//   delete cmd;
-
-class SYNC_EXPORT_PRIVATE SyncerCommand {
- public:
-  SyncerCommand();
-  virtual ~SyncerCommand();
-
-  // Execute dispatches to a derived class's ExecuteImpl.
-  SyncerError Execute(sessions::SyncSession* session);
-
-  // ExecuteImpl is where derived classes actually do work.
-  virtual SyncerError ExecuteImpl(sessions::SyncSession* session) = 0;
- private:
-  DISALLOW_COPY_AND_ASSIGN(SyncerCommand);
-};
-
-}  // namespace syncer
-
-#endif  // SYNC_ENGINE_SYNCER_COMMAND_H_
diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc
index 684b404..1678ba6 100644
--- a/sync/engine/syncer_unittest.cc
+++ b/sync/engine/syncer_unittest.cc
@@ -189,7 +189,7 @@ class SyncerTest : public testing::Test,
 
     EXPECT_TRUE(
         syncer_->NormalSyncShare(
-            GetRoutingInfoTypes(context_->routing_info()),
+            context_->enabled_types(),
             nudge_tracker_,
             session_.get()));
   }
@@ -197,7 +197,7 @@ class SyncerTest : public testing::Test,
   void SyncShareConfigure() {
     session_.reset(SyncSession::Build(context_.get(), this));
     EXPECT_TRUE(syncer_->ConfigureSyncShare(
-            GetRoutingInfoTypes(context_->routing_info()),
+            context_->enabled_types(),
             sync_pb::GetUpdatesCallerInfo::RECONFIGURATION,
             session_.get()));
   }
@@ -544,8 +544,7 @@ TEST_F(SyncerTest, GetCommitIdsFiltersThrottledEntries) {
 
   // Now sync without enabling bookmarks.
   syncer_->NormalSyncShare(
-      Difference(GetRoutingInfoTypes(context_->routing_info()),
-                 ModelTypeSet(BOOKMARKS)),
+      Difference(context_->enabled_types(), ModelTypeSet(BOOKMARKS)),
       nudge_tracker_,
       session_.get());
 
@@ -559,7 +558,7 @@ TEST_F(SyncerTest, GetCommitIdsFiltersThrottledEntries) {
 
   // Sync again with bookmarks enabled.
   syncer_->NormalSyncShare(
-      GetRoutingInfoTypes(context_->routing_info()),
+      context_->enabled_types(),
       nudge_tracker_,
       session_.get());
   SyncShareNudge();
diff --git a/sync/engine/update_applicator.cc b/sync/engine/update_applicator.cc
index 3edf1f8..e8731ce 100644
--- a/sync/engine/update_applicator.cc
+++ b/sync/engine/update_applicator.cc
@@ -19,12 +19,8 @@ namespace syncer {
 
 using syncable::ID;
 
-UpdateApplicator::UpdateApplicator(Cryptographer* cryptographer,
-                                   const ModelSafeRoutingInfo& routes,
-                                   ModelSafeGroup group_filter)
+UpdateApplicator::UpdateApplicator(Cryptographer* cryptographer)
     : cryptographer_(cryptographer),
-      group_filter_(group_filter),
-      routing_info_(routes),
       updates_applied_(0),
       encryption_conflicts_(0),
       hierarchy_conflicts_(0) {
@@ -58,11 +54,6 @@ void UpdateApplicator::AttemptApplications(
 
     for (std::vector<int64>::iterator i = to_apply.begin();
          i != to_apply.end(); ++i) {
-      syncable::Entry read_entry(trans, syncable::GET_BY_HANDLE, *i);
-      if (SkipUpdate(read_entry)) {
-        continue;
-      }
-
       syncable::MutableEntry entry(trans, syncable::GET_BY_HANDLE, *i);
       UpdateAttemptResponse result = AttemptToUpdateEntry(
           trans, &entry, cryptographer_);
@@ -103,23 +94,4 @@ void UpdateApplicator::AttemptApplications(
   }
 }
 
-bool UpdateApplicator::SkipUpdate(const syncable::Entry& entry) {
-  ModelType type = entry.GetServerModelType();
-  ModelSafeGroup g = GetGroupForModelType(type, routing_info_);
-  // The set of updates passed to the UpdateApplicator should already
-  // be group-filtered.
-  if (g != group_filter_) {
-    NOTREACHED();
-    return true;
-  }
-  if (g == GROUP_PASSIVE &&
-      !routing_info_.count(type) &&
-      type != UNSPECIFIED &&
-      type != TOP_LEVEL_FOLDER) {
-    DVLOG(1) << "Skipping update application, type not permitted.";
-    return true;
-  }
-  return false;
-}
-
 }  // namespace syncer
diff --git a/sync/engine/update_applicator.h b/sync/engine/update_applicator.h
index b04325c..ff8fa15 100644
--- a/sync/engine/update_applicator.h
+++ b/sync/engine/update_applicator.h
@@ -35,9 +35,7 @@ class Cryptographer;
 
 class UpdateApplicator {
  public:
-  UpdateApplicator(Cryptographer* cryptographer,
-                   const ModelSafeRoutingInfo& routes,
-                   ModelSafeGroup group_filter);
+  UpdateApplicator(Cryptographer* cryptographer);
   ~UpdateApplicator();
 
   // Attempt to apply the specified updates.
@@ -67,10 +65,6 @@ class UpdateApplicator {
   // Used to decrypt sensitive sync nodes.
   Cryptographer* cryptographer_;
 
-  ModelSafeGroup group_filter_;
-
-  const ModelSafeRoutingInfo routing_info_;
-
   DISALLOW_COPY_AND_ASSIGN(UpdateApplicator);
 
   int updates_applied_;
diff --git a/sync/internal_api/public/util/syncer_error.h b/sync/internal_api/public/util/syncer_error.h
index 471bc7e..db59b7d 100644
--- a/sync/internal_api/public/util/syncer_error.h
+++ b/sync/internal_api/public/util/syncer_error.h
@@ -9,15 +9,7 @@
 
 namespace syncer {
 
-// This enum describes all the ways a SyncerCommand can fail.
-//
-// SyncerCommands do many different things, but they share a common function
-// signature.  This enum, the return value for all SyncerCommands, must be able
-// to describe any possible failure for all SyncerComand.
-//
-// For convenience, functions which are invoked only by SyncerCommands may also
-// return a SyncerError.  It saves us having to write a conversion function, and
-// it makes refactoring easier.
+// This enum describes all the possible results of a sync cycle.
 enum SYNC_EXPORT_PRIVATE SyncerError {
   UNSET = 0,       // Default value.
   CANNOT_DO_WORK,  // A model worker could not process a work item.
diff --git a/sync/internal_api/sync_manager_impl_unittest.cc b/sync/internal_api/sync_manager_impl_unittest.cc
index 2a1129b..91be3b8 100644
--- a/sync/internal_api/sync_manager_impl_unittest.cc
+++ b/sync/internal_api/sync_manager_impl_unittest.cc
@@ -66,6 +66,7 @@
 #include "sync/syncable/syncable_util.h"
 #include "sync/syncable/syncable_write_transaction.h"
 #include "sync/test/callback_counter.h"
+#include "sync/test/engine/fake_model_worker.h"
 #include "sync/test/engine/fake_sync_scheduler.h"
 #include "sync/test/engine/test_id_factory.h"
 #include "sync/test/fake_encryptor.h"
@@ -816,6 +817,12 @@ class SyncManagerTest : public testing::Test,
     ModelSafeRoutingInfo routing_info;
     GetModelSafeRoutingInfo(&routing_info);
 
+    // This works only because all routing info types are GROUP_PASSIVE.
+    // If we had types in other groups, we would need additional workers
+    // to support them.
+    scoped_refptr<ModelSafeWorker> worker = new FakeModelWorker(GROUP_PASSIVE);
+    workers.push_back(worker.get());
+
     // Takes ownership of |fake_invalidator_|.
     sync_manager_.Init(
         temp_dir_.path(),
diff --git a/sync/sessions/status_controller.cc b/sync/sessions/status_controller.cc
index f6fbdb5..314a232 100644
--- a/sync/sessions/status_controller.cc
+++ b/sync/sessions/status_controller.cc
@@ -13,9 +13,7 @@
 namespace syncer {
 namespace sessions {
 
-StatusController::StatusController()
-    : group_restriction_in_effect_(false),
-      group_restriction_(GROUP_PASSIVE) {
+StatusController::StatusController() {
 }
 
 StatusController::~StatusController() {}
@@ -129,20 +127,14 @@ int StatusController::num_encryption_conflicts() const {
 }
 
 int StatusController::num_hierarchy_conflicts() const {
-  DCHECK(!group_restriction_in_effect_)
-      << "num_hierarchy_conflicts applies to all ModelSafeGroups";
   return model_neutral_.num_hierarchy_conflicts;
 }
 
 int StatusController::num_server_conflicts() const {
-  DCHECK(!group_restriction_in_effect_)
-      << "num_server_conflicts applies to all ModelSafeGroups";
   return model_neutral_.num_server_conflicts;
 }
 
 int StatusController::TotalNumConflictingItems() const {
-  DCHECK(!group_restriction_in_effect_)
-      << "TotalNumConflictingItems applies to all ModelSafeGroups";
   int sum = 0;
   sum += num_encryption_conflicts();
   sum += num_hierarchy_conflicts();
diff --git a/sync/sessions/status_controller.h b/sync/sessions/status_controller.h
index 75b8b82..0e2c6f2 100644
--- a/sync/sessions/status_controller.h
+++ b/sync/sessions/status_controller.h
@@ -5,25 +5,14 @@
 // StatusController handles all counter and status related number crunching and
 // state tracking on behalf of a SyncSession.
 //
-// The most important feature of StatusController is the
-// ScopedModelSafeGroupRestriction.  Some of its functions expose per-thread
-// state, and can be called only when the restriction is in effect.  For
-// example, if GROUP_UI is set then the value returned from
-// commit_id_projection() will be useful for iterating over the commit IDs of
-// items that live on the UI thread.
+// This object may be accessed from many different threads.  It will be accessed
+// most often from the syncer thread.  However, when update application is in
+// progress it may also be accessed from the worker threads.  This is safe
+// because only one of them will run at a time, and the syncer thread will be
+// blocked until update application completes.
 //
-// Other parts of its state are global, and do not require the restriction.
-//
-// 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.
+// This object contains only global state.  None of its members are per model
+// type counters.
 
 #ifndef SYNC_SESSIONS_STATUS_CONTROLLER_H_
 #define SYNC_SESSIONS_STATUS_CONTROLLER_H_
@@ -96,10 +85,6 @@ class SYNC_EXPORT_PRIVATE StatusController {
   // download: in that case, this also returns false.
   bool ServerSaysNothingMoreToDownload() const;
 
-  ModelSafeGroup group_restriction() const {
-    return group_restriction_;
-  }
-
   base::Time sync_start_time() const {
     // The time at which we sent the first GetUpdates command for this sync.
     return sync_start_time_;
@@ -142,40 +127,13 @@ class SYNC_EXPORT_PRIVATE StatusController {
   void UpdateStartTime();
 
  private:
-  friend class ScopedModelSafeGroupRestriction;
-
   ModelNeutralState model_neutral_;
 
-  // 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_;
-
   base::Time sync_start_time_;
 
   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);
-};
-
 }  // namespace sessions
 }  // namespace syncer
 
diff --git a/sync/sessions/status_controller_unittest.cc b/sync/sessions/status_controller_unittest.cc
index e6b59e8..32e7b64 100644
--- a/sync/sessions/status_controller_unittest.cc
+++ b/sync/sessions/status_controller_unittest.cc
@@ -52,14 +52,5 @@ TEST_F(StatusControllerTest, TotalNumConflictingItems) {
   EXPECT_EQ(6, status.TotalNumConflictingItems());
 }
 
-// Basic test that non group-restricted state accessors don't cause violations.
-TEST_F(StatusControllerTest, Unrestricted) {
-  StatusController status;
-  status.model_neutral_state();
-  status.download_updates_succeeded();
-  status.ServerSaysNothingMoreToDownload();
-  status.group_restriction();
-}
-
 }  // namespace sessions
 }  // namespace syncer
diff --git a/sync/sessions/sync_session.h b/sync/sessions/sync_session.h
index af846d4..f576720 100644
--- a/sync/sessions/sync_session.h
+++ b/sync/sessions/sync_session.h
@@ -4,12 +4,8 @@
 
 // A class representing an attempt to synchronize the local syncable data
 // store with a sync server. A SyncSession instance is passed as a stateful
-// bundle to and from various SyncerCommands with the goal of converging the
-// client view of data with that of the server. The commands twiddle with
-// session status in response to events and hiccups along the way, set and
-// query session progress with regards to conflict resolution and applying
-// server updates, and access the SyncSessionContext for the current session
-// via SyncSession instances.
+// bundle throughout the sync cycle.  The SyncSession is not reused across
+// sync cycles; each cycle starts with a new one.
 
 #ifndef SYNC_SESSIONS_SYNC_SESSION_H_
 #define SYNC_SESSIONS_SYNC_SESSION_H_
diff --git a/sync/sessions/sync_session_context.cc b/sync/sessions/sync_session_context.cc
index d44591d..aa5dfa5 100644
--- a/sync/sessions/sync_session_context.cc
+++ b/sync/sessions/sync_session_context.cc
@@ -35,8 +35,10 @@ SyncSessionContext::SyncSessionContext(
       server_enabled_pre_commit_update_avoidance_(false),
       client_enabled_pre_commit_update_avoidance_(
           client_enabled_pre_commit_update_avoidance) {
-  for (size_t i = 0u; i < workers.size(); ++i)
-    workers_.push_back(workers[i]);
+  for (size_t i = 0u; i < workers.size(); ++i) {
+    workers_.insert(
+        std::make_pair(workers[i]->GetModelSafeGroup(), workers[i]));
+  }
 
   std::vector<SyncEngineEventListener*>::const_iterator it;
   for (it = listeners.begin(); it != listeners.end(); ++it)
@@ -48,24 +50,28 @@ SyncSessionContext::~SyncSessionContext() {
 
 void SyncSessionContext::set_routing_info(
     const ModelSafeRoutingInfo& routing_info) {
-  routing_info_ = routing_info;
+  enabled_types_ = GetRoutingInfoTypes(routing_info);
 
   // TODO(rlarocque): This is not a good long-term solution.  We must find a
   // better way to initialize the set of CommitContributors and UpdateHandlers.
-  ModelTypeSet enabled_types = GetRoutingInfoTypes(routing_info);
-
+  STLDeleteValues<UpdateHandlerMap>(&update_handler_map_);
   STLDeleteValues<CommitContributorMap>(&commit_contributor_map_);
-  for (ModelTypeSet::Iterator it = enabled_types.First(); it.Good(); it.Inc()) {
-    SyncDirectoryCommitContributor* contributor =
-        new SyncDirectoryCommitContributor(directory(), it.Get());
-    commit_contributor_map_.insert(std::make_pair(it.Get(), contributor));
-  }
+  for (ModelSafeRoutingInfo::const_iterator routing_iter = routing_info.begin();
+       routing_iter != routing_info.end(); ++routing_iter) {
+    ModelType type = routing_iter->first;
+    ModelSafeGroup group = routing_iter->second;
+    std::map<ModelSafeGroup, scoped_refptr<ModelSafeWorker> >::iterator
+        worker_it = workers_.find(group);
+    DCHECK(worker_it != workers_.end());
+    scoped_refptr<ModelSafeWorker> worker = worker_it->second;
 
-  STLDeleteValues<UpdateHandlerMap>(&update_handler_map_);
-  for (ModelTypeSet::Iterator it = enabled_types.First(); it.Good(); it.Inc()) {
     SyncDirectoryUpdateHandler* handler =
-        new SyncDirectoryUpdateHandler(directory(), it.Get());
-    update_handler_map_.insert(std::make_pair(it.Get(), handler));
+        new SyncDirectoryUpdateHandler(directory(), type, worker);
+    update_handler_map_.insert(std::make_pair(type, handler));
+
+    SyncDirectoryCommitContributor* contributor =
+        new SyncDirectoryCommitContributor(directory(), type);
+    commit_contributor_map_.insert(std::make_pair(type, contributor));
   }
 }
 
diff --git a/sync/sessions/sync_session_context.h b/sync/sessions/sync_session_context.h
index d195943..5995ab1 100644
--- a/sync/sessions/sync_session_context.h
+++ b/sync/sessions/sync_session_context.h
@@ -3,15 +3,12 @@
 // found in the LICENSE file.
 
 // SyncSessionContext encapsulates the contextual information and engine
-// components specific to a SyncSession. A context is accessible via
-// a SyncSession so that session SyncerCommands and parts of the engine have
-// a convenient way to access other parts. In this way it can be thought of as
-// the surrounding environment for the SyncSession. The components of this
-// environment are either valid or not valid for the entire context lifetime,
-// or they are valid for explicitly scoped periods of time by using Scoped
-// installation utilities found below. This means that the context assumes no
-// ownership whatsoever of any object that was not created by the context
-// itself.
+// components specific to a SyncSession.  Unlike the SyncSession, the context
+// can be reused across several sync cycles.
+//
+// The context does not take ownership of its pointer members.  It's up to
+// the surrounding classes to ensure those members remain valid while the
+// context is in use.
 //
 // It can only be used from the SyncerThread.
 
@@ -70,8 +67,8 @@ class SYNC_EXPORT_PRIVATE SyncSessionContext {
     return directory_;
   }
 
-  const ModelSafeRoutingInfo& routing_info() const {
-    return routing_info_;
+  ModelTypeSet enabled_types() const {
+    return enabled_types_;
   }
 
   void set_routing_info(const ModelSafeRoutingInfo& routing_info);
@@ -84,10 +81,6 @@ class SYNC_EXPORT_PRIVATE SyncSessionContext {
     return &commit_contributor_map_;
   }
 
-  const std::vector<scoped_refptr<ModelSafeWorker> >& workers() const {
-    return workers_;
-  }
-
   ExtensionsActivity* extensions_activity() {
     return extensions_activity_.get();
   }
@@ -159,9 +152,9 @@ class SYNC_EXPORT_PRIVATE SyncSessionContext {
   ServerConnectionManager* const connection_manager_;
   syncable::Directory* const directory_;
 
-  // A cached copy of SyncBackendRegistrar's routing info.
-  // Must be updated manually when SBR's state is modified.
-  ModelSafeRoutingInfo routing_info_;
+  // The set of enabled types.  Derrived from the routing info set with
+  // set_routing_info().
+  ModelTypeSet enabled_types_;
 
   // A map of 'update handlers', one for each enabled type.
   // This must be kept in sync with the routing info.  Our temporary solution to
@@ -180,7 +173,7 @@ class SYNC_EXPORT_PRIVATE SyncSessionContext {
   STLValueDeleter<CommitContributorMap> commit_contributor_deleter_;
 
   // The set of ModelSafeWorkers.  Used to execute tasks of various threads.
-  std::vector<scoped_refptr<ModelSafeWorker> > workers_;
+  std::map<ModelSafeGroup, scoped_refptr<ModelSafeWorker> > workers_;
 
   // We use this to stuff extensions activity into CommitMessages so the server
   // can correlate commit traffic with extension-related bookmark mutations.
diff --git a/sync/sync_core.gypi b/sync/sync_core.gypi
index 5d6d989..f4912fe 100644
--- a/sync/sync_core.gypi
+++ b/sync/sync_core.gypi
@@ -32,8 +32,6 @@
     'engine/all_status.h',
     'engine/apply_control_data_updates.cc',
     'engine/apply_control_data_updates.h',
-    'engine/apply_updates_and_resolve_conflicts_command.cc',
-    'engine/apply_updates_and_resolve_conflicts_command.h',
     'engine/backoff_delay_provider.cc',
     'engine/backoff_delay_provider.h',
     'engine/commit_util.cc',
@@ -54,8 +52,6 @@
     'engine/download.h',
     'engine/get_commit_ids.cc',
     'engine/get_commit_ids.h',
-    'engine/model_changing_syncer_command.cc',
-    'engine/model_changing_syncer_command.h',
     'engine/net/server_connection_manager.cc',
     'engine/net/server_connection_manager.h',
     'engine/net/url_translator.cc',
@@ -72,8 +68,6 @@
     'engine/sync_scheduler_impl.h',
     'engine/syncer.cc',
     'engine/syncer.h',
-    'engine/syncer_command.cc',
-    'engine/syncer_command.h',
     'engine/syncer_proto_util.cc',
     'engine/syncer_proto_util.h',
     'engine/syncer_types.h',
diff --git a/sync/sync_tests.gypi b/sync/sync_tests.gypi
index 010cb5b..683ca52 100644
--- a/sync/sync_tests.gypi
+++ b/sync/sync_tests.gypi
@@ -40,8 +40,6 @@
         'test/engine/fake_sync_scheduler.h',
         'test/engine/mock_connection_manager.cc',
         'test/engine/mock_connection_manager.h',
-        'test/engine/syncer_command_test.cc',
-        'test/engine/syncer_command_test.h',
         'test/engine/test_directory_setter_upper.cc',
         'test/engine/test_directory_setter_upper.h',
         'test/engine/test_id_factory.h',
@@ -243,10 +241,8 @@
           'internal_api/public/util/immutable_unittest.cc',
           'internal_api/public/util/weak_handle_unittest.cc',
           'engine/apply_control_data_updates_unittest.cc',
-          'engine/apply_updates_and_resolve_conflicts_command_unittest.cc',
           'engine/backoff_delay_provider_unittest.cc',
           'engine/download_unittest.cc',
-          'engine/model_changing_syncer_command_unittest.cc',
           'engine/sync_scheduler_unittest.cc',
           'engine/syncer_proto_util_unittest.cc',
           'engine/syncer_unittest.cc',
diff --git a/sync/syncable/directory.cc b/sync/syncable/directory.cc
index 85e5107..33b7e15 100644
--- a/sync/syncable/directory.cc
+++ b/sync/syncable/directory.cc
@@ -910,17 +910,9 @@ int64 Directory::unsynced_entity_count() const {
   return kernel_->unsynced_metahandles.size();
 }
 
-FullModelTypeSet Directory::GetServerTypesWithUnappliedUpdates(
-    BaseTransaction* trans) const {
-  FullModelTypeSet server_types;
+bool Directory::TypeHasUnappliedUpdates(ModelType type) {
   ScopedKernelLock lock(this);
-  for (int i = UNSPECIFIED; i < MODEL_TYPE_COUNT; ++i) {
-    const ModelType type = ModelTypeFromInt(i);
-    if (!kernel_->unapplied_update_metahandles[type].empty()) {
-      server_types.Put(type);
-    }
-  }
-  return server_types;
+  return !kernel_->unapplied_update_metahandles[type].empty();
 }
 
 void Directory::GetUnappliedUpdateMetaHandles(
diff --git a/sync/syncable/directory.h b/sync/syncable/directory.h
index 7a58dfc..0206dbb 100644
--- a/sync/syncable/directory.h
+++ b/sync/syncable/directory.h
@@ -315,11 +315,8 @@ class SYNC_EXPORT Directory {
   void GetUnsyncedMetaHandles(BaseTransaction* trans,
                               Metahandles* result);
 
-  // Returns all server types with unapplied updates.  A subset of
-  // those types can then be passed into
-  // GetUnappliedUpdateMetaHandles() below.
-  FullModelTypeSet GetServerTypesWithUnappliedUpdates(
-      BaseTransaction* trans) const;
+  // Returns whether or not this |type| has unapplied updates.
+  bool TypeHasUnappliedUpdates(ModelType type);
 
   // Get all the metahandles for unapplied updates for a given set of
   // server types.
diff --git a/sync/test/engine/syncer_command_test.cc b/sync/test/engine/syncer_command_test.cc
deleted file mode 100644
index 8cf8e13..0000000
--- a/sync/test/engine/syncer_command_test.cc
+++ /dev/null
@@ -1,83 +0,0 @@
-// Copyright (c) 2012 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 "sync/test/engine/syncer_command_test.h"
-
-namespace syncer {
-
-const unsigned int kMaxMessages = 10;
-const unsigned int kMaxMessageSize = 5 * 1024;
-
-void SyncerCommandTestBase::OnThrottled(
-    const base::TimeDelta& throttle_duration) {
-  FAIL() << "Should not get silenced.";
-}
-
-void SyncerCommandTestBase::OnTypesThrottled(
-    ModelTypeSet types,
-    const base::TimeDelta& throttle_duration) {
-  FAIL() << "Should not get silenced.";
-}
-
-bool SyncerCommandTestBase::IsCurrentlyThrottled() {
-  return false;
-}
-
-void SyncerCommandTestBase::OnReceivedLongPollIntervalUpdate(
-    const base::TimeDelta& new_interval) {
-  FAIL() << "Should not get poll interval update.";
-}
-
-void SyncerCommandTestBase::OnReceivedShortPollIntervalUpdate(
-    const base::TimeDelta& new_interval) {
-  FAIL() << "Should not get poll interval update.";
-}
-
-void SyncerCommandTestBase::OnReceivedSessionsCommitDelay(
-    const base::TimeDelta& new_delay) {
-  FAIL() << "Should not get sessions commit delay.";
-}
-
-void SyncerCommandTestBase::OnReceivedClientInvalidationHintBufferSize(
-    int size) {
-  FAIL() << "Should not get hint buffer size.";
-}
-
-void SyncerCommandTestBase::OnSyncProtocolError(
-    const sessions::SyncSessionSnapshot& session) {
-  return;
-}
-SyncerCommandTestBase::SyncerCommandTestBase()
-    : traffic_recorder_(kMaxMessages, kMaxMessageSize) {
-}
-
-SyncerCommandTestBase::~SyncerCommandTestBase() {
-}
-
-void SyncerCommandTestBase::SetUp() {
-  extensions_activity_ = new ExtensionsActivity();
-
-  // The session always expects there to be a passive worker.
-  workers()->push_back(
-      make_scoped_refptr(new FakeModelWorker(GROUP_PASSIVE)));
-  ResetContext();
-}
-
-void SyncerCommandTestBase::TearDown() {
-}
-
-syncable::Directory* SyncerCommandTest::directory() {
-  return dir_maker_.directory();
-}
-
-void SyncerCommandTest::SetUp() {
-  dir_maker_.SetUp();
-  SyncerCommandTestBase::SetUp();
-}
-
-void SyncerCommandTest::TearDown() {
-  dir_maker_.TearDown();
-}
-
-}  // namespace syncer
diff --git a/sync/test/engine/syncer_command_test.h b/sync/test/engine/syncer_command_test.h
deleted file mode 100644
index 0f106c5..0000000
--- a/sync/test/engine/syncer_command_test.h
+++ /dev/null
@@ -1,189 +0,0 @@
-// Copyright (c) 2012 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 SYNC_TEST_ENGINE_SYNCER_COMMAND_TEST_H_
-#define SYNC_TEST_ENGINE_SYNCER_COMMAND_TEST_H_
-
-#include <algorithm>
-#include <string>
-#include <vector>
-
-#include "base/compiler_specific.h"
-#include "base/memory/ref_counted.h"
-#include "base/message_loop/message_loop.h"
-#include "sync/engine/model_changing_syncer_command.h"
-#include "sync/engine/traffic_recorder.h"
-#include "sync/internal_api/debug_info_event_listener.h"
-#include "sync/internal_api/public/base/cancelation_signal.h"
-#include "sync/internal_api/public/engine/model_safe_worker.h"
-#include "sync/sessions/debug_info_getter.h"
-#include "sync/sessions/sync_session.h"
-#include "sync/sessions/sync_session_context.h"
-#include "sync/syncable/directory.h"
-#include "sync/test/engine/fake_model_worker.h"
-#include "sync/test/engine/mock_connection_manager.h"
-#include "sync/test/engine/test_directory_setter_upper.h"
-#include "sync/util/extensions_activity.h"
-#include "testing/gmock/include/gmock/gmock.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace syncer {
-
-// A test fixture that simplifies writing unit tests for individual
-// SyncerCommands, providing convenient access to a test directory
-// and a syncer session.
-class SyncerCommandTestBase : public testing::Test,
-                              public sessions::SyncSession::Delegate {
- public:
-  // SyncSession::Delegate implementation.
-  virtual void OnThrottled(
-      const base::TimeDelta& throttle_duration) OVERRIDE;
-  virtual void OnTypesThrottled(
-      ModelTypeSet types,
-      const base::TimeDelta& throttle_duration) OVERRIDE;
-  virtual bool IsCurrentlyThrottled() OVERRIDE;
-  virtual void OnReceivedLongPollIntervalUpdate(
-      const base::TimeDelta& new_interval) OVERRIDE;
-  virtual void OnReceivedShortPollIntervalUpdate(
-      const base::TimeDelta& new_interval) OVERRIDE;
-  virtual void OnReceivedSessionsCommitDelay(
-      const base::TimeDelta& new_delay) OVERRIDE;
-  virtual void OnReceivedClientInvalidationHintBufferSize(int size) OVERRIDE;
-  virtual void OnSyncProtocolError(
-      const sessions::SyncSessionSnapshot& session) OVERRIDE;
-
-  std::vector<ModelSafeWorker*> GetWorkers() {
-    std::vector<ModelSafeWorker*> workers;
-    std::vector<scoped_refptr<ModelSafeWorker> >::iterator it;
-    for (it = workers_.begin(); it != workers_.end(); ++it)
-      workers.push_back(it->get());
-    return workers;
-  }
-  void GetModelSafeRoutingInfo(ModelSafeRoutingInfo* out) {
-    ModelSafeRoutingInfo copy(routing_info_);
-    out->swap(copy);
-  }
-
- protected:
-  SyncerCommandTestBase();
-
-  virtual ~SyncerCommandTestBase();
-  virtual void SetUp();
-  virtual void TearDown();
-
-  sessions::SyncSessionContext* context() const { return context_.get(); }
-  sessions::SyncSession::Delegate* delegate() { return this; }
-
-  // Lazily create a session requesting all datatypes with no state.
-  sessions::SyncSession* session() {
-    if (!session_.get())
-      session_.reset(sessions::SyncSession::Build(context(), delegate()));
-    return session_.get();
-  }
-
-  void ClearSession() {
-    session_.reset();
-  }
-
-  void ResetContext() {
-    context_.reset(new sessions::SyncSessionContext(
-            mock_server_.get(), directory(),
-            GetWorkers(), extensions_activity_.get(),
-            std::vector<SyncEngineEventListener*>(),
-            &debug_info_event_listener_,
-            &traffic_recorder_,
-            true,  // enable keystore encryption
-            false,  // force enable pre-commit GU avoidance experiment
-            "fake_invalidator_client_id"));
-    context_->set_routing_info(routing_info_);
-    context_->set_account_name(directory()->name());
-    ClearSession();
-  }
-
-  // Install a MockServerConnection.  Resets the context.  By default,
-  // the context does not have a MockServerConnection attached.
-  void ConfigureMockServerConnection() {
-    mock_server_.reset(new MockConnectionManager(directory(),
-                                                 &cancelation_signal_));
-    ResetContext();
-  }
-
-  virtual syncable::Directory* directory() = 0;
-
-  std::vector<scoped_refptr<ModelSafeWorker> >* workers() {
-    return &workers_;
-  }
-
-  const ModelSafeRoutingInfo& routing_info() { return routing_info_; }
-  ModelSafeRoutingInfo* mutable_routing_info() { return &routing_info_; }
-
-  MockConnectionManager* mock_server() {
-    return mock_server_.get();
-  }
-
-  DebugInfoEventListener* debug_info_event_listener() {
-    return &debug_info_event_listener_;
-  }
-  // Helper functions to check command.GetGroupsToChange().
-
-  void ExpectNoGroupsToChange(const ModelChangingSyncerCommand& command) {
-    EXPECT_TRUE(command.GetGroupsToChangeForTest(*session()).empty());
-  }
-
-  void ExpectGroupToChange(
-      const ModelChangingSyncerCommand& command, ModelSafeGroup group) {
-    std::set<ModelSafeGroup> expected_groups_to_change;
-    expected_groups_to_change.insert(group);
-    EXPECT_EQ(expected_groups_to_change,
-              command.GetGroupsToChangeForTest(*session()));
-  }
-
-  void ExpectGroupsToChange(
-      const ModelChangingSyncerCommand& command,
-      ModelSafeGroup group1, ModelSafeGroup group2) {
-    std::set<ModelSafeGroup> expected_groups_to_change;
-    expected_groups_to_change.insert(group1);
-    expected_groups_to_change.insert(group2);
-    EXPECT_EQ(expected_groups_to_change,
-              command.GetGroupsToChangeForTest(*session()));
-  }
-
-  void ExpectGroupsToChange(
-      const ModelChangingSyncerCommand& command,
-      ModelSafeGroup group1, ModelSafeGroup group2, ModelSafeGroup group3) {
-    std::set<ModelSafeGroup> expected_groups_to_change;
-    expected_groups_to_change.insert(group1);
-    expected_groups_to_change.insert(group2);
-    expected_groups_to_change.insert(group3);
-    EXPECT_EQ(expected_groups_to_change,
-              command.GetGroupsToChangeForTest(*session()));
-  }
-
- private:
-  base::MessageLoop message_loop_;
-  scoped_ptr<sessions::SyncSessionContext> context_;
-  scoped_ptr<MockConnectionManager> mock_server_;
-  scoped_ptr<sessions::SyncSession> session_;
-  std::vector<scoped_refptr<ModelSafeWorker> > workers_;
-  ModelSafeRoutingInfo routing_info_;
-  DebugInfoEventListener debug_info_event_listener_;
-  scoped_refptr<ExtensionsActivity> extensions_activity_;
-  TrafficRecorder traffic_recorder_;
-  CancelationSignal cancelation_signal_;
-  DISALLOW_COPY_AND_ASSIGN(SyncerCommandTestBase);
-};
-
-class SyncerCommandTest : public SyncerCommandTestBase {
- public:
-  virtual void SetUp() OVERRIDE;
-  virtual void TearDown() OVERRIDE;
-  virtual syncable::Directory* directory() OVERRIDE;
-
- private:
-  TestDirectorySetterUpper dir_maker_;
-};
-
-}  // namespace syncer
-
-#endif  // SYNC_TEST_ENGINE_SYNCER_COMMAND_TEST_H_