sync: Remove the remaining conflict sets code

chrome/browser/sync/engine/apply_updates_command_unittest.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <string>
 
 #include "base/format_macros.h"
 #include "base/location.h"
 #include "base/stringprintf.h"
 #include "chrome/browser/sync/engine/apply_updates_command.h"
@@ skipping 125 matching lines @@
     if (item_id.ServerKnows()) {
       entry.Put(syncable::SERVER_SPECIFICS, default_specifics);
       entry.Put(syncable::SERVER_IS_DIR, is_folder);
       entry.Put(syncable::SERVER_PARENT_ID, parent_id);
       entry.Put(syncable::SERVER_IS_DEL, false);
     }
     if (metahandle_out)
       *metahandle_out = entry.Get(syncable::META_HANDLE);
   }
 
+  // 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());
+    EXPECT_TRUE(dir.good());

[akalin, 2012/02/10 23:11:54]

if dir is bad, probably want to return invalid metahandle, too

[rlarocque, 2012/02/11 00:02:16]

The dir is never bad.  Will fix anyway, just to be consistent.

+    WriteTransaction trans(FROM_HERE, UNITTEST, dir);
+    MutableEntry entry(&trans, syncable::GET_BY_HANDLE, metahandle);
+    EXPECT_TRUE(entry.good());

[akalin, 2012/02/10 23:11:54]

you can do

if(!entry.good()) {
  ADD_FAILURE();
  return -1;
}

to be less redundant

[rlarocque, 2012/02/11 00:02:16]

Good idea, thanks!

+    if (!entry.good())
+      return -1;

[akalin, 2012/02/10 23:11:54]

use kInvalidMetaHandle from syncable.h

[rlarocque, 2012/02/11 00:02:16]

I was looking for something like that.  Will do.

+
+    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());
+    EXPECT_TRUE(dir.good());

[akalin, 2012/02/10 23:11:54]

as above

[rlarocque, 2012/02/11 00:02:16]

Done.

+    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);
+    EXPECT_TRUE(entry.good());
+    if (!entry.good())
+      return -1;

[akalin, 2012/02/10 23:11:54]

as above

[rlarocque, 2012/02/11 00:02:16]

Done.

+
+    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);
+
+    EXPECT_TRUE(entry.PutPredecessor(id_factory_.root()));

[akalin, 2012/02/10 23:11:54]

return invalidmetahandle here?

[rlarocque, 2012/02/11 00:02:16]

Done.

+    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:
   int64 next_revision_;
   DISALLOW_COPY_AND_ASSIGN(ApplyUpdatesCommandTest);
 };
 
 TEST_F(ApplyUpdatesCommandTest, Simple) {
   string root_server_id = syncable::GetNullId().GetServerId();
   CreateUnappliedNewItemWithParent("parent",
                                    DefaultBookmarkSpecifics(),
                                    root_server_id);
   CreateUnappliedNewItemWithParent("child",
                                    DefaultBookmarkSpecifics(),
                                    "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);
   ASSERT_TRUE(status->update_progress());
   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";
 }
 
 TEST_F(ApplyUpdatesCommandTest, UpdateWithChildrenBeforeParents) {
   // Set a bunch of updates which are difficult to apply in the order
   // they're received due to dependencies on other unseen items.
   string root_server_id = syncable::GetNullId().GetServerId();
   CreateUnappliedNewItemWithParent("a_child_created_first",
@@ skipping 14 matching lines @@
 
   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->update_progress());
   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(),
                                    "unknown_parent");
   CreateUnappliedNewItemWithParent("some_other_item",
                                    DefaultBookmarkSpecifics(),
                                    "some_item");
 
   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->update_progress());
   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";
 }
 
 TEST_F(ApplyUpdatesCommandTest, ItemsBothKnownAndUnknown) {
   // See what happens when there's a mixture of good and bad updates.
   string root_server_id = syncable::GetNullId().GetServerId();
   CreateUnappliedNewItemWithParent("first_unknown_item",
                                    DefaultBookmarkSpecifics(),
@@ skipping 16 matching lines @@
 
   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->update_progress());
   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";
 }
 
 TEST_F(ApplyUpdatesCommandTest, DecryptablePassword) {
   // Decryptable password updates should be applied.
   Cryptographer* cryptographer;
   {
       // Storing the cryptographer separately is bad, but for this test we
@@ skipping 18 matching lines @@
 
   ExpectGroupToChange(apply_updates_command_, GROUP_PASSWORD);
   apply_updates_command_.ExecuteImpl(session());
 
   sessions::StatusController* status = session()->mutable_status_controller();
   sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSWORD);
   ASSERT_TRUE(status->update_progress());
   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";
 }
 
 TEST_F(ApplyUpdatesCommandTest, UndecryptableData) {
   // Undecryptable updates should not be applied.
   sync_pb::EntitySpecifics encrypted_bookmark;
   encrypted_bookmark.mutable_encrypted();
   AddDefaultExtensionValue(syncable::BOOKMARKS, &encrypted_bookmark);
@@ skipping 12 matching lines @@
   sessions::StatusController* status = session()->mutable_status_controller();
   EXPECT_TRUE(status->HasConflictingUpdates())
     << "Updates that can't be decrypted should trigger the syncer to have "
     << "conflicting updates.";
   {
     sessions::ScopedModelSafeGroupRestriction r(status, GROUP_UI);
     ASSERT_TRUE(status->update_progress());
     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";
   }
   {
     sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSWORD);
     ASSERT_TRUE(status->update_progress());
     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";
   }
 }
 
 TEST_F(ApplyUpdatesCommandTest, SomeUndecryptablePassword) {
   // Only decryptable password updates should be applied.
   {
     sync_pb::EntitySpecifics specifics;
@@ skipping 35 matching lines @@
   sessions::StatusController* status = session()->mutable_status_controller();
   EXPECT_TRUE(status->HasConflictingUpdates())
     << "Updates that can't be decrypted should trigger the syncer to have "
     << "conflicting updates.";
   {
     sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSWORD);
     ASSERT_TRUE(status->update_progress());
     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";
   }
 }
 
 TEST_F(ApplyUpdatesCommandTest, NigoriUpdate) {
   // Storing the cryptographer separately is bad, but for this test we
   // know it's safe.
   Cryptographer* cryptographer;
@@ skipping 26 matching lines @@
 
   ExpectGroupToChange(apply_updates_command_, GROUP_PASSIVE);
   apply_updates_command_.ExecuteImpl(session());
 
   sessions::StatusController* status = session()->mutable_status_controller();
   sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE);
   ASSERT_TRUE(status->update_progress());
   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";
 
   EXPECT_FALSE(cryptographer->is_ready());
   EXPECT_TRUE(cryptographer->has_pending_keys());
   EXPECT_TRUE(
       cryptographer->GetEncryptedTypes()
           .Equals(syncable::ModelTypeSet::All()));
 }
@@ skipping 33 matching lines @@
 
   ExpectGroupToChange(apply_updates_command_, GROUP_PASSIVE);
   apply_updates_command_.ExecuteImpl(session());
 
   sessions::StatusController* status = session()->mutable_status_controller();
   sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE);
   ASSERT_TRUE(status->update_progress());
   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";
 
   EXPECT_FALSE(cryptographer->is_ready());
   EXPECT_TRUE(cryptographer->has_pending_keys());
   EXPECT_TRUE(
       cryptographer->GetEncryptedTypes()
           .Equals(syncable::ModelTypeSet::All()));
 }
@@ skipping 69 matching lines @@
 
   ExpectGroupToChange(apply_updates_command_, GROUP_PASSIVE);
   apply_updates_command_.ExecuteImpl(session());
 
   sessions::StatusController* status = session()->mutable_status_controller();
   sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE);
   ASSERT_TRUE(status->update_progress());
   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";
   EXPECT_FALSE(cryptographer->has_pending_keys());
   EXPECT_TRUE(cryptographer->is_ready());
   {
     ScopedDirLookup dir(syncdb()->manager(), syncdb()->name());
     ASSERT_TRUE(dir.good());
     ReadTransaction trans(FROM_HERE, dir);
 
@@ skipping 80 matching lines @@
 
   ExpectGroupToChange(apply_updates_command_, GROUP_PASSIVE);
   apply_updates_command_.ExecuteImpl(session());
 
   sessions::StatusController* status = session()->mutable_status_controller();
   sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE);
   ASSERT_TRUE(status->update_progress());
   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";
   EXPECT_FALSE(cryptographer->is_ready());
   EXPECT_TRUE(cryptographer->has_pending_keys());
   {
     ScopedDirLookup dir(syncdb()->manager(), syncdb()->name());
     ASSERT_TRUE(dir.good());
     ReadTransaction trans(FROM_HERE, dir);
 
     // Since we have pending keys, we would have failed to encrypt, but the
     // cryptographer should be updated.
     EXPECT_FALSE(VerifyUnsyncedChangesAreEncrypted(&trans, encrypted_types));
     EXPECT_TRUE(cryptographer->GetEncryptedTypes().Equals(
         syncable::ModelTypeSet().All()));
     EXPECT_FALSE(cryptographer->is_ready());
     EXPECT_TRUE(cryptographer->has_pending_keys());
 
     Syncer::UnsyncedMetaHandles handles;
     SyncerUtil::GetUnsyncedEntries(&trans, &handles);
     EXPECT_EQ(2*batch_s+1, handles.size());
   }
 }
 
 }  // namespace browser_sync