sync: Remove the remaining conflict sets code

Conflict sets were intended to help us deal with updates that could
corrupt the directory tree if applied.  This could happen, for example,
when the server sends us a new item in a directory that has been deleted
locally.

Due to a bug, the code to deal with conflict sets was never run.  It
seems that the consequences of this weren't as bad as one would expect.
By this point, it's not worth fixing, since we are a few weeks away from
adding code to handle conflict sets entirely on the server side.

Over a series of commits, we have removed lots of dead code related to
conflict set handling.  The only remaining function of conflict sets
code was to prevent items that were in both a "conflict set" and a
"simple conflict" state from being processed as simple conflicts.  If
those items had been processed as simple conflicts, there is a small
chance we could accidentally apply them, or do other bad things.

The code that detects conflict sets is overkill for our current needs,
because it was built with the idea that we had to gather information
about these conflict sets in order to resolve them.  Now that this
requirement has been removed, I have been able to greatly simplify the
implementation of conflict set detection by moving it to the update
application code.  Since this was the last remaining purpose of the
conflict set processing code, it is now safe to entirely remove the
concept of conflict sets.

Other changes:
- Removed ConflictProgress::EraseNonBlockingConflictingItemById(); the
  function was never used.
- Added some unit tests for the new functionality.

BUG=107816
TEST=sync_unit_tests


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

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

26 files changed, 564 insertions, 789 deletions

diff --git a/chrome/browser/sync/engine/apply_updates_command_unittest.cc b/chrome/browser/sync/engine/apply_updates_command_unittest.cc
index 19bc480..40d9df1 100644
--- a/chrome/browser/sync/engine/apply_updates_command_unittest.cc
+++ b/chrome/browser/sync/engine/apply_updates_command_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -143,6 +143,88 @@ class ApplyUpdatesCommandTest : public SyncerCommandTest {
       *metahandle_out = entry.Get(syncable::META_HANDLE);
   }
 
+  // Creates an item that is both unsynced an an unapplied update.  Returns the
+  // metahandle of the created item.
+  int64 CreateUnappliedAndUnsyncedItem(const string& name,
+                                       syncable::ModelType model_type) {
+    int64 metahandle = 0;
+    CreateUnsyncedItem(id_factory_.MakeServer(name), id_factory_.root(), name,
+                       false, model_type, &metahandle);
+
+    ScopedDirLookup dir(syncdb()->manager(), syncdb()->name());
+    if (!dir.good()) {
+      ADD_FAILURE();
+      return syncable::kInvalidMetaHandle;
+    }
+    WriteTransaction trans(FROM_HERE, UNITTEST, dir);
+    MutableEntry entry(&trans, syncable::GET_BY_HANDLE, metahandle);
+    if (!entry.good()) {
+      ADD_FAILURE();
+      return syncable::kInvalidMetaHandle;
+    }
+
+    entry.Put(syncable::IS_UNAPPLIED_UPDATE, true);
+    entry.Put(syncable::SERVER_VERSION, GetNextRevision());
+
+    return metahandle;
+  }
+
+
+  // Creates an item that has neither IS_UNSYNED or IS_UNAPPLIED_UPDATE.  The
+  // item is known to both the server and client.  Returns the metahandle of
+  // the created item.
+  int64 CreateSyncedItem(const std::string& name, syncable::ModelType
+                         model_type, bool is_folder) {
+    ScopedDirLookup dir(syncdb()->manager(), syncdb()->name());
+    if (!dir.good()) {
+      ADD_FAILURE();
+      return syncable::kInvalidMetaHandle;
+    }
+    WriteTransaction trans(FROM_HERE, UNITTEST, dir);
+
+    syncable::Id parent_id(id_factory_.root());
+    syncable::Id item_id(id_factory_.MakeServer(name));
+    int64 version = GetNextRevision();
+
+    sync_pb::EntitySpecifics default_specifics;
+    syncable::AddDefaultExtensionValue(model_type, &default_specifics);
+
+    MutableEntry entry(&trans, syncable::CREATE, parent_id, name);
+    if (!entry.good()) {
+      ADD_FAILURE();
+      return syncable::kInvalidMetaHandle;
+    }
+
+    entry.Put(syncable::ID, item_id);
+    entry.Put(syncable::BASE_VERSION, version);
+    entry.Put(syncable::IS_UNSYNCED, false);
+    entry.Put(syncable::NON_UNIQUE_NAME, name);
+    entry.Put(syncable::IS_DIR, is_folder);
+    entry.Put(syncable::IS_DEL, false);
+    entry.Put(syncable::PARENT_ID, parent_id);
+
+    if (!entry.PutPredecessor(id_factory_.root())) {
+      ADD_FAILURE();
+      return syncable::kInvalidMetaHandle;
+    }
+    entry.Put(syncable::SPECIFICS, default_specifics);
+
+    entry.Put(syncable::SERVER_VERSION, GetNextRevision());
+    entry.Put(syncable::IS_UNAPPLIED_UPDATE, true);
+    entry.Put(syncable::SERVER_NON_UNIQUE_NAME, "X");
+    entry.Put(syncable::SERVER_PARENT_ID, id_factory_.MakeServer("Y"));
+    entry.Put(syncable::SERVER_IS_DIR, is_folder);
+    entry.Put(syncable::SERVER_IS_DEL, false);
+    entry.Put(syncable::SERVER_SPECIFICS, default_specifics);
+    entry.Put(syncable::SERVER_PARENT_ID, parent_id);
+
+    return entry.Get(syncable::META_HANDLE);
+  }
+
+  int64 GetNextRevision() {
+    return next_revision_++;
+  }
+
   ApplyUpdatesCommand apply_updates_command_;
   TestIdFactory id_factory_;
  private:
@@ -169,7 +251,11 @@ TEST_F(ApplyUpdatesCommandTest, Simple) {
   EXPECT_EQ(2, status->update_progress()->AppliedUpdatesSize())
       << "All updates should have been attempted";
   ASSERT_TRUE(status->conflict_progress());
-  EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize())
+  EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize())
+      << "Simple update shouldn't result in conflicts";
+  EXPECT_EQ(0, status->conflict_progress()->EncryptionConflictingItemsSize())
+      << "Simple update shouldn't result in conflicts";
+  EXPECT_EQ(0, status->conflict_progress()->HierarchyConflictingItemsSize())
       << "Simple update shouldn't result in conflicts";
   EXPECT_EQ(2, status->update_progress()->SuccessfullyAppliedUpdateCount())
       << "All items should have been successfully applied";
@@ -204,13 +290,197 @@ TEST_F(ApplyUpdatesCommandTest, UpdateWithChildrenBeforeParents) {
   EXPECT_EQ(5, status->update_progress()->AppliedUpdatesSize())
       << "All updates should have been attempted";
   ASSERT_TRUE(status->conflict_progress());
-  EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize())
+  EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize())
       << "Simple update shouldn't result in conflicts, even if out-of-order";
   EXPECT_EQ(5, status->update_progress()->SuccessfullyAppliedUpdateCount())
       << "All updates should have been successfully applied";
 }
 
-TEST_F(ApplyUpdatesCommandTest, NestedItemsWithUnknownParent) {
+// Runs the ApplyUpdatesCommand 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(ApplyUpdatesCommandTest, SimpleConflict) {
+  CreateUnappliedAndUnsyncedItem("item", syncable::BOOKMARKS);
+
+  ExpectGroupToChange(apply_updates_command_, GROUP_UI);
+  apply_updates_command_.ExecuteImpl(session());
+
+  sessions::StatusController* status = session()->mutable_status_controller();
+  sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI);
+  ASSERT_TRUE(status->conflict_progress());
+  EXPECT_EQ(1, status->conflict_progress()->SimpleConflictingItemsSize())
+      << "Unsynced and unapplied item should be a simple conflict";
+}
+
+// Runs the ApplyUpdatesCommand 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(ApplyUpdatesCommandTest, HierarchyAndSimpleConflict) {
+  // Create a simply-conflicting item.  It will start with valid parent ids.
+  int64 handle = CreateUnappliedAndUnsyncedItem("orphaned_by_server",
+                                                syncable::BOOKMARKS);
+  {
+    // Manually set the SERVER_PARENT_ID to bad value.
+    // A bad parent indicates a hierarchy conflict.
+    ScopedDirLookup dir(syncdb()->manager(), syncdb()->name());
+    ASSERT_TRUE(dir.good());
+    WriteTransaction trans(FROM_HERE, UNITTEST, dir);
+    MutableEntry entry(&trans, syncable::GET_BY_HANDLE, handle);
+    ASSERT_TRUE(entry.good());
+
+    entry.Put(syncable::SERVER_PARENT_ID,
+              id_factory_.MakeServer("bogus_parent"));
+  }
+
+  ExpectGroupToChange(apply_updates_command_, GROUP_UI);
+  apply_updates_command_.ExecuteImpl(session());
+
+  sessions::StatusController* status = session()->mutable_status_controller();
+  sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI);
+
+  EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize());
+
+  // An update that is both a simple conflict and a hierarchy conflict should be
+  // treated as a hierarchy conflict.
+  ASSERT_TRUE(status->conflict_progress());
+  EXPECT_EQ(1, status->conflict_progress()->HierarchyConflictingItemsSize());
+  EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize());
+}
+
+
+// Runs the ApplyUpdatesCommand 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(ApplyUpdatesCommandTest, 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 = CreateSyncedItem("X", syncable::BOOKMARKS, true);
+
+    ScopedDirLookup dir(syncdb()->manager(), syncdb()->name());
+    ASSERT_TRUE(dir.good());
+    WriteTransaction trans(FROM_HERE, UNITTEST, dir);
+    MutableEntry entry(&trans, syncable::GET_BY_HANDLE, handle);
+    ASSERT_TRUE(entry.good());
+
+    // Re-parent from root to "Y"
+    entry.Put(syncable::SERVER_VERSION, GetNextRevision());
+    entry.Put(syncable::IS_UNAPPLIED_UPDATE, true);
+    entry.Put(syncable::SERVER_PARENT_ID, id_factory_.MakeServer("Y"));
+  }
+
+  // Item 'Y' is child of 'X'.
+  CreateUnsyncedItem(id_factory_.MakeServer("Y"), id_factory_.MakeServer("X"),
+                     "Y", true, syncable::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());
+
+  sessions::StatusController* status = session()->mutable_status_controller();
+  sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI);
+
+  EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize());
+
+  // This should count as a hierarchy conflict.
+  ASSERT_TRUE(status->conflict_progress());
+  EXPECT_EQ(1, status->conflict_progress()->HierarchyConflictingItemsSize());
+  EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize());
+}
+
+// Runs the ApplyUpdatesCommand 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(ApplyUpdatesCommandTest, HierarchyConflictDeletedParent) {
+  // Create a locally deleted parent item.
+  int64 parent_handle;
+  CreateUnsyncedItem(Id::CreateFromServerId("parent"), id_factory_.root(),
+                     "parent", true, syncable::BOOKMARKS, &parent_handle);
+  {
+    ScopedDirLookup dir(syncdb()->manager(), syncdb()->name());
+    ASSERT_TRUE(dir.good());
+    WriteTransaction trans(FROM_HERE, UNITTEST, dir);
+    MutableEntry entry(&trans, syncable::GET_BY_HANDLE, parent_handle);
+    entry.Put(syncable::IS_DEL, true);
+  }
+
+  // Create an incoming child from the server.
+  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());
+
+  sessions::StatusController* status = session()->mutable_status_controller();
+  sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI);
+
+  // This should count as a hierarchy conflict.
+  ASSERT_TRUE(status->conflict_progress());
+  EXPECT_EQ(1, status->conflict_progress()->HierarchyConflictingItemsSize());
+  EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize());
+}
+
+// Runs the ApplyUpdatesCommand 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(ApplyUpdatesCommandTest, HierarchyConflictDeleteNonEmptyDirectory) {
+  // Create a server-deleted directory.
+  {
+    // Create it as a child of root node.
+    int64 handle = CreateSyncedItem("parent", syncable::BOOKMARKS, true);
+
+    ScopedDirLookup dir(syncdb()->manager(), syncdb()->name());
+    ASSERT_TRUE(dir.good());
+    WriteTransaction trans(FROM_HERE, UNITTEST, dir);
+    MutableEntry entry(&trans, syncable::GET_BY_HANDLE, handle);
+    ASSERT_TRUE(entry.good());
+
+    // Delete it on the server.
+    entry.Put(syncable::SERVER_VERSION, GetNextRevision());
+    entry.Put(syncable::IS_UNAPPLIED_UPDATE, true);
+    entry.Put(syncable::SERVER_PARENT_ID, id_factory_.root());
+    entry.Put(syncable::SERVER_IS_DEL, true);
+  }
+
+  // Create a local child of the server-deleted directory.
+  CreateUnsyncedItem(id_factory_.MakeServer("child"),
+                     id_factory_.MakeServer("parent"), "child", false,
+                     syncable::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());
+
+  sessions::StatusController* status = session()->mutable_status_controller();
+  sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI);
+
+  // This should count as a hierarchy conflict.
+  ASSERT_TRUE(status->conflict_progress());
+  EXPECT_EQ(1, status->conflict_progress()->HierarchyConflictingItemsSize());
+  EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize());
+}
+
+// Runs the ApplyUpdatesCommand 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(ApplyUpdatesCommandTest, HierarchyConflictUnknownParent) {
   // We shouldn't be able to do anything with either of these items.
   CreateUnappliedNewItemWithParent("some_item",
                                    DefaultBookmarkSpecifics(),
@@ -228,7 +498,10 @@ TEST_F(ApplyUpdatesCommandTest, NestedItemsWithUnknownParent) {
   EXPECT_EQ(2, status->update_progress()->AppliedUpdatesSize())
       << "All updates should have been attempted";
   ASSERT_TRUE(status->conflict_progress());
-  EXPECT_EQ(2, status->conflict_progress()->ConflictingItemsSize())
+  EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize())
+      << "Updates with unknown parent should not be treated as 'simple'"
+      << " conflicts";
+  EXPECT_EQ(2, status->conflict_progress()->HierarchyConflictingItemsSize())
       << "All updates with an unknown ancestors should be in conflict";
   EXPECT_EQ(0, status->update_progress()->SuccessfullyAppliedUpdateCount())
       << "No item with an unknown ancestor should be applied";
@@ -265,7 +538,7 @@ TEST_F(ApplyUpdatesCommandTest, ItemsBothKnownAndUnknown) {
   EXPECT_EQ(6, status->update_progress()->AppliedUpdatesSize())
       << "All updates should have been attempted";
   ASSERT_TRUE(status->conflict_progress());
-  EXPECT_EQ(2, status->conflict_progress()->ConflictingItemsSize())
+  EXPECT_EQ(2, status->conflict_progress()->HierarchyConflictingItemsSize())
       << "The updates with unknown ancestors should be in conflict";
   EXPECT_EQ(4, status->update_progress()->SuccessfullyAppliedUpdateCount())
       << "The updates with known ancestors should be successfully applied";
@@ -304,7 +577,7 @@ TEST_F(ApplyUpdatesCommandTest, DecryptablePassword) {
   EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize())
       << "All updates should have been attempted";
   ASSERT_TRUE(status->conflict_progress());
-  EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize())
+  EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize())
       << "No update should be in conflict because they're all decryptable";
   EXPECT_EQ(1, status->update_progress()->SuccessfullyAppliedUpdateCount())
       << "The updates that can be decrypted should be applied";
@@ -337,11 +610,11 @@ TEST_F(ApplyUpdatesCommandTest, UndecryptableData) {
     EXPECT_EQ(2, status->update_progress()->AppliedUpdatesSize())
         << "All updates should have been attempted";
     ASSERT_TRUE(status->conflict_progress());
-    EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize())
+    EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize())
         << "The updates that can't be decrypted should not be in regular "
         << "conflict";
-    EXPECT_EQ(2, status->conflict_progress()->NonblockingConflictingItemsSize())
-        << "The updates that can't be decrypted should be in nonblocking "
+    EXPECT_EQ(2, status->conflict_progress()->EncryptionConflictingItemsSize())
+        << "The updates that can't be decrypted should be in encryption "
         << "conflict";
     EXPECT_EQ(0, status->update_progress()->SuccessfullyAppliedUpdateCount())
         << "No update that can't be decrypted should be applied";
@@ -352,11 +625,11 @@ TEST_F(ApplyUpdatesCommandTest, UndecryptableData) {
     EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize())
         << "All updates should have been attempted";
   ASSERT_TRUE(status->conflict_progress());
-    EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize())
+    EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize())
         << "The updates that can't be decrypted should not be in regular "
         << "conflict";
-    EXPECT_EQ(1, status->conflict_progress()->NonblockingConflictingItemsSize())
-        << "The updates that can't be decrypted should be in nonblocking "
+    EXPECT_EQ(1, status->conflict_progress()->EncryptionConflictingItemsSize())
+        << "The updates that can't be decrypted should be in encryption "
         << "conflict";
     EXPECT_EQ(0, status->update_progress()->SuccessfullyAppliedUpdateCount())
         << "No update that can't be decrypted should be applied";
@@ -412,11 +685,11 @@ TEST_F(ApplyUpdatesCommandTest, SomeUndecryptablePassword) {
     EXPECT_EQ(2, status->update_progress()->AppliedUpdatesSize())
         << "All updates should have been attempted";
     ASSERT_TRUE(status->conflict_progress());
-    EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize())
+    EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize())
         << "The updates that can't be decrypted should not be in regular "
         << "conflict";
-    EXPECT_EQ(1, status->conflict_progress()->NonblockingConflictingItemsSize())
-        << "The updates that can't be decrypted should be in nonblocking "
+    EXPECT_EQ(1, status->conflict_progress()->EncryptionConflictingItemsSize())
+        << "The updates that can't be decrypted should be in encryption "
         << "conflict";
     EXPECT_EQ(1, status->update_progress()->SuccessfullyAppliedUpdateCount())
         << "The undecryptable password update shouldn't be applied";
@@ -463,7 +736,7 @@ TEST_F(ApplyUpdatesCommandTest, NigoriUpdate) {
   EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize())
       << "All updates should have been attempted";
   ASSERT_TRUE(status->conflict_progress());
-  EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize())
+  EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize())
       << "The nigori update shouldn't be in conflict";
   EXPECT_EQ(1, status->update_progress()->SuccessfullyAppliedUpdateCount())
       << "The nigori update should be applied";
@@ -517,7 +790,7 @@ TEST_F(ApplyUpdatesCommandTest, NigoriUpdateForDisabledTypes) {
   EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize())
       << "All updates should have been attempted";
   ASSERT_TRUE(status->conflict_progress());
-  EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize())
+  EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize())
       << "The nigori update shouldn't be in conflict";
   EXPECT_EQ(1, status->update_progress()->SuccessfullyAppliedUpdateCount())
       << "The nigori update should be applied";
@@ -607,9 +880,9 @@ TEST_F(ApplyUpdatesCommandTest, EncryptUnsyncedChanges) {
   EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize())
       << "All updates should have been attempted";
   ASSERT_TRUE(status->conflict_progress());
-  EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize())
+  EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize())
       << "No updates should be in conflict";
-  EXPECT_EQ(0, status->conflict_progress()->NonblockingConflictingItemsSize())
+  EXPECT_EQ(0, status->conflict_progress()->EncryptionConflictingItemsSize())
       << "No updates should be in conflict";
   EXPECT_EQ(1, status->update_progress()->SuccessfullyAppliedUpdateCount())
       << "The nigori update should be applied";
@@ -710,11 +983,11 @@ TEST_F(ApplyUpdatesCommandTest, CannotEncryptUnsyncedChanges) {
   EXPECT_EQ(1, status->update_progress()->AppliedUpdatesSize())
       << "All updates should have been attempted";
   ASSERT_TRUE(status->conflict_progress());
-  EXPECT_EQ(0, status->conflict_progress()->ConflictingItemsSize())
+  EXPECT_EQ(0, status->conflict_progress()->SimpleConflictingItemsSize())
       << "The unsynced changes don't trigger a blocking conflict with the "
       << "nigori update.";
-  EXPECT_EQ(0, status->conflict_progress()->NonblockingConflictingItemsSize())
-      << "The unsynced changes don't trigger a non-blocking conflict with the "
+  EXPECT_EQ(0, status->conflict_progress()->EncryptionConflictingItemsSize())
+      << "The unsynced changes don't trigger an encryption conflict with the "
       << "nigori update.";
   EXPECT_EQ(1, status->update_progress()->SuccessfullyAppliedUpdateCount())
       << "The nigori update should be applied";
diff --git a/chrome/browser/sync/engine/build_and_process_conflict_sets_command.cc b/chrome/browser/sync/engine/build_and_process_conflict_sets_command.cc
deleted file mode 100644
index 3db89f0..0000000
--- a/chrome/browser/sync/engine/build_and_process_conflict_sets_command.cc
+++ /dev/null
@@ -1,238 +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 "chrome/browser/sync/engine/build_and_process_conflict_sets_command.h"
-
-#include <set>
-#include <string>
-#include <sstream>
-#include <vector>
-
-#include "base/basictypes.h"
-#include "base/format_macros.h"
-#include "base/location.h"
-#include "chrome/browser/sync/engine/syncer_util.h"
-#include "chrome/browser/sync/engine/update_applicator.h"
-#include "chrome/browser/sync/sessions/sync_session.h"
-#include "chrome/browser/sync/syncable/directory_manager.h"
-
-namespace browser_sync {
-
-using sessions::ConflictProgress;
-using sessions::StatusController;
-using sessions::SyncSession;
-using sessions::UpdateProgress;
-using std::set;
-using std::string;
-using std::vector;
-
-BuildAndProcessConflictSetsCommand::BuildAndProcessConflictSetsCommand() {}
-BuildAndProcessConflictSetsCommand::~BuildAndProcessConflictSetsCommand() {}
-
-std::set<ModelSafeGroup> BuildAndProcessConflictSetsCommand::GetGroupsToChange(
-    const sessions::SyncSession& session) const {
-  return session.GetEnabledGroupsWithConflicts();
-}
-
-SyncerError BuildAndProcessConflictSetsCommand::ModelChangingExecuteImpl(
-    SyncSession* session) {
-  syncable::ScopedDirLookup dir(session->context()->directory_manager(),
-                                session->context()->account_name());
-  if (!dir.good())
-    return DIRECTORY_LOOKUP_FAILED;
-
-  syncable::WriteTransaction trans(FROM_HERE, syncable::SYNCER, dir);
-  BuildConflictSets(&trans,
-      session->mutable_status_controller()->mutable_conflict_progress());
-
-  return SYNCER_OK;
-}
-
-void BuildAndProcessConflictSetsCommand::BuildConflictSets(
-    syncable::BaseTransaction* trans,
-    ConflictProgress* conflict_progress) {
-  conflict_progress->CleanupSets();
-  set<syncable::Id>::const_iterator i =
-      conflict_progress->ConflictingItemsBegin();
-  while (i != conflict_progress->ConflictingItemsEnd()) {
-    syncable::Entry entry(trans, syncable::GET_BY_ID, *i);
-    if (!entry.good() ||
-        (!entry.Get(syncable::IS_UNSYNCED) &&
-         !entry.Get(syncable::IS_UNAPPLIED_UPDATE))) {
-      // This can happen very rarely. It means we had a simply conflicting item
-      // that randomly committed; its ID could have changed during the commit.
-      // We drop the entry as it's no longer conflicting.
-      conflict_progress->EraseConflictingItemById(*(i++));
-      continue;
-    }
-    if (entry.ExistsOnClientBecauseNameIsNonEmpty() &&
-       (entry.Get(syncable::IS_DEL) || entry.Get(syncable::SERVER_IS_DEL))) {
-       // If we're deleted on client or server we can't be in a complex set.
-      ++i;
-      continue;
-    }
-    bool new_parent =
-        entry.Get(syncable::PARENT_ID) != entry.Get(syncable::SERVER_PARENT_ID);
-    if (new_parent)
-      MergeSetsForIntroducedLoops(trans, &entry, conflict_progress);
-    MergeSetsForNonEmptyDirectories(trans, &entry, conflict_progress);
-    ++i;
-  }
-}
-
-void BuildAndProcessConflictSetsCommand::MergeSetsForIntroducedLoops(
-    syncable::BaseTransaction* trans, syncable::Entry* entry,
-    ConflictProgress* conflict_progress) {
-  // This code crawls up from the item in question until it gets to the root
-  // or itself. If it gets to the root it does nothing. If it finds a loop all
-  // moved unsynced entries in the list of crawled entries have their sets
-  // merged with the entry.
-  // TODO(sync): Build test cases to cover this function when the argument list
-  // has settled.
-  syncable::Id parent_id = entry->Get(syncable::SERVER_PARENT_ID);
-  syncable::Entry parent(trans, syncable::GET_BY_ID, parent_id);
-  if (!parent.good()) {
-    return;
-  }
-  // Don't check for loop if the server parent is deleted.
-  if (parent.Get(syncable::IS_DEL))
-    return;
-  vector<syncable::Id> conflicting_entries;
-  while (!parent_id.IsRoot()) {
-    syncable::Entry parent(trans, syncable::GET_BY_ID, parent_id);
-    if (!parent.good()) {
-      DVLOG(1) << "Bad parent in loop check, skipping. Bad parent id: "
-               << parent_id << " entry: " << *entry;
-      return;
-    }
-    if (parent.Get(syncable::IS_UNSYNCED) &&
-        entry->Get(syncable::PARENT_ID) !=
-            entry->Get(syncable::SERVER_PARENT_ID))
-      conflicting_entries.push_back(parent_id);
-    parent_id = parent.Get(syncable::PARENT_ID);
-    if (parent_id == entry->Get(syncable::ID))
-      break;
-  }
-  if (parent_id.IsRoot())
-    return;
-  for (size_t i = 0; i < conflicting_entries.size(); i++) {
-    conflict_progress->MergeSets(entry->Get(syncable::ID),
-                                 conflicting_entries[i]);
-  }
-}
-
-namespace {
-
-class ServerDeletedPathChecker {
- public:
-  static bool CausingConflict(const syncable::Entry& e,
-                              const syncable::Entry& log_entry) {
-    CHECK(e.good()) << "Missing parent in path of: " << log_entry;
-    if (e.Get(syncable::IS_UNAPPLIED_UPDATE) &&
-        e.Get(syncable::SERVER_IS_DEL)) {
-      CHECK(!e.Get(syncable::IS_DEL)) << " Inconsistency in local tree. "
-                            "syncable::Entry: " << e << " Leaf: " << log_entry;
-      return true;
-    } else {
-      CHECK(!e.Get(syncable::IS_DEL)) << " Deleted entry has children. "
-                            "syncable::Entry: " << e << " Leaf: " << log_entry;
-      return false;
-    }
-  }
-
-  // returns 0 if we should stop investigating the path.
-  static syncable::Id GetAndExamineParent(syncable::BaseTransaction* trans,
-                                          const syncable::Id& id,
-                                          const syncable::Id& check_id,
-                                          const syncable::Entry& log_entry) {
-    syncable::Entry parent(trans, syncable::GET_BY_ID, id);
-    CHECK(parent.good()) << "Tree inconsitency, missing id" << id << " "
-        << log_entry;
-    syncable::Id parent_id = parent.Get(syncable::PARENT_ID);
-    CHECK(parent_id != check_id) << "Loop in dir tree! "
-      << log_entry << " " << parent;
-    return parent_id;
-  }
-};
-
-class LocallyDeletedPathChecker {
- public:
-  static bool CausingConflict(const syncable::Entry& e,
-                              const syncable::Entry& log_entry) {
-    return e.good() && e.Get(syncable::IS_DEL) && e.Get(syncable::IS_UNSYNCED);
-  }
-
-  // returns 0 if we should stop investigating the path.
-  static syncable::Id GetAndExamineParent(syncable::BaseTransaction* trans,
-                                          const syncable::Id& id,
-                                          const syncable::Id& check_id,
-                                          const syncable::Entry& log_entry) {
-    syncable::Entry parent(trans, syncable::GET_BY_ID, id);
-    if (!parent.good())
-      return syncable::GetNullId();
-    syncable::Id parent_id = parent.Get(syncable::PARENT_ID);
-    if (parent_id == check_id)
-      return syncable::GetNullId();
-    return parent_id;
-  }
-};
-
-template <typename Checker>
-void CrawlDeletedTreeMergingSets(syncable::BaseTransaction* trans,
-                                 const syncable::Entry& entry,
-                                 ConflictProgress* conflict_progress,
-                                 Checker checker) {
-  syncable::Id parent_id = entry.Get(syncable::PARENT_ID);
-  syncable::Id double_step_parent_id = parent_id;
-  // This block builds sets where we've got an entry in a directory the server
-  // wants to delete.
-  //
-  // Here we're walking up the tree to find all entries that the pass checks
-  // deleted. We can be extremely strict here as anything unexpected means
-  // invariants in the local hierarchy have been broken.
-  while (!parent_id.IsRoot()) {
-    if (!double_step_parent_id.IsRoot()) {
-      // Checks to ensure we don't loop.
-      double_step_parent_id = checker.GetAndExamineParent(
-          trans, double_step_parent_id, parent_id, entry);
-      double_step_parent_id = checker.GetAndExamineParent(
-          trans, double_step_parent_id, parent_id, entry);
-    }
-    syncable::Entry parent(trans, syncable::GET_BY_ID, parent_id);
-    if (checker.CausingConflict(parent, entry)) {
-      conflict_progress->MergeSets(entry.Get(syncable::ID),
-                                   parent.Get(syncable::ID));
-    } else {
-      break;
-    }
-    parent_id = parent.Get(syncable::PARENT_ID);
-  }
-}
-
-}  // namespace
-
-void BuildAndProcessConflictSetsCommand::MergeSetsForNonEmptyDirectories(
-    syncable::BaseTransaction* trans, syncable::Entry* entry,
-    ConflictProgress* conflict_progress) {
-  if (entry->Get(syncable::IS_UNSYNCED) && !entry->Get(syncable::IS_DEL)) {
-    ServerDeletedPathChecker checker;
-    CrawlDeletedTreeMergingSets(trans, *entry, conflict_progress, checker);
-  }
-  if (entry->Get(syncable::IS_UNAPPLIED_UPDATE) &&
-      !entry->Get(syncable::SERVER_IS_DEL)) {
-    syncable::Entry parent(trans, syncable::GET_BY_ID,
-                           entry->Get(syncable::SERVER_PARENT_ID));
-    syncable::Id parent_id = entry->Get(syncable::SERVER_PARENT_ID);
-    if (!parent.good())
-      return;
-    LocallyDeletedPathChecker checker;
-    if (!checker.CausingConflict(parent, *entry))
-      return;
-    conflict_progress->MergeSets(entry->Get(syncable::ID),
-                                 parent.Get(syncable::ID));
-    CrawlDeletedTreeMergingSets(trans, parent, conflict_progress, checker);
-  }
-}
-
-}  // namespace browser_sync
diff --git a/chrome/browser/sync/engine/build_and_process_conflict_sets_command.h b/chrome/browser/sync/engine/build_and_process_conflict_sets_command.h
deleted file mode 100644
index 99f130d..0000000
--- a/chrome/browser/sync/engine/build_and_process_conflict_sets_command.h
+++ /dev/null
@@ -1,64 +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 CHROME_BROWSER_SYNC_ENGINE_BUILD_AND_PROCESS_CONFLICT_SETS_COMMAND_H_
-#define CHROME_BROWSER_SYNC_ENGINE_BUILD_AND_PROCESS_CONFLICT_SETS_COMMAND_H_
-#pragma once
-
-#include <vector>
-
-#include "base/basictypes.h"
-#include "base/compiler_specific.h"
-#include "chrome/browser/sync/engine/model_changing_syncer_command.h"
-#include "chrome/browser/sync/engine/model_safe_worker.h"
-
-namespace syncable {
-class BaseTransaction;
-class Entry;
-class Id;
-class WriteTransaction;
-}  // namespace syncable
-
-namespace browser_sync {
-
-class ConflictResolver;
-class Cryptographer;
-
-namespace sessions {
-class ConflictProgress;
-class StatusController;
-}
-
-class BuildAndProcessConflictSetsCommand : public ModelChangingSyncerCommand {
- public:
-  BuildAndProcessConflictSetsCommand();
-  virtual ~BuildAndProcessConflictSetsCommand();
-
- protected:
-  // ModelChangingSyncerCommand implementation.
-  virtual std::set<ModelSafeGroup> GetGroupsToChange(
-      const sessions::SyncSession& session) const OVERRIDE;
-  virtual SyncerError ModelChangingExecuteImpl(
-      sessions::SyncSession* session) OVERRIDE;
-
- private:
-  void BuildConflictSets(syncable::BaseTransaction* trans,
-                         sessions::ConflictProgress* conflict_progress);
-
-  void MergeSetsForIntroducedLoops(syncable::BaseTransaction* trans,
-      syncable::Entry* entry,
-      sessions::ConflictProgress* conflict_progress);
-  void MergeSetsForNonEmptyDirectories(syncable::BaseTransaction* trans,
-      syncable::Entry* entry,
-      sessions::ConflictProgress* conflict_progress);
-  void MergeSetsForPositionUpdate(syncable::BaseTransaction* trans,
-      syncable::Entry* entry,
-      sessions::ConflictProgress* conflict_progress);
-
-  DISALLOW_COPY_AND_ASSIGN(BuildAndProcessConflictSetsCommand);
-};
-
-}  // namespace browser_sync
-
-#endif  // CHROME_BROWSER_SYNC_ENGINE_BUILD_AND_PROCESS_CONFLICT_SETS_COMMAND_H_
diff --git a/chrome/browser/sync/engine/build_and_process_conflict_sets_command_unittest.cc b/chrome/browser/sync/engine/build_and_process_conflict_sets_command_unittest.cc
deleted file mode 100644
index 3152484..0000000
--- a/chrome/browser/sync/engine/build_and_process_conflict_sets_command_unittest.cc
+++ /dev/null
@@ -1,52 +0,0 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "base/basictypes.h"
-#include "base/memory/ref_counted.h"
-#include "chrome/browser/sync/engine/build_and_process_conflict_sets_command.h"
-#include "chrome/browser/sync/sessions/session_state.h"
-#include "chrome/browser/sync/sessions/sync_session.h"
-#include "chrome/browser/sync/syncable/model_type.h"
-#include "chrome/browser/sync/syncable/syncable_id.h"
-#include "chrome/browser/sync/test/engine/fake_model_worker.h"
-#include "chrome/browser/sync/test/engine/syncer_command_test.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace browser_sync {
-
-namespace {
-
-class BuildAndProcessConflictSetsCommandTest : public SyncerCommandTest {
- protected:
-  BuildAndProcessConflictSetsCommandTest() {}
-  virtual ~BuildAndProcessConflictSetsCommandTest() {}
-
-  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())[syncable::BOOKMARKS] = GROUP_UI;
-    (*mutable_routing_info())[syncable::PASSWORDS] = GROUP_PASSWORD;
-    SyncerCommandTest::SetUp();
-  }
-
-  BuildAndProcessConflictSetsCommand command_;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(BuildAndProcessConflictSetsCommandTest);
-};
-
-TEST_F(BuildAndProcessConflictSetsCommandTest, GetGroupsToChange) {
-  ExpectNoGroupsToChange(command_);
-  // Put GROUP_UI in conflict.
-  session()->mutable_status_controller()->
-      GetUnrestrictedMutableConflictProgressForTest(GROUP_UI)->
-      AddConflictingItemById(syncable::Id());
-  ExpectGroupToChange(command_, GROUP_UI);
-}
-
-}  // namespace
-
-}  // namespace browser_sync
diff --git a/chrome/browser/sync/engine/conflict_resolver.cc b/chrome/browser/sync/engine/conflict_resolver.cc
index a262ff7..60651e6 100644
--- a/chrome/browser/sync/engine/conflict_resolver.cc
+++ b/chrome/browser/sync/engine/conflict_resolver.cc
@@ -77,21 +77,13 @@ ConflictResolver::ProcessSimpleConflict(WriteTransaction* trans,
   MutableEntry entry(trans, syncable::GET_BY_ID, id);
   // Must be good as the entry won't have been cleaned up.
   CHECK(entry.good());
-  // If an update fails, locally we have to be in a set or unsynced. We're not
-  // in a set here, so we must be unsynced.
-  if (!entry.Get(syncable::IS_UNSYNCED)) {
-    return NO_SYNC_PROGRESS;
-  }
 
-  if (!entry.Get(syncable::IS_UNAPPLIED_UPDATE)) {
-    if (!entry.Get(syncable::PARENT_ID).ServerKnows()) {
-      DVLOG(1) << "Item conflicting because its parent not yet committed. Id: "
-               << id;
-    } else {
-      DVLOG(1) << "No set for conflicting entry id " << id << ". There should "
-               << "be an update/commit that will fix this soon. This message "
-               << "should not repeat.";
-    }
+  // This function can only resolve simple conflicts.  Simple conflicts have
+  // both IS_UNSYNCED and IS_UNAPPLIED_UDPATE set.
+  if (!entry.Get(syncable::IS_UNAPPLIED_UPDATE) ||
+      !entry.Get(syncable::IS_UNSYNCED)) {
+    // This is very unusual, but it can happen in tests.  We may be able to
+    // assert NOTREACHED() here when those tests are updated.
     return NO_SYNC_PROGRESS;
   }
 
@@ -313,7 +305,10 @@ ConflictResolver::ProcessSimpleConflict(WriteTransaction* trans,
     entry.Put(syncable::BASE_SERVER_SPECIFICS, sync_pb::EntitySpecifics());
     return SYNC_PROGRESS;
   } else {  // SERVER_IS_DEL is true
-    // If a server deleted folder has local contents we should be in a set.
+    // If a server deleted folder has local contents it should be a hierarchy
+    // conflict.  Hierarchy conflicts should not be processed by this function.
+    // We could end up here if a change was made since we last tried to detect
+    // conflicts, which was during update application.
     if (entry.Get(syncable::IS_DIR)) {
       Directory::ChildHandles children;
       trans->directory()->GetChildHandlesById(trans,
@@ -321,7 +316,7 @@ ConflictResolver::ProcessSimpleConflict(WriteTransaction* trans,
                                               &children);
       if (0 != children.size()) {
         DVLOG(1) << "Entry is a server deleted directory with local contents, "
-                 << "should be in a set. (race condition).";
+                 << "should be a hierarchy conflict. (race condition).";
         return NO_SYNC_PROGRESS;
       }
     }
@@ -361,79 +356,53 @@ ConflictResolver::ProcessSimpleConflict(WriteTransaction* trans,
   }
 }
 
-bool ConflictResolver::ResolveSimpleConflicts(
-    syncable::WriteTransaction* trans,
-    const Cryptographer* cryptographer,
-    const ConflictProgress& progress,
-    sessions::StatusController* status) {
+bool ConflictResolver::ResolveConflicts(syncable::WriteTransaction* trans,
+                                        const Cryptographer* cryptographer,
+                                        const ConflictProgress& progress,
+                                        sessions::StatusController* status) {
   bool forward_progress = false;
-  // First iterate over simple conflict items (those that belong to no set).
+  // Iterate over simple conflict items.
   set<Id>::const_iterator conflicting_item_it;
   set<Id> processed_items;
-  for (conflicting_item_it = progress.ConflictingItemsBegin();
-       conflicting_item_it != progress.ConflictingItemsEnd();
+  for (conflicting_item_it = progress.SimpleConflictingItemsBegin();
+       conflicting_item_it != progress.SimpleConflictingItemsEnd();
        ++conflicting_item_it) {
     Id id = *conflicting_item_it;
     if (processed_items.count(id) > 0)
       continue;
-    map<Id, ConflictSet*>::const_iterator item_set_it =
-        progress.IdToConflictSetFind(id);
-    if (item_set_it == progress.IdToConflictSetEnd() ||
-        0 == item_set_it->second) {
-      // We have a simple conflict. In order check if positions have changed,
-      // we need to process conflicting predecessors before successors. Traverse
-      // backwards through all continuous conflicting predecessors, building a
-      // stack of items to resolve in predecessor->successor order, then process
-      // each item individually.
-      list<Id> predecessors;
-      Id prev_id = id;
-      do {
-        predecessors.push_back(prev_id);
-        Entry entry(trans, syncable::GET_BY_ID, prev_id);
-        // Any entry in conflict must be valid.
-        CHECK(entry.good());
-        Id new_prev_id = entry.Get(syncable::PREV_ID);
-        if (new_prev_id == prev_id)
+
+    // We have a simple conflict. In order check if positions have changed,
+    // we need to process conflicting predecessors before successors. Traverse
+    // backwards through all continuous conflicting predecessors, building a
+    // stack of items to resolve in predecessor->successor order, then process
+    // each item individually.
+    list<Id> predecessors;
+    Id prev_id = id;
+    do {
+      predecessors.push_back(prev_id);
+      Entry entry(trans, syncable::GET_BY_ID, prev_id);
+      // Any entry in conflict must be valid.
+      CHECK(entry.good());
+      Id new_prev_id = entry.Get(syncable::PREV_ID);
+      if (new_prev_id == prev_id)
+        break;
+      prev_id = new_prev_id;
+    } while (processed_items.count(prev_id) == 0 &&
+             progress.HasSimpleConflictItem(prev_id));  // Excludes root.
+    while (!predecessors.empty()) {
+      id = predecessors.back();
+      predecessors.pop_back();
+      switch (ProcessSimpleConflict(trans, id, cryptographer, status)) {
+        case NO_SYNC_PROGRESS:
+          break;
+        case SYNC_PROGRESS:
+          forward_progress = true;
           break;
-        prev_id = new_prev_id;
-      } while (processed_items.count(prev_id) == 0 &&
-               progress.HasSimpleConflictItem(prev_id));  // Excludes root.
-      while (!predecessors.empty()) {
-        id = predecessors.back();
-        predecessors.pop_back();
-        switch (ProcessSimpleConflict(trans, id, cryptographer, status)) {
-          case NO_SYNC_PROGRESS:
-            break;
-          case SYNC_PROGRESS:
-            forward_progress = true;
-            break;
-        }
-        processed_items.insert(id);
       }
+      processed_items.insert(id);
     }
   }
   return forward_progress;
 }
 
-bool ConflictResolver::ResolveConflicts(syncable::WriteTransaction* trans,
-                                        const Cryptographer* cryptographer,
-                                        const ConflictProgress& progress,
-                                        sessions::StatusController* status) {
-  // TODO(rlarocque): A good amount of code related to the resolution of
-  // conflict sets has been deleted here.  This was done because the code had
-  // not been used in years.  An unrelated bug fix accidentally re-enabled the
-  // code, forcing us to make a decision about what we should do with the code.
-  // We decided to do the safe thing and delete it for now.  This restores the
-  // behaviour we've relied on for quite some time.  We should think about what
-  // that code was trying to do and consider re-enabling parts of it.
-
-  if (progress.ConflictSetsSize() > 0) {
-    DVLOG(1) << "Detected " << progress.IdToConflictSetSize()
-        << " non-simple conflicting entries in " << progress.ConflictSetsSize()
-        << " unprocessed conflict sets.";
-  }
-
-  return ResolveSimpleConflicts(trans, cryptographer, progress, status);
-}
-
 }  // namespace browser_sync
diff --git a/chrome/browser/sync/engine/process_commit_response_command.cc b/chrome/browser/sync/engine/process_commit_response_command.cc
index 536db0e..6135ef6 100644
--- a/chrome/browser/sync/engine/process_commit_response_command.cc
+++ b/chrome/browser/sync/engine/process_commit_response_command.cc
@@ -165,7 +165,7 @@ SyncerError ProcessCommitResponseCommand::ProcessCommitResponse(
         case CommitResponse::CONFLICT:
           ++conflicting_commits;
           // Only server CONFLICT responses will activate conflict resolution.
-          conflict_progress->AddConflictingItemById(
+          conflict_progress->AddServerConflictingItemById(
               status->GetCommitIdAt(proj[i]));
           break;
         case CommitResponse::SUCCESS:
diff --git a/chrome/browser/sync/engine/resolve_conflicts_command_unittest.cc b/chrome/browser/sync/engine/resolve_conflicts_command_unittest.cc
index a36d5e2..7f7ddfd 100644
--- a/chrome/browser/sync/engine/resolve_conflicts_command_unittest.cc
+++ b/chrome/browser/sync/engine/resolve_conflicts_command_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -42,7 +42,7 @@ TEST_F(ResolveConflictsCommandTest, GetGroupsToChange) {
   // Put GROUP_PASSWORD in conflict.
   session()->mutable_status_controller()->
       GetUnrestrictedMutableConflictProgressForTest(GROUP_PASSWORD)->
-      AddConflictingItemById(syncable::Id());
+      AddSimpleConflictingItemById(syncable::Id());
   ExpectGroupToChange(command_, GROUP_PASSWORD);
 }
 
diff --git a/chrome/browser/sync/engine/syncer.cc b/chrome/browser/sync/engine/syncer.cc
index dbab2fb..f64b030 100644
--- a/chrome/browser/sync/engine/syncer.cc
+++ b/chrome/browser/sync/engine/syncer.cc
@@ -10,7 +10,6 @@
 #include "base/message_loop.h"
 #include "base/time.h"
 #include "chrome/browser/sync/engine/apply_updates_command.h"
-#include "chrome/browser/sync/engine/build_and_process_conflict_sets_command.h"
 #include "chrome/browser/sync/engine/build_commit_command.h"
 #include "chrome/browser/sync/engine/cleanup_disabled_types_command.h"
 #include "chrome/browser/sync/engine/clear_data_command.h"
@@ -70,7 +69,6 @@ const char* SyncerStepToString(const SyncerStep step)
     ENUM_CASE(BUILD_COMMIT_REQUEST);
     ENUM_CASE(POST_COMMIT_MESSAGE);
     ENUM_CASE(PROCESS_COMMIT_RESPONSE);
-    ENUM_CASE(BUILD_AND_PROCESS_CONFLICT_SETS);
     ENUM_CASE(RESOLVE_CONFLICTS);
     ENUM_CASE(APPLY_UPDATES_TO_RESOLVE_CONFLICTS);
     ENUM_CASE(CLEAR_PRIVATE_DATA);
@@ -216,7 +214,7 @@ void Syncer::SyncShare(sessions::SyncSession* session,
 
           next_step = POST_COMMIT_MESSAGE;
         } else {
-          next_step = BUILD_AND_PROCESS_CONFLICT_SETS;
+          next_step = RESOLVE_CONFLICTS;
         }
 
         break;
@@ -234,12 +232,6 @@ void Syncer::SyncShare(sessions::SyncSession* session,
         session->mutable_status_controller()->
             set_last_process_commit_response_result(
                 process_response_command.Execute(session));
-        next_step = BUILD_AND_PROCESS_CONFLICT_SETS;
-        break;
-      }
-      case BUILD_AND_PROCESS_CONFLICT_SETS: {
-        BuildAndProcessConflictSetsCommand build_process_conflict_sets;
-        build_process_conflict_sets.Execute(session);
         next_step = RESOLVE_CONFLICTS;
         break;
       }
@@ -249,7 +241,7 @@ void Syncer::SyncShare(sessions::SyncSession* session,
         ResolveConflictsCommand resolve_conflicts_command;
         resolve_conflicts_command.Execute(session);
 
-        // Has ConflictingUpdates includes both blocking and non-blocking
+        // Has ConflictingUpdates includes both resolvable and unresolvable
         // conflicts. If we have either, we want to attempt to reapply.
         if (status->HasConflictingUpdates())
           next_step = APPLY_UPDATES_TO_RESOLVE_CONFLICTS;
@@ -263,13 +255,12 @@ void Syncer::SyncShare(sessions::SyncSession* session,
         ApplyUpdatesCommand apply_updates;
 
         // We only care to resolve conflicts again if we made progress on the
-        // blocking conflicts. Whether or not we made progress on the
-        // non-blocking doesn't matter.
+        // simple conflicts.
         int before_blocking_conflicting_updates =
-            status->TotalNumBlockingConflictingItems();
+            status->TotalNumSimpleConflictingItems();
         apply_updates.Execute(session);
         int after_blocking_conflicting_updates =
-            status->TotalNumBlockingConflictingItems();
+            status->TotalNumSimpleConflictingItems();
         // If the following call sets the conflicts_resolved value to true,
         // SyncSession::HasMoreToSync() will send us into another sync cycle
         // after this one completes.
diff --git a/chrome/browser/sync/engine/syncer.h b/chrome/browser/sync/engine/syncer.h
index 9a17025..f9f3dbe 100644
--- a/chrome/browser/sync/engine/syncer.h
+++ b/chrome/browser/sync/engine/syncer.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -40,7 +40,6 @@ enum SyncerStep {
   BUILD_COMMIT_REQUEST,
   POST_COMMIT_MESSAGE,
   PROCESS_COMMIT_RESPONSE,
-  BUILD_AND_PROCESS_CONFLICT_SETS,
   RESOLVE_CONFLICTS,
   APPLY_UPDATES_TO_RESOLVE_CONFLICTS,
   CLEAR_PRIVATE_DATA,  // TODO(tim): Rename 'private' to 'user'.
diff --git a/chrome/browser/sync/engine/syncer_types.h b/chrome/browser/sync/engine/syncer_types.h
index e4fde71..4948884 100644
--- a/chrome/browser/sync/engine/syncer_types.h
+++ b/chrome/browser/sync/engine/syncer_types.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -30,15 +30,36 @@ enum UpdateAttemptResponse {
   // Update was applied or safely ignored.
   SUCCESS,
 
-  // Conflicts with the local data representation. This can also mean that the
-  // entry doesn't currently make sense if we applied it.
-  CONFLICT,
+  // The conditions described by the following enum values are not mutually
+  // exclusive.  The list has been ordered according to priority in the case of
+  // overlap, with highest priority first.
+  //
+  // For example, in the case where an item had both the IS_UNSYCNED and
+  // IS_UNAPPLIED_UPDATE flags set (CONFLICT_SIMPLE), and a SERVER_PARENT_ID
+  // that, if applied, would cause a directory loop (CONFLICT_HIERARCHY), and
+  // specifics that we can't decrypt right now (CONFLICT_ENCRYPTION), the
+  // UpdateApplicator would return CONFLICT_ENCRYPTION when attempting to
+  // process the item.
+  //
+  // We do not attempt to resolve CONFLICT_HIERARCHY or CONFLICT_ENCRYPTION
+  // items.  We will leave these updates unapplied and wait for the server
+  // to send us newer updates that will resolve the conflict.
 
   // We were unable to decrypt/encrypt this server data. As such, we can't make
   // forward progress on this node, but because the passphrase may not arrive
   // until later we don't want to get the syncer stuck. See UpdateApplicator
   // for how this is handled.
-  CONFLICT_ENCRYPTION
+  CONFLICT_ENCRYPTION,
+
+  // These are some updates that, if applied, would violate the tree's
+  // invariants.  Examples of this include the server adding children to locally
+  // deleted directories and directory moves that would create loops.
+  CONFLICT_HIERARCHY,
+
+  // This indicates that item was modified both remotely (IS_UNAPPLIED_UPDATE)
+  // and locally (IS_UNSYNCED).  We use the ConflictResolver to decide which of
+  // the changes should take priority, or if we can possibly merge the data.
+  CONFLICT_SIMPLE
 };
 
 enum ServerUpdateProcessingResult {
@@ -132,14 +153,6 @@ class SyncEngineEventListener {
   virtual ~SyncEngineEventListener() {}
 };
 
-// This struct is passed between parts of the syncer during the processing of
-// one sync loop. It lives on the stack. We don't expose the number of
-// conflicts during SyncShare as the conflicts may be solved automatically
-// by the conflict resolver.
-typedef std::vector<syncable::Id> ConflictSet;
-
-typedef std::map<syncable::Id, ConflictSet*> IdToConflictSetMap;
-
 }  // namespace browser_sync
 
 #endif  // CHROME_BROWSER_SYNC_ENGINE_SYNCER_TYPES_H_
diff --git a/chrome/browser/sync/engine/syncer_unittest.cc b/chrome/browser/sync/engine/syncer_unittest.cc
index 098c1b4..8933ba8 100644
--- a/chrome/browser/sync/engine/syncer_unittest.cc
+++ b/chrome/browser/sync/engine/syncer_unittest.cc
@@ -1728,7 +1728,7 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) {
   {
     sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE);
     ASSERT_TRUE(status->conflict_progress());
-    EXPECT_EQ(1, status->conflict_progress()->ConflictingItemsSize());
+    EXPECT_EQ(1, status->conflict_progress()->HierarchyConflictingItemsSize());
   }
 
   // These entries will be used in the second set of updates.
@@ -1746,7 +1746,7 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) {
   {
     sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE);
     ASSERT_TRUE(status->conflict_progress());
-    EXPECT_EQ(3, status->conflict_progress()->ConflictingItemsSize());
+    EXPECT_EQ(3, status->conflict_progress()->HierarchyConflictingItemsSize());
   }
 
   {
@@ -1840,7 +1840,7 @@ TEST_F(SyncerTest, IllegalAndLegalUpdates) {
   {
     sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE);
     ASSERT_TRUE(status->conflict_progress());
-    EXPECT_EQ(4, status->conflict_progress()->ConflictingItemsSize());
+    EXPECT_EQ(4, status->conflict_progress()->HierarchyConflictingItemsSize());
   }
 }
 
@@ -2526,7 +2526,7 @@ TEST_F(SyncerTest, UnappliedUpdateDuringCommit) {
   }
   syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END);
   syncer_->SyncShare(session_.get(), SYNCER_BEGIN, SYNCER_END);
-  EXPECT_EQ(0, session_->status_controller().TotalNumConflictingItems());
+  EXPECT_EQ(1, session_->status_controller().TotalNumConflictingItems());
   saw_syncer_event_ = false;
 }
 
@@ -2939,31 +2939,6 @@ TEST_F(SyncerTest, SiblingDirectoriesBecomeCircular) {
   }
 }
 
-TEST_F(SyncerTest, ConflictSetClassificationError) {
-  // This code used to cause a CHECK failure because we incorrectly thought
-  // a set was only unapplied updates.
-  ScopedDirLookup dir(syncdb_.manager(), syncdb_.name());
-  CHECK(dir.good());
-  mock_server_->AddUpdateDirectory(1, 0, "A", 10, 10);
-  mock_server_->AddUpdateDirectory(2, 0, "B", 10, 10);
-  mock_server_->set_conflict_all_commits(true);
-  SyncShareAsDelegate();
-  {
-    WriteTransaction wtrans(FROM_HERE, UNITTEST, dir);
-    MutableEntry A(&wtrans, GET_BY_ID, ids_.FromNumber(1));
-    ASSERT_TRUE(A.good());
-    A.Put(IS_UNSYNCED, true);
-    A.Put(IS_UNAPPLIED_UPDATE, true);
-    A.Put(SERVER_NON_UNIQUE_NAME, "B");
-    MutableEntry B(&wtrans, GET_BY_ID, ids_.FromNumber(2));
-    ASSERT_TRUE(B.good());
-    B.Put(IS_UNAPPLIED_UPDATE, true);
-    B.Put(SERVER_NON_UNIQUE_NAME, "A");
-  }
-  SyncShareAsDelegate();
-  saw_syncer_event_ = false;
-}
-
 TEST_F(SyncerTest, SwapEntryNames) {
   // Simple transaction test.
   ScopedDirLookup dir(syncdb_.manager(), syncdb_.name());
@@ -3168,39 +3143,6 @@ TEST_F(SyncerTest, UpdateFlipsTheFolderBit) {
   }
 }
 
-TEST(SyncerSyncProcessState, MergeSetsTest) {
-  TestIdFactory id_factory;
-  syncable::Id id[7];
-  for (int i = 1; i < 7; i++) {
-    id[i] = id_factory.NewServerId();
-  }
-  bool is_dirty = false;
-  ConflictProgress c(&is_dirty);
-  c.MergeSets(id[1], id[2]);
-  c.MergeSets(id[2], id[3]);
-  c.MergeSets(id[4], id[5]);
-  c.MergeSets(id[5], id[6]);
-  EXPECT_EQ(6u, c.IdToConflictSetSize());
-  EXPECT_FALSE(is_dirty);
-  for (int i = 1; i < 7; i++) {
-    EXPECT_TRUE(NULL != c.IdToConflictSetGet(id[i]));
-    EXPECT_TRUE(c.IdToConflictSetGet(id[(i & ~3) + 1]) ==
-                c.IdToConflictSetGet(id[i]));
-  }
-  c.MergeSets(id[1], id[6]);
-  for (int i = 1; i < 7; i++) {
-    EXPECT_TRUE(NULL != c.IdToConflictSetGet(id[i]));
-    EXPECT_TRUE(c.IdToConflictSetGet(id[1]) == c.IdToConflictSetGet(id[i]));
-  }
-
-  // Check dupes don't cause double sets.
-  ConflictProgress identical_set(&is_dirty);
-  identical_set.MergeSets(id[1], id[1]);
-  EXPECT_TRUE(identical_set.IdToConflictSetSize() == 1);
-  EXPECT_TRUE(identical_set.IdToConflictSetGet(id[1])->size() == 1);
-  EXPECT_FALSE(is_dirty);
-}
-
 // Bug Synopsis:
 // Merge conflict resolution will merge a new local entry with another entry
 // that needs updates, resulting in CHECK.
@@ -3453,31 +3395,6 @@ TEST_F(SyncerTest, DirectoryCommitTest) {
   }
 }
 
-TEST_F(SyncerTest, ConflictSetSizeReducedToOne) {
-  ScopedDirLookup dir(syncdb_.manager(), syncdb_.name());
-  CHECK(dir.good());
-
-  syncable::Id in_root_id = ids_.NewServerId();
-
-  mock_server_->AddUpdateBookmark(in_root_id, TestIdFactory::root(),
-      "in_root", 1, 1);
-  SyncShareAsDelegate();
-  {
-    WriteTransaction trans(FROM_HERE, UNITTEST, dir);
-    MutableEntry oentry(&trans, GET_BY_ID, in_root_id);
-    ASSERT_TRUE(oentry.good());
-    oentry.Put(NON_UNIQUE_NAME, "old_in_root");
-    WriteTestDataToEntry(&trans, &oentry);
-    MutableEntry entry(&trans, CREATE, trans.root_id(), "in_root");
-    ASSERT_TRUE(entry.good());
-    WriteTestDataToEntry(&trans, &entry);
-  }
-  mock_server_->set_conflict_all_commits(true);
-  // This SyncShare call used to result in a CHECK failure.
-  SyncShareAsDelegate();
-  saw_syncer_event_ = false;
-}
-
 TEST_F(SyncerTest, TestClientCommand) {
   ScopedDirLookup dir(syncdb_.manager(), syncdb_.name());
   CHECK(dir.good());
diff --git a/chrome/browser/sync/engine/syncer_util.cc b/chrome/browser/sync/engine/syncer_util.cc
index bf1456e..b3557f4 100644
--- a/chrome/browser/sync/engine/syncer_util.cc
+++ b/chrome/browser/sync/engine/syncer_util.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -311,7 +311,7 @@ UpdateAttemptResponse SyncerUtil::AttemptToUpdateEntry(
   // passphrase may not arrive within this GetUpdates, we can't just return
   // conflict, else we try to perform normal conflict resolution prematurely or
   // the syncer may get stuck. As such, we return CONFLICT_ENCRYPTION, which is
-  // treated as a non-blocking conflict. See the description in syncer_types.h.
+  // treated as an unresolvable conflict. See the description in syncer_types.h.
   // This prevents any unsynced changes from commiting and postpones conflict
   // resolution until all data can be decrypted.
   if (specifics.has_encrypted() &&
@@ -333,11 +333,6 @@ UpdateAttemptResponse SyncerUtil::AttemptToUpdateEntry(
     }
   }
 
-  if (entry->Get(IS_UNSYNCED)) {
-    DVLOG(1) << "Skipping update, returning conflict for: " << id
-             << " ; it's unsynced.";
-    return CONFLICT;
-  }
   if (!entry->Get(SERVER_IS_DEL)) {
     syncable::Id new_parent = entry->Get(SERVER_PARENT_ID);
     Entry parent(trans, GET_BY_ID,  new_parent);
@@ -348,13 +343,13 @@ UpdateAttemptResponse SyncerUtil::AttemptToUpdateEntry(
     // different ways we deal with it once here to reduce the amount of code and
     // potential errors.
     if (!parent.good() || parent.Get(IS_DEL) || !parent.Get(IS_DIR)) {
-      return CONFLICT;
+      return CONFLICT_HIERARCHY;
     }
     if (entry->Get(PARENT_ID) != new_parent) {
       if (!entry->Get(IS_DEL) && !IsLegalNewParent(trans, id, new_parent)) {
         DVLOG(1) << "Not updating item " << id
                  << ", illegal new parent (would cause loop).";
-        return CONFLICT;
+        return CONFLICT_HIERARCHY;
       }
     }
   } else if (entry->Get(IS_DIR)) {
@@ -364,10 +359,16 @@ UpdateAttemptResponse SyncerUtil::AttemptToUpdateEntry(
       // If we have still-existing children, then we need to deal with
       // them before we can process this change.
       DVLOG(1) << "Not deleting directory; it's not empty " << *entry;
-      return CONFLICT;
+      return CONFLICT_HIERARCHY;
     }
   }
 
+  if (entry->Get(IS_UNSYNCED)) {
+    DVLOG(1) << "Skipping update, returning conflict for: " << id
+             << " ; it's unsynced.";
+    return CONFLICT_SIMPLE;
+  }
+
   if (specifics.has_encrypted()) {
     DVLOG(2) << "Received a decryptable "
              << syncable::ModelTypeToString(entry->GetServerModelType())
diff --git a/chrome/browser/sync/engine/update_applicator.cc b/chrome/browser/sync/engine/update_applicator.cc
index e44083c..0c8122a 100644
--- a/chrome/browser/sync/engine/update_applicator.cc
+++ b/chrome/browser/sync/engine/update_applicator.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -71,14 +71,18 @@ bool UpdateApplicator::AttemptOneApplication(
       progress_ = true;
       application_results_.AddSuccess(entry.Get(syncable::ID));
       break;
-    case CONFLICT:
+    case CONFLICT_SIMPLE:
       pointer_++;
-      application_results_.AddConflict(entry.Get(syncable::ID));
+      application_results_.AddSimpleConflict(entry.Get(syncable::ID));
       break;
     case CONFLICT_ENCRYPTION:
       pointer_++;
       application_results_.AddEncryptionConflict(entry.Get(syncable::ID));
       break;
+    case CONFLICT_HIERARCHY:
+      pointer_++;
+      application_results_.AddHierarchyConflict(entry.Get(syncable::ID));
+      break;
     default:
       NOTREACHED();
       break;
@@ -133,7 +137,7 @@ UpdateApplicator::ResultTracker::ResultTracker(size_t num_results) {
 UpdateApplicator::ResultTracker::~ResultTracker() {
 }
 
-void UpdateApplicator::ResultTracker::AddConflict(syncable::Id id) {
+void UpdateApplicator::ResultTracker::AddSimpleConflict(syncable::Id id) {
   conflicting_ids_.push_back(id);
 }
 
@@ -141,6 +145,10 @@ void UpdateApplicator::ResultTracker::AddEncryptionConflict(syncable::Id id) {
   encryption_conflict_ids_.push_back(id);
 }
 
+void UpdateApplicator::ResultTracker::AddHierarchyConflict(syncable::Id id) {
+  hierarchy_conflict_ids_.push_back(id);
+}
+
 void UpdateApplicator::ResultTracker::AddSuccess(syncable::Id id) {
   successful_ids_.push_back(id);
 }
@@ -150,19 +158,21 @@ void UpdateApplicator::ResultTracker::SaveProgress(
     sessions::UpdateProgress* update_progress) {
   vector<syncable::Id>::const_iterator i;
   for (i = conflicting_ids_.begin(); i != conflicting_ids_.end(); ++i) {
-    conflict_progress->AddConflictingItemById(*i);
-    update_progress->AddAppliedUpdate(CONFLICT, *i);
+    conflict_progress->AddSimpleConflictingItemById(*i);
+    update_progress->AddAppliedUpdate(CONFLICT_SIMPLE, *i);
   }
   for (i = encryption_conflict_ids_.begin();
        i != encryption_conflict_ids_.end(); ++i) {
-    // Encryption conflicts should not put the syncer into a stuck state. We
-    // mark as conflict, so that we reattempt to apply updates, but add it to
-    // the list of nonblocking conflicts instead of normal conflicts.
-    conflict_progress->AddNonblockingConflictingItemById(*i);
-    update_progress->AddAppliedUpdate(CONFLICT, *i);
+    conflict_progress->AddEncryptionConflictingItemById(*i);
+    update_progress->AddAppliedUpdate(CONFLICT_ENCRYPTION, *i);
+  }
+  for (i = hierarchy_conflict_ids_.begin();
+       i != hierarchy_conflict_ids_.end(); ++i) {
+    conflict_progress->AddHierarchyConflictingItemById(*i);
+    update_progress->AddAppliedUpdate(CONFLICT_HIERARCHY, *i);
   }
   for (i = successful_ids_.begin(); i != successful_ids_.end(); ++i) {
-    conflict_progress->EraseConflictingItemById(*i);
+    conflict_progress->EraseSimpleConflictingItemById(*i);
     update_progress->AddAppliedUpdate(SUCCESS, *i);
   }
 }
@@ -170,6 +180,7 @@ void UpdateApplicator::ResultTracker::SaveProgress(
 void UpdateApplicator::ResultTracker::ClearConflicts() {
   conflicting_ids_.clear();
   encryption_conflict_ids_.clear();
+  hierarchy_conflict_ids_.clear();
 }
 
 bool UpdateApplicator::ResultTracker::no_conflicts() const {
diff --git a/chrome/browser/sync/engine/update_applicator.h b/chrome/browser/sync/engine/update_applicator.h
index 1196fcc..bcc928d 100644
--- a/chrome/browser/sync/engine/update_applicator.h
+++ b/chrome/browser/sync/engine/update_applicator.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 //
@@ -57,14 +57,13 @@ class UpdateApplicator {
 
  private:
   // Track the status of all applications.
-  // We treat encryption conflicts as nonblocking conflict items when we save
-  // progress.
   class ResultTracker {
    public:
      explicit ResultTracker(size_t num_results);
      virtual ~ResultTracker();
-     void AddConflict(syncable::Id);
+     void AddSimpleConflict(syncable::Id);
      void AddEncryptionConflict(syncable::Id);
+     void AddHierarchyConflict(syncable::Id);
      void AddSuccess(syncable::Id);
      void SaveProgress(sessions::ConflictProgress* conflict_progress,
                        sessions::UpdateProgress* update_progress);
@@ -77,6 +76,7 @@ class UpdateApplicator {
     std::vector<syncable::Id> conflicting_ids_;
     std::vector<syncable::Id> successful_ids_;
     std::vector<syncable::Id> encryption_conflict_ids_;
+    std::vector<syncable::Id> hierarchy_conflict_ids_;
   };
 
   // If true, AttemptOneApplication will skip over |entry| and return true.
diff --git a/chrome/browser/sync/internal_api/debug_info_event_listener.cc b/chrome/browser/sync/internal_api/debug_info_event_listener.cc
index d783dee..8c49eb5 100644
--- a/chrome/browser/sync/internal_api/debug_info_event_listener.cc
+++ b/chrome/browser/sync/internal_api/debug_info_event_listener.cc
@@ -23,10 +23,11 @@ void DebugInfoEventListener::OnSyncCycleCompleted(
   sync_pb::SyncCycleCompletedEventInfo* sync_completed_event_info =
       event_info.mutable_sync_cycle_completed_event_info();
 
+  // TODO(rlarocque): These counters are reversed.  We should fix this bug.
   sync_completed_event_info->set_num_blocking_conflicts(
       snapshot->num_conflicting_updates);
   sync_completed_event_info->set_num_non_blocking_conflicts(
-      snapshot->num_blocking_conflicting_updates);
+      snapshot->num_simple_conflicting_updates);
 
   AddEventToQueue(event_info);
 }
diff --git a/chrome/browser/sync/profile_sync_service_harness.cc b/chrome/browser/sync/profile_sync_service_harness.cc
index 16303c6..48c2159 100644
--- a/chrome/browser/sync/profile_sync_service_harness.cc
+++ b/chrome/browser/sync/profile_sync_service_harness.cc
@@ -776,7 +776,7 @@ ProfileSyncService::Status ProfileSyncServiceHarness::GetStatus() {
 bool ProfileSyncServiceHarness::IsDataSyncedImpl(
     const browser_sync::sessions::SyncSessionSnapshot *snap) {
   return snap &&
-      snap->num_blocking_conflicting_updates == 0 &&
+      snap->num_simple_conflicting_updates == 0 &&
       ServiceIsPushingChanges() &&
       GetStatus().notifications_enabled &&
       !service()->HasUnsyncedItems() &&
@@ -1017,8 +1017,8 @@ std::string ProfileSyncServiceHarness::GetClientInfoString(
          << snap->unsynced_count
          << ", num_conflicting_updates: "
          << snap->num_conflicting_updates
-         << ", num_blocking_conflicting_updates: "
-         << snap->num_blocking_conflicting_updates
+         << ", num_simple_conflicting_updates: "
+         << snap->num_simple_conflicting_updates
          << ", num_updates_downloaded : "
          << snap->syncer_status.num_updates_downloaded_total
          << ", passphrase_required_reason: "
diff --git a/chrome/browser/sync/protocol/client_debug_info.proto b/chrome/browser/sync/protocol/client_debug_info.proto
index 6d67b72..d58f61c 100644
--- a/chrome/browser/sync/protocol/client_debug_info.proto
+++ b/chrome/browser/sync/protocol/client_debug_info.proto
@@ -15,6 +15,8 @@ package sync_pb;
 // SYNC_CYCLE_COMPLETED  is sent.
 message SyncCycleCompletedEventInfo {
   // optional bool syncer_stuck = 1; // Was always false, now obsolete.
+
+  // TODO(rlarocque): rename these counters.
   optional int32 num_blocking_conflicts = 2;
   optional int32 num_non_blocking_conflicts = 3;
 }
diff --git a/chrome/browser/sync/sessions/session_state.cc b/chrome/browser/sync/sessions/session_state.cc
index 18f4e59..eff0a22 100644
--- a/chrome/browser/sync/sessions/session_state.cc
+++ b/chrome/browser/sync/sessions/session_state.cc
@@ -114,7 +114,7 @@ SyncSessionSnapshot::SyncSessionSnapshot(
     bool more_to_sync,
     bool is_silenced,
     int64 unsynced_count,
-    int num_blocking_conflicting_updates,
+    int num_simple_conflicting_updates,
     int num_conflicting_updates,
     bool did_commit_items,
     const SyncSourceInfo& source,
@@ -130,7 +130,7 @@ SyncSessionSnapshot::SyncSessionSnapshot(
       has_more_to_sync(more_to_sync),
       is_silenced(is_silenced),
       unsynced_count(unsynced_count),
-      num_blocking_conflicting_updates(num_blocking_conflicting_updates),
+      num_simple_conflicting_updates(num_simple_conflicting_updates),
       num_conflicting_updates(num_conflicting_updates),
       did_commit_items(did_commit_items),
       source(source),
@@ -163,8 +163,8 @@ DictionaryValue* SyncSessionSnapshot::ToValue() const {
   // We don't care too much if we lose precision here, also.
   value->SetInteger("unsyncedCount",
                     static_cast<int>(unsynced_count));
-  value->SetInteger("numBlockingConflictingUpdates",
-                    num_blocking_conflicting_updates);
+  value->SetInteger("numSimpleConflictingUpdates",
+                    num_simple_conflicting_updates);
   value->SetInteger("numConflictingUpdates", num_conflicting_updates);
   value->SetBoolean("didCommitItems", did_commit_items);
   value->SetInteger("numEntries", num_entries);
@@ -179,144 +179,70 @@ std::string SyncSessionSnapshot::ToString() const {
   return json;
 }
 
-ConflictProgress::ConflictProgress(bool* dirty_flag) : dirty_(dirty_flag) {}
+ConflictProgress::ConflictProgress(bool* dirty_flag)
+  : num_server_conflicting_items(0), num_hierarchy_conflicting_items(0),
+    num_encryption_conflicting_items(0), dirty_(dirty_flag) {
+}
 
 ConflictProgress::~ConflictProgress() {
-  CleanupSets();
 }
 
 bool ConflictProgress::HasSimpleConflictItem(const syncable::Id& id) const {
-  return conflicting_item_ids_.count(id) > 0 &&
-         (IdToConflictSetFind(id) == IdToConflictSetEnd());
-}
-
-IdToConflictSetMap::const_iterator ConflictProgress::IdToConflictSetFind(
-    const syncable::Id& the_id) const {
-  return id_to_conflict_set_.find(the_id);
-}
-
-IdToConflictSetMap::const_iterator
-ConflictProgress::IdToConflictSetBegin() const {
-  return id_to_conflict_set_.begin();
-}
-
-IdToConflictSetMap::const_iterator
-ConflictProgress::IdToConflictSetEnd() const {
-  return id_to_conflict_set_.end();
-}
-
-IdToConflictSetMap::size_type ConflictProgress::IdToConflictSetSize() const {
-  return id_to_conflict_set_.size();
-}
-
-const ConflictSet* ConflictProgress::IdToConflictSetGet(
-    const syncable::Id& the_id) {
-  return id_to_conflict_set_[the_id];
-}
-
-std::set<ConflictSet*>::const_iterator
-ConflictProgress::ConflictSetsBegin() const {
-  return conflict_sets_.begin();
-}
-
-std::set<ConflictSet*>::const_iterator
-ConflictProgress::ConflictSetsEnd() const {
-  return conflict_sets_.end();
-}
-
-std::set<ConflictSet*>::size_type
-ConflictProgress::ConflictSetsSize() const {
-  return conflict_sets_.size();
+  return simple_conflicting_item_ids_.count(id) > 0;
 }
 
 std::set<syncable::Id>::const_iterator
-ConflictProgress::ConflictingItemsBegin() const {
-  return conflicting_item_ids_.begin();
+ConflictProgress::SimpleConflictingItemsBegin() const {
+  return simple_conflicting_item_ids_.begin();
 }
 std::set<syncable::Id>::const_iterator
-ConflictProgress::ConflictingItemsEnd() const {
-  return conflicting_item_ids_.end();
+ConflictProgress::SimpleConflictingItemsEnd() const {
+  return simple_conflicting_item_ids_.end();
 }
 
-void ConflictProgress::AddConflictingItemById(const syncable::Id& the_id) {
+void ConflictProgress::AddSimpleConflictingItemById(
+    const syncable::Id& the_id) {
   std::pair<std::set<syncable::Id>::iterator, bool> ret =
-    conflicting_item_ids_.insert(the_id);
+      simple_conflicting_item_ids_.insert(the_id);
   if (ret.second)
     *dirty_ = true;
 }
 
-void ConflictProgress::EraseConflictingItemById(const syncable::Id& the_id) {
-  int items_erased = conflicting_item_ids_.erase(the_id);
+void ConflictProgress::EraseSimpleConflictingItemById(
+    const syncable::Id& the_id) {
+  int items_erased = simple_conflicting_item_ids_.erase(the_id);
   if (items_erased != 0)
     *dirty_ = true;
 }
 
-void ConflictProgress::AddNonblockingConflictingItemById(
+void ConflictProgress::AddEncryptionConflictingItemById(
     const syncable::Id& the_id) {
   std::pair<std::set<syncable::Id>::iterator, bool> ret =
-      nonblocking_conflicting_item_ids_.insert(the_id);
-  if (ret.second)
+      unresolvable_conflicting_item_ids_.insert(the_id);
+  if (ret.second) {
+    num_encryption_conflicting_items++;
     *dirty_ = true;
+  }
 }
 
-void ConflictProgress::EraseNonblockingConflictingItemById(
+void ConflictProgress::AddHierarchyConflictingItemById(
     const syncable::Id& the_id) {
-  int items_erased = nonblocking_conflicting_item_ids_.erase(the_id);
-  if (items_erased != 0)
+  std::pair<std::set<syncable::Id>::iterator, bool> ret =
+      unresolvable_conflicting_item_ids_.insert(the_id);
+  if (ret.second) {
+    num_hierarchy_conflicting_items++;
     *dirty_ = true;
-}
-
-void ConflictProgress::MergeSets(const syncable::Id& id1,
-                                 const syncable::Id& id2) {
-  // There are no single item sets, we just leave those entries == 0
-  vector<syncable::Id>* set1 = id_to_conflict_set_[id1];
-  vector<syncable::Id>* set2 = id_to_conflict_set_[id2];
-  vector<syncable::Id>* rv = 0;
-  if (0 == set1 && 0 == set2) {
-    // Neither item currently has a set so we build one.
-    rv = new vector<syncable::Id>();
-    rv->push_back(id1);
-    if (id1 != id2) {
-      rv->push_back(id2);
-    } else {
-      LOG(WARNING) << "[BUG] Attempting to merge two identical conflict ids.";
-    }
-    conflict_sets_.insert(rv);
-  } else if (0 == set1) {
-    // Add the item to the existing set.
-    rv = set2;
-    rv->push_back(id1);
-  } else if (0 == set2) {
-    // Add the item to the existing set.
-    rv = set1;
-    rv->push_back(id2);
-  } else if (set1 == set2) {
-    // It's the same set already.
-    return;
-  } else {
-    // Merge the two sets.
-    rv = set1;
-    // Point all the second sets id's back to the first.
-    vector<syncable::Id>::iterator i;
-    for (i = set2->begin() ; i != set2->end() ; ++i) {
-      id_to_conflict_set_[*i] = rv;
-    }
-    // Copy the second set to the first.
-    rv->insert(rv->end(), set2->begin(), set2->end());
-    conflict_sets_.erase(set2);
-    delete set2;
   }
-  id_to_conflict_set_[id1] = id_to_conflict_set_[id2] = rv;
 }
 
-void ConflictProgress::CleanupSets() {
-  // Clean up all the sets.
-  set<ConflictSet*>::iterator i;
-  for (i = conflict_sets_.begin(); i != conflict_sets_.end(); i++) {
-    delete *i;
+void ConflictProgress::AddServerConflictingItemById(
+    const syncable::Id& the_id) {
+  std::pair<std::set<syncable::Id>::iterator, bool> ret =
+      unresolvable_conflicting_item_ids_.insert(the_id);
+  if (ret.second) {
+    num_server_conflicting_items++;
+    *dirty_ = true;
   }
-  conflict_sets_.clear();
-  id_to_conflict_set_.clear();
 }
 
 UpdateProgress::UpdateProgress() {}
@@ -369,7 +295,7 @@ int UpdateProgress::SuccessfullyAppliedUpdateCount() const {
 bool UpdateProgress::HasConflictingUpdates() const {
   std::vector<AppliedUpdate>::const_iterator it;
   for (it = applied_updates_.begin(); it != applied_updates_.end(); ++it) {
-    if (it->first == CONFLICT) {
+    if (it->first != SUCCESS) {
       return true;
     }
   }
diff --git a/chrome/browser/sync/sessions/session_state.h b/chrome/browser/sync/sessions/session_state.h
index 7b96ec8..f98a94f 100644
--- a/chrome/browser/sync/sessions/session_state.h
+++ b/chrome/browser/sync/sessions/session_state.h
@@ -120,7 +120,7 @@ struct SyncSessionSnapshot {
       bool more_to_sync,
       bool is_silenced,
       int64 unsynced_count,
-      int num_blocking_conflicting_updates,
+      int num_simple_conflicting_updates,
       int num_conflicting_updates,
       bool did_commit_items,
       const SyncSourceInfo& source,
@@ -143,7 +143,7 @@ struct SyncSessionSnapshot {
   const bool has_more_to_sync;
   const bool is_silenced;
   const int64 unsynced_count;
-  const int num_blocking_conflicting_updates;
+  const int num_simple_conflicting_updates;
   const int num_conflicting_updates;
   const bool did_commit_items;
   const SyncSourceInfo source;
@@ -152,49 +152,52 @@ struct SyncSessionSnapshot {
   const bool retry_scheduled;
 };
 
-// Tracks progress of conflicts and their resolution using conflict sets.
+// Tracks progress of conflicts and their resolutions.
 class ConflictProgress {
  public:
   explicit ConflictProgress(bool* dirty_flag);
   ~ConflictProgress();
-  // Various iterators, size, and retrieval functions for conflict sets.
-  IdToConflictSetMap::const_iterator IdToConflictSetBegin() const;
-  IdToConflictSetMap::const_iterator IdToConflictSetEnd() const;
-  IdToConflictSetMap::size_type IdToConflictSetSize() const;
-  IdToConflictSetMap::const_iterator IdToConflictSetFind(
-      const syncable::Id& the_id) const;
-  const ConflictSet* IdToConflictSetGet(const syncable::Id& the_id);
-  std::set<ConflictSet*>::const_iterator ConflictSetsBegin() const;
-  std::set<ConflictSet*>::const_iterator ConflictSetsEnd() const;
-  std::set<ConflictSet*>::size_type ConflictSetsSize() const;
-  bool HasSimpleConflictItem(const syncable::Id& id) const;
+
+  bool HasSimpleConflictItem(const syncable::Id &id) const;
 
   // Various mutators for tracking commit conflicts.
-  void AddConflictingItemById(const syncable::Id& the_id);
-  void EraseConflictingItemById(const syncable::Id& the_id);
-  int ConflictingItemsSize() const { return conflicting_item_ids_.size(); }
-  std::set<syncable::Id>::const_iterator ConflictingItemsBegin() const;
-  std::set<syncable::Id>::const_iterator ConflictingItemsEnd() const;
-
-  // Mutators for nonblocking conflicting items (see description below).
-  void AddNonblockingConflictingItemById(const syncable::Id& the_id);
-  void EraseNonblockingConflictingItemById(const syncable::Id& the_id);
-  int NonblockingConflictingItemsSize() const {
-    return nonblocking_conflicting_item_ids_.size();
+  void AddSimpleConflictingItemById(const syncable::Id& the_id);
+  void EraseSimpleConflictingItemById(const syncable::Id& the_id);
+  std::set<syncable::Id>::const_iterator SimpleConflictingItemsBegin() const;
+  std::set<syncable::Id>::const_iterator SimpleConflictingItemsEnd() const;
+  int SimpleConflictingItemsSize() const {
+    return simple_conflicting_item_ids_.size();
+  }
+
+  // Mutators for unresolvable conflicting items (see description below).
+  void AddEncryptionConflictingItemById(const syncable::Id& the_id);
+  int EncryptionConflictingItemsSize() const {
+    return num_encryption_conflicting_items;
   }
 
-  void MergeSets(const syncable::Id& set1, const syncable::Id& set2);
-  void CleanupSets();
+  void AddHierarchyConflictingItemById(const syncable::Id& id);
+  int HierarchyConflictingItemsSize() const {
+    return num_hierarchy_conflicting_items;
+  }
+
+  void AddServerConflictingItemById(const syncable::Id& id);
+  int ServerConflictingItemsSize() const {
+    return num_server_conflicting_items;
+  }
 
  private:
-  // TODO(sync): move away from sets if it makes more sense.
-  std::set<syncable::Id> conflicting_item_ids_;
-  std::map<syncable::Id, ConflictSet*> id_to_conflict_set_;
-  std::set<ConflictSet*> conflict_sets_;
-
-  // Nonblocking conflicts are not processed by the conflict resolver, but
-  // they will be processed in the APPLY_UDPATES_TO_RESOLVE_CONFLICTS step.
-  std::set<syncable::Id> nonblocking_conflicting_item_ids_;
+  // Conflicts that occur when local and server changes collide and can be
+  // resolved locally.
+  std::set<syncable::Id> simple_conflicting_item_ids_;
+
+  // Unresolvable conflicts are not processed by the conflict resolver.  We wait
+  // and hope the server will provide us with an update that resolves these
+  // conflicts.
+  std::set<syncable::Id> unresolvable_conflicting_item_ids_;
+
+  size_t num_server_conflicting_items;
+  size_t num_hierarchy_conflicting_items;
+  size_t num_encryption_conflicting_items;
 
   // Whether a conflicting item was added or removed since
   // the last call to reset_progress_changed(), if any. In practice this
diff --git a/chrome/browser/sync/sessions/session_state_unittest.cc b/chrome/browser/sync/sessions/session_state_unittest.cc
index 244ece8e..5a373247 100644
--- a/chrome/browser/sync/sessions/session_state_unittest.cc
+++ b/chrome/browser/sync/sessions/session_state_unittest.cc
@@ -130,7 +130,7 @@ TEST_F(SessionStateTest, SyncSessionSnapshotToValue) {
   const bool kHasMoreToSync = false;
   const bool kIsSilenced = true;
   const int kUnsyncedCount = 1053;
-  const int kNumBlockingConflictingUpdates = 1054;
+  const int kNumSimpleConflictingUpdates = 1054;
   const int kNumConflictingUpdates = 1055;
   const bool kDidCommitItems = true;
 
@@ -146,7 +146,7 @@ TEST_F(SessionStateTest, SyncSessionSnapshotToValue) {
                                kHasMoreToSync,
                                kIsSilenced,
                                kUnsyncedCount,
-                               kNumBlockingConflictingUpdates,
+                               kNumSimpleConflictingUpdates,
                                kNumConflictingUpdates,
                                kDidCommitItems,
                                source,
@@ -168,8 +168,8 @@ TEST_F(SessionStateTest, SyncSessionSnapshotToValue) {
   ExpectDictBooleanValue(kHasMoreToSync, *value, "hasMoreToSync");
   ExpectDictBooleanValue(kIsSilenced, *value, "isSilenced");
   ExpectDictIntegerValue(kUnsyncedCount, *value, "unsyncedCount");
-  ExpectDictIntegerValue(kNumBlockingConflictingUpdates, *value,
-                         "numBlockingConflictingUpdates");
+  ExpectDictIntegerValue(kNumSimpleConflictingUpdates, *value,
+                         "numSimpleConflictingUpdates");
   ExpectDictIntegerValue(kNumConflictingUpdates, *value,
                          "numConflictingUpdates");
   ExpectDictBooleanValue(kDidCommitItems, *value,
diff --git a/chrome/browser/sync/sessions/status_controller.cc b/chrome/browser/sync/sessions/status_controller.cc
index ca9903d..bfd22732 100644
--- a/chrome/browser/sync/sessions/status_controller.cc
+++ b/chrome/browser/sync/sessions/status_controller.cc
@@ -233,14 +233,14 @@ bool StatusController::HasConflictingUpdates() const {
   return false;
 }
 
-int StatusController::TotalNumBlockingConflictingItems() const {
+int StatusController::TotalNumSimpleConflictingItems() const {
   DCHECK(!group_restriction_in_effect_)
-      << "TotalNumBlockingConflictingItems applies to all ModelSafeGroups";
+      << "TotalNumSimpleConflictingItems applies to all ModelSafeGroups";
   std::map<ModelSafeGroup, PerModelSafeGroupState*>::const_iterator it =
       per_model_group_.begin();
   int sum = 0;
   for (; it != per_model_group_.end(); ++it) {
-    sum += it->second->conflict_progress.ConflictingItemsSize();
+    sum += it->second->conflict_progress.SimpleConflictingItemsSize();
   }
   return sum;
 }
@@ -252,8 +252,10 @@ int StatusController::TotalNumConflictingItems() const {
     per_model_group_.begin();
   int sum = 0;
   for (; it != per_model_group_.end(); ++it) {
-    sum += it->second->conflict_progress.ConflictingItemsSize();
-    sum += it->second->conflict_progress.NonblockingConflictingItemsSize();
+    sum += it->second->conflict_progress.SimpleConflictingItemsSize();
+    sum += it->second->conflict_progress.EncryptionConflictingItemsSize();
+    sum += it->second->conflict_progress.HierarchyConflictingItemsSize();
+    sum += it->second->conflict_progress.ServerConflictingItemsSize();
   }
   return sum;
 }
diff --git a/chrome/browser/sync/sessions/status_controller.h b/chrome/browser/sync/sessions/status_controller.h
index 651e90c6..a1287fd 100644
--- a/chrome/browser/sync/sessions/status_controller.h
+++ b/chrome/browser/sync/sessions/status_controller.h
@@ -147,17 +147,17 @@ class StatusController {
 
   // If a GetUpdates for any data type resulted in downloading an update that
   // is in conflict, this method returns true.
-  // Note: this includes non-blocking conflicts.
+  // Note: this includes unresolvable conflicts.
   bool HasConflictingUpdates() const;
 
-  // Aggregate sum of ConflictingItemSize() over all ConflictProgress objects
-  // (one for each ModelSafeGroup currently in-use).
-  // Note: this does not include non-blocking conflicts.
-  int TotalNumBlockingConflictingItems() const;
+  // Aggregate sum of SimpleConflictingItemSize() over all ConflictProgress
+  // objects (one for each ModelSafeGroup currently in-use).
+  // Note: this does not include unresolvable conflicts.
+  int TotalNumSimpleConflictingItems() const;
 
-  // Aggregate sum of ConflictingItemSize() and NonblockingConflictingItemsSize
-  // over all ConflictProgress objects (one for each ModelSafeGroup currently
-  // in-use).
+  // Aggregate sum of SimpleConflictingItemSize() and other
+  // ${Type}ConflictingItemSize() methods over all ConflictProgress objects (one
+  // for each ModelSafeGroup currently in-use).
   int TotalNumConflictingItems() const;
 
   // Returns the number of updates received from the sync server.
diff --git a/chrome/browser/sync/sessions/status_controller_unittest.cc b/chrome/browser/sync/sessions/status_controller_unittest.cc
index a207893..c640e53 100644
--- a/chrome/browser/sync/sessions/status_controller_unittest.cc
+++ b/chrome/browser/sync/sessions/status_controller_unittest.cc
@@ -43,7 +43,8 @@ TEST_F(StatusControllerTest, GetsDirty) {
 
   {
     ScopedModelSafeGroupRestriction r(&status, GROUP_UI);
-    status.mutable_conflict_progress()->AddConflictingItemById(syncable::Id());
+    status.mutable_conflict_progress()->
+        AddSimpleConflictingItemById(syncable::Id());
   }
   EXPECT_TRUE(status.TestAndClearIsDirty());
 
@@ -118,7 +119,7 @@ TEST_F(StatusControllerTest, HasConflictingUpdates) {
     EXPECT_FALSE(status.update_progress());
     status.mutable_update_progress()->AddAppliedUpdate(SUCCESS,
         syncable::Id());
-    status.mutable_update_progress()->AddAppliedUpdate(CONFLICT,
+    status.mutable_update_progress()->AddAppliedUpdate(CONFLICT_SIMPLE,
         syncable::Id());
     EXPECT_TRUE(status.update_progress()->HasConflictingUpdates());
   }
@@ -131,6 +132,22 @@ TEST_F(StatusControllerTest, HasConflictingUpdates) {
   }
 }
 
+TEST_F(StatusControllerTest, HasConflictingUpdates_NonBlockingUpdates) {
+  StatusController status(routes_);
+  EXPECT_FALSE(status.HasConflictingUpdates());
+  {
+    ScopedModelSafeGroupRestriction r(&status, GROUP_UI);
+    EXPECT_FALSE(status.update_progress());
+    status.mutable_update_progress()->AddAppliedUpdate(SUCCESS,
+        syncable::Id());
+    status.mutable_update_progress()->AddAppliedUpdate(CONFLICT_HIERARCHY,
+        syncable::Id());
+    EXPECT_TRUE(status.update_progress()->HasConflictingUpdates());
+  }
+
+  EXPECT_TRUE(status.HasConflictingUpdates());
+}
+
 TEST_F(StatusControllerTest, CountUpdates) {
   StatusController status(routes_);
   EXPECT_EQ(0, status.CountUpdates());
@@ -148,17 +165,21 @@ TEST_F(StatusControllerTest, TotalNumConflictingItems) {
   {
     ScopedModelSafeGroupRestriction r(&status, GROUP_UI);
     EXPECT_FALSE(status.conflict_progress());
-    status.mutable_conflict_progress()->AddConflictingItemById(f.NewLocalId());
-    status.mutable_conflict_progress()->AddConflictingItemById(f.NewLocalId());
-    EXPECT_EQ(2, status.conflict_progress()->ConflictingItemsSize());
+    status.mutable_conflict_progress()->
+        AddSimpleConflictingItemById(f.NewLocalId());
+    status.mutable_conflict_progress()->
+        AddSimpleConflictingItemById(f.NewLocalId());
+    EXPECT_EQ(2, status.conflict_progress()->SimpleConflictingItemsSize());
   }
   EXPECT_EQ(2, status.TotalNumConflictingItems());
   {
     ScopedModelSafeGroupRestriction r(&status, GROUP_DB);
     EXPECT_FALSE(status.conflict_progress());
-    status.mutable_conflict_progress()->AddConflictingItemById(f.NewLocalId());
-    status.mutable_conflict_progress()->AddConflictingItemById(f.NewLocalId());
-    EXPECT_EQ(2, status.conflict_progress()->ConflictingItemsSize());
+    status.mutable_conflict_progress()->
+        AddSimpleConflictingItemById(f.NewLocalId());
+    status.mutable_conflict_progress()->
+        AddSimpleConflictingItemById(f.NewLocalId());
+    EXPECT_EQ(2, status.conflict_progress()->SimpleConflictingItemsSize());
   }
   EXPECT_EQ(4, status.TotalNumConflictingItems());
 }
diff --git a/chrome/browser/sync/sessions/sync_session.cc b/chrome/browser/sync/sessions/sync_session.cc
index a1cea48..5ba19ebe 100644
--- a/chrome/browser/sync/sessions/sync_session.cc
+++ b/chrome/browser/sync/sessions/sync_session.cc
@@ -160,7 +160,7 @@ SyncSessionSnapshot SyncSession::TakeSnapshot() const {
       HasMoreToSync(),
       delegate_->IsSyncingCurrentlySilenced(),
       status_controller_->unsynced_handles().size(),
-      status_controller_->TotalNumBlockingConflictingItems(),
+      status_controller_->TotalNumSimpleConflictingItems(),
       status_controller_->TotalNumConflictingItems(),
       status_controller_->did_commit_items(),
       source_,
@@ -207,8 +207,8 @@ std::set<ModelSafeGroup> SyncSession::GetEnabledGroupsWithConflicts() const {
     const sessions::ConflictProgress* conflict_progress =
         status_controller_->GetUnrestrictedConflictProgress(*it);
     if (conflict_progress &&
-        (conflict_progress->ConflictingItemsBegin() !=
-         conflict_progress->ConflictingItemsEnd())) {
+        (conflict_progress->SimpleConflictingItemsBegin() !=
+         conflict_progress->SimpleConflictingItemsEnd())) {
       enabled_groups_with_conflicts.insert(*it);
     }
   }
diff --git a/chrome/browser/sync/sessions/sync_session_unittest.cc b/chrome/browser/sync/sessions/sync_session_unittest.cc
index 830aa01..f41d9d2 100644
--- a/chrome/browser/sync/sessions/sync_session_unittest.cc
+++ b/chrome/browser/sync/sessions/sync_session_unittest.cc
@@ -162,7 +162,7 @@ TEST_F(SyncSessionTest, EnabledGroupsWithConflicts) {
   // Put GROUP_UI in conflict.
   session->mutable_status_controller()->
       GetUnrestrictedMutableConflictProgressForTest(GROUP_UI)->
-      AddConflictingItemById(syncable::Id());
+      AddSimpleConflictingItemById(syncable::Id());
   std::set<ModelSafeGroup> expected_enabled_groups_with_conflicts;
   expected_enabled_groups_with_conflicts.insert(GROUP_UI);
   EXPECT_EQ(expected_enabled_groups_with_conflicts,
diff --git a/chrome/browser/sync/test/integration/two_client_sessions_sync_test.cc b/chrome/browser/sync/test/integration/two_client_sessions_sync_test.cc
index 00dd5f8..e04e5ae 100644
--- a/chrome/browser/sync/test/integration/two_client_sessions_sync_test.cc
+++ b/chrome/browser/sync/test/integration/two_client_sessions_sync_test.cc
@@ -168,7 +168,7 @@ IN_PROC_BROWSER_TEST_F(TwoClientSessionsSyncTest,
   ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
   ASSERT_TRUE(GetClient(1)->AwaitPassphraseRequired());
   ASSERT_EQ(0, GetClient(1)->GetLastSessionSnapshot()->
-      num_blocking_conflicting_updates);
+      num_simple_conflicting_updates);
   // We have two meta nodes (one for each client), the one tab node, plus the
   // basic preference/themes/search engines items.
   ASSERT_EQ(NumberOfDefaultSyncItems() + 3,
@@ -218,9 +218,9 @@ IN_PROC_BROWSER_TEST_F(TwoClientSessionsSyncTest,
   ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
   ASSERT_TRUE(GetClient(1)->AwaitPassphraseRequired());
   ASSERT_EQ(0, GetClient(1)->GetLastSessionSnapshot()->
-      num_blocking_conflicting_updates);
-  // We have nine non-blocking conflicts due to the two meta nodes (one for
-  // each client), plus the basic preference/themes/search engines nodes.
+      num_simple_conflicting_updates);
+  // We have nine encryption conflicts due to the two meta nodes (one for each
+  // client), plus the basic preference/themes/search engines nodes.
   ASSERT_EQ(NumberOfDefaultSyncItems() + 2,
       GetClient(1)->GetLastSessionSnapshot()->
       num_conflicting_updates);  // The encrypted nodes.
@@ -230,7 +230,7 @@ IN_PROC_BROWSER_TEST_F(TwoClientSessionsSyncTest,
       client0_windows.GetMutable()));
   ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
   ASSERT_EQ(0, GetClient(1)->GetLastSessionSnapshot()->
-      num_blocking_conflicting_updates);
+      num_simple_conflicting_updates);
   ASSERT_EQ(NumberOfDefaultSyncItems() + 3,
       GetClient(1)->GetLastSessionSnapshot()->
       num_conflicting_updates);  // The encrypted nodes.
@@ -270,8 +270,8 @@ IN_PROC_BROWSER_TEST_F(TwoClientSessionsSyncTest,
   ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
   ASSERT_TRUE(GetClient(1)->AwaitPassphraseRequired());
   ASSERT_EQ(0, GetClient(1)->GetLastSessionSnapshot()->
-      num_blocking_conflicting_updates);
-  // We have two non-blocking conflicts due to the two meta nodes (one for each
+      num_simple_conflicting_updates);
+  // We have two encryption conflicts due to the two meta nodes (one for each
   // client), plus the basic preference/themes/search engines nodes.
   ASSERT_EQ(NumberOfDefaultSyncItems() + 2,
       GetClient(1)->GetLastSessionSnapshot()->
@@ -335,8 +335,8 @@ IN_PROC_BROWSER_TEST_F(TwoClientSessionsSyncTest,
   ASSERT_TRUE(AwaitQuiescence());
   ASSERT_TRUE(GetClient(1)->AwaitPassphraseRequired());
   ASSERT_EQ(0, GetClient(1)->GetLastSessionSnapshot()->
-      num_blocking_conflicting_updates);
-  // We have three non-blocking conflicts due to the two meta nodes (one for
+      num_simple_conflicting_updates);
+  // We have three encryption conflicts due to the two meta nodes (one for
   // each client), the one tab node, plus the basic preference/themes/search
   // engines nodes.
   ASSERT_GE(NumberOfDefaultSyncItems() + 3,
@@ -394,8 +394,8 @@ IN_PROC_BROWSER_TEST_F(TwoClientSessionsSyncTest,
   ASSERT_TRUE(EnableEncryption(0, syncable::SESSIONS));
   ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
   ASSERT_EQ(0, GetClient(1)->GetLastSessionSnapshot()->
-      num_blocking_conflicting_updates);
-  // We have three non-blocking conflicts due to the two meta nodes (one for
+      num_simple_conflicting_updates);
+  // We have three encryption conflicts due to the two meta nodes (one for
   // each client), the one tab node, plus the basic preference/themes/search
   // engines nodes.
   ASSERT_EQ(NumberOfDefaultSyncItems() + 3,