sync: Per-type update application

sync/engine/sync_directory_update_handler_unittest.cc

Index: sync/engine/sync_directory_update_handler_unittest.cc
diff --git a/sync/engine/sync_directory_update_handler_unittest.cc b/sync/engine/sync_directory_update_handler_unittest.cc
index 53589b850151c5a4498044bc7d49f9c4a7640917..a4970bf4c496312e87ab39b19e1e3729aaf1cd95 100644
--- a/sync/engine/sync_directory_update_handler_unittest.cc
+++ b/sync/engine/sync_directory_update_handler_unittest.cc
@@ -6,16 +6,22 @@
 
 #include "base/compiler_specific.h"
 #include "base/message_loop/message_loop.h"
+#include "base/stl_util.h"
 #include "sync/engine/syncer_proto_util.h"
 #include "sync/internal_api/public/base/model_type.h"
+#include "sync/internal_api/public/test/test_entry_factory.h"
 #include "sync/protocol/sync.pb.h"
 #include "sync/sessions/status_controller.h"
 #include "sync/syncable/directory.h"
 #include "sync/syncable/entry.h"
+#include "sync/syncable/mutable_entry.h"
 #include "sync/syncable/syncable_model_neutral_write_transaction.h"
 #include "sync/syncable/syncable_proto_util.h"
 #include "sync/syncable/syncable_read_transaction.h"
+#include "sync/syncable/syncable_write_transaction.h"
+#include "sync/test/engine/fake_model_worker.h"
 #include "sync/test/engine/test_directory_setter_upper.h"
+#include "sync/test/engine/test_id_factory.h"
 #include "sync/test/engine/test_syncable_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -23,8 +29,14 @@ namespace syncer {
 
 using syncable::UNITTEST;
 
-class SyncDirectoryUpdateHandlerTest : public ::testing::Test {
+class SyncDirectoryUpdateHandlerProcessUpdateTest : public ::testing::Test {

[Nicolas Zea, 2013/11/19 22:45:26]

Comment about what parts of the update handler this is capable of testing?

[rlarocque, 2013/11/21 18:28:47]

Done.

  public:
+  SyncDirectoryUpdateHandlerProcessUpdateTest()
+      : ui_worker_(new FakeModelWorker(GROUP_UI)) {
+  }
+
+  virtual ~SyncDirectoryUpdateHandlerProcessUpdateTest() {}
+
   virtual void SetUp() OVERRIDE {
     dir_maker_.SetUp();
   }
@@ -54,12 +66,18 @@ class SyncDirectoryUpdateHandlerTest : public ::testing::Test {
       SyncDirectoryUpdateHandler* handler,
       const sync_pb::DataTypeProgressMarker& progress);
 
+  scoped_refptr<FakeModelWorker> ui_worker() {
+    return ui_worker_;
+  }
+
  private:
   base::MessageLoop loop_;  // Needed to initialize the directory.
   TestDirectorySetterUpper dir_maker_;
+  scoped_refptr<FakeModelWorker> ui_worker_;
 };
 
-scoped_ptr<sync_pb::SyncEntity> SyncDirectoryUpdateHandlerTest::CreateUpdate(
+scoped_ptr<sync_pb::SyncEntity>
+SyncDirectoryUpdateHandlerProcessUpdateTest::CreateUpdate(
     const std::string& id,
     const std::string& parent,
     const ModelType& type) {
@@ -73,7 +91,7 @@ scoped_ptr<sync_pb::SyncEntity> SyncDirectoryUpdateHandlerTest::CreateUpdate(
   return e.Pass();
 }
 
-void SyncDirectoryUpdateHandlerTest::UpdateSyncEntities(
+void SyncDirectoryUpdateHandlerProcessUpdateTest::UpdateSyncEntities(
     SyncDirectoryUpdateHandler* handler,
     const SyncEntityList& applicable_updates,
     sessions::StatusController* status) {
@@ -81,7 +99,7 @@ void SyncDirectoryUpdateHandlerTest::UpdateSyncEntities(
   handler->UpdateSyncEntities(&trans, applicable_updates, status);
 }
 
-void SyncDirectoryUpdateHandlerTest::UpdateProgressMarkers(
+void SyncDirectoryUpdateHandlerProcessUpdateTest::UpdateProgressMarkers(
     SyncDirectoryUpdateHandler* handler,
     const sync_pb::DataTypeProgressMarker& progress) {
   handler->UpdateProgressMarker(progress);
@@ -90,8 +108,8 @@ void SyncDirectoryUpdateHandlerTest::UpdateProgressMarkers(
 static const char kCacheGuid[] = "IrcjZ2jyzHDV9Io4+zKcXQ==";
 
 // Test that the bookmark tag is set on newly downloaded items.
-TEST_F(SyncDirectoryUpdateHandlerTest, NewBookmarkTag) {
-  SyncDirectoryUpdateHandler handler(dir(), BOOKMARKS);
+TEST_F(SyncDirectoryUpdateHandlerProcessUpdateTest, NewBookmarkTag) {
+  SyncDirectoryUpdateHandler handler(dir(), BOOKMARKS, ui_worker());
   sync_pb::GetUpdatesResponse gu_response;
   sessions::StatusController status;
 
@@ -127,8 +145,9 @@ TEST_F(SyncDirectoryUpdateHandlerTest, NewBookmarkTag) {
 }
 
 // Test the receipt of a type root node.
-TEST_F(SyncDirectoryUpdateHandlerTest, ReceiveServerCreatedBookmarkFolders) {
-  SyncDirectoryUpdateHandler handler(dir(), BOOKMARKS);
+TEST_F(SyncDirectoryUpdateHandlerProcessUpdateTest,
+       ReceiveServerCreatedBookmarkFolders) {
+  SyncDirectoryUpdateHandler handler(dir(), BOOKMARKS, ui_worker());
   sync_pb::GetUpdatesResponse gu_response;
   sessions::StatusController status;
 
@@ -161,15 +180,15 @@ TEST_F(SyncDirectoryUpdateHandlerTest, ReceiveServerCreatedBookmarkFolders) {
 }
 
 // Test the receipt of a non-bookmark item.
-TEST_F(SyncDirectoryUpdateHandlerTest, ReceiveNonBookmarkItem) {
-  SyncDirectoryUpdateHandler handler(dir(), AUTOFILL);
+TEST_F(SyncDirectoryUpdateHandlerProcessUpdateTest, ReceiveNonBookmarkItem) {
+  SyncDirectoryUpdateHandler handler(dir(), PREFERENCES, ui_worker());
   sync_pb::GetUpdatesResponse gu_response;
   sessions::StatusController status;
 
   std::string root = syncable::GetNullId().GetServerId();
   syncable::Id server_id = syncable::Id::CreateFromServerId("xyz");
   scoped_ptr<sync_pb::SyncEntity> e =
-      CreateUpdate(SyncableIdToProto(server_id), root, AUTOFILL);
+      CreateUpdate(SyncableIdToProto(server_id), root, PREFERENCES);
   e->set_server_defined_unique_tag("9PGRuKdX5sHyGMB17CvYTXuC43I=");
 
   // Add it to the applicable updates list.
@@ -192,8 +211,8 @@ TEST_F(SyncDirectoryUpdateHandlerTest, ReceiveNonBookmarkItem) {
 }
 
 // Tests the setting of progress markers.
-TEST_F(SyncDirectoryUpdateHandlerTest, ProcessNewProgressMarkers) {
-  SyncDirectoryUpdateHandler handler(dir(), BOOKMARKS);
+TEST_F(SyncDirectoryUpdateHandlerProcessUpdateTest, ProcessNewProgressMarkers) {
+  SyncDirectoryUpdateHandler handler(dir(), BOOKMARKS, ui_worker());
 
   sync_pb::DataTypeProgressMarker progress;
   progress.set_data_type_id(GetSpecificsFieldNumberFromModelType(BOOKMARKS));
@@ -208,4 +227,417 @@ TEST_F(SyncDirectoryUpdateHandlerTest, ProcessNewProgressMarkers) {
   EXPECT_EQ(progress.data_type_id(), saved.data_type_id());
 }
 
+class SyncDirectoryUpdateHandlerApplyUpdateTest : public ::testing::Test {

[Nicolas Zea, 2013/11/19 22:45:26]

Here too? (and why it's separate/different)

[rlarocque, 2013/11/21 18:28:47]

Done.

The reasons are:
- History.
- Update application involves threads.  I could see these tests becoming more
complicated if for some reason the FakeModelWorkers become insufficient for
testing.

Also, I didn't mention it in the comment, but I could see a justification for
having different classes for update application and processing updates.  The
argument against is that the two concepts are somewhat tied to each other;
Bismarck types will have a no-op update application step.  On the other hand,
should we really need to instantiate a FakeModelWorker that we're not going to
use just to test update processing?

+ public:
+  SyncDirectoryUpdateHandlerApplyUpdateTest()
+      : ui_worker_(new FakeModelWorker(GROUP_UI)),
+        password_worker_(new FakeModelWorker(GROUP_PASSWORD)),
+        passive_worker_(new FakeModelWorker(GROUP_PASSIVE)),
+        update_handler_map_deleter_(&update_handler_map_) {}
+
+  virtual void SetUp() OVERRIDE {
+    dir_maker_.SetUp();
+    entry_factory_.reset(new TestEntryFactory(directory()));
+
+    update_handler_map_.insert(std::make_pair(
+        BOOKMARKS,
+        new SyncDirectoryUpdateHandler(directory(), BOOKMARKS, ui_worker_)));
+    update_handler_map_.insert(std::make_pair(
+        PASSWORDS,
+        new SyncDirectoryUpdateHandler(directory(),
+                                       PASSWORDS,
+                                       password_worker_)));
+  }
+
+  virtual void TearDown() OVERRIDE {
+    dir_maker_.TearDown();
+  }
+
+ protected:
+  void ApplyBookmarkUpdates(sessions::StatusController* status) {
+    update_handler_map_[BOOKMARKS]->ApplyUpdates(status);
+  }
+
+  void ApplyPasswordUpdates(sessions::StatusController* status) {
+    update_handler_map_[PASSWORDS]->ApplyUpdates(status);
+  }
+
+  TestEntryFactory* entry_factory() {
+    return entry_factory_.get();
+  }
+
+  syncable::Directory* directory() {
+    return dir_maker_.directory();
+  }
+
+ private:
+  base::MessageLoop loop_;  // Needed to initialize the directory.
+  TestDirectorySetterUpper dir_maker_;
+  scoped_ptr<TestEntryFactory> entry_factory_;
+
+  scoped_refptr<FakeModelWorker> ui_worker_;
+  scoped_refptr<FakeModelWorker> password_worker_;
+  scoped_refptr<FakeModelWorker> passive_worker_;
+
+  UpdateHandlerMap update_handler_map_;
+  STLValueDeleter<UpdateHandlerMap> update_handler_map_deleter_;
+};
+
+namespace {
+sync_pb::EntitySpecifics DefaultBookmarkSpecifics() {
+  sync_pb::EntitySpecifics result;
+  AddDefaultFieldValue(BOOKMARKS, &result);
+  return result;
+}
+} // namespace
+
+// Test update application for a few bookmark items.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest, SimpleBookmark) {
+  sessions::StatusController status;
+
+  std::string root_server_id = syncable::GetNullId().GetServerId();
+  entry_factory()->CreateUnappliedNewBookmarkItemWithParent(
+      "parent", DefaultBookmarkSpecifics(), root_server_id);
+  entry_factory()->CreateUnappliedNewBookmarkItemWithParent(
+      "child", DefaultBookmarkSpecifics(), "parent");
+
+  ApplyBookmarkUpdates(&status);
+
+  EXPECT_EQ(0, status.num_encryption_conflicts())

[Nicolas Zea, 2013/11/19 22:45:26]

Check the is_unsynced/is_unapplied bits are set appropriately? (here and below)

[rlarocque, 2013/11/21 18:28:47]

Good idea.

We won't be able to rely on StatusController for much longer.  I've added a
bunch of assertions based on the entries' state, rather than the status
controller.

+      << "Simple update shouldn't result in conflicts";
+  EXPECT_EQ(0, status.num_hierarchy_conflicts())
+      << "Simple update shouldn't result in conflicts";
+  EXPECT_EQ(2, status.num_updates_applied())
+      << "All items should have been successfully applied";
+}
+
+// Test that the applicator can handle updates delivered out of order.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest,
+       BookmarkChildrenBeforeParent) {
+  // Start with some bookmarks whose parents are unknown.
+  std::string root_server_id = syncable::GetNullId().GetServerId();
+  entry_factory()->CreateUnappliedNewBookmarkItemWithParent(
+      "a_child_created_first", DefaultBookmarkSpecifics(), "parent");
+  entry_factory()->CreateUnappliedNewBookmarkItemWithParent(
+      "x_child_created_first", DefaultBookmarkSpecifics(), "parent");
+
+  // Update application will fail.
+  sessions::StatusController status1;
+  ApplyBookmarkUpdates(&status1);
+  EXPECT_EQ(0, status1.num_updates_applied());
+  EXPECT_EQ(2, status1.num_hierarchy_conflicts());
+
+  // Now add their parent and a few siblings.
+  entry_factory()->CreateUnappliedNewBookmarkItemWithParent(
+      "parent", DefaultBookmarkSpecifics(), root_server_id);
+  entry_factory()->CreateUnappliedNewBookmarkItemWithParent(
+      "a_child_created_second", DefaultBookmarkSpecifics(), "parent");
+  entry_factory()->CreateUnappliedNewBookmarkItemWithParent(
+      "x_child_created_second", DefaultBookmarkSpecifics(), "parent");
+
+  // Update application will succeed.
+  sessions::StatusController status2;
+  ApplyBookmarkUpdates(&status2);
+  EXPECT_EQ(5, status2.num_updates_applied())
+      << "All updates should have been successfully applied";
+}
+
+// Try to apply changes on an item that is both IS_UNSYNCED and
+// IS_UNAPPLIED_UPDATE.  Conflict resolution should be performed.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest, SimpleBookmarkConflict) {
+  int64 handle = entry_factory()->CreateUnappliedAndUnsyncedBookmarkItem("x");
+
+  int original_server_version = -10;
+  {
+    syncable::ReadTransaction trans(FROM_HERE, directory());
+    syncable::Entry e(&trans, syncable::GET_BY_HANDLE, handle);
+    original_server_version = e.GetServerVersion();
+    ASSERT_NE(original_server_version, e.GetBaseVersion());
+  }
+
+  sessions::StatusController status;
+  ApplyBookmarkUpdates(&status);
+  EXPECT_EQ(1, status.num_server_overwrites())
+      << "Unsynced and unapplied item conflict should be resolved";
+  EXPECT_EQ(0, status.num_updates_applied())
+      << "Update should not be applied; we should override the server.";
+
+  {
+    syncable::ReadTransaction trans(FROM_HERE, directory());
+    syncable::Entry e(&trans, syncable::GET_BY_HANDLE, handle);
+    EXPECT_EQ(original_server_version, e.GetServerVersion());
+    EXPECT_EQ(original_server_version, e.GetBaseVersion());
+  }
+}
+
+// Create a simple conflict that is also a hierarchy conflict.  If we were to
+// follow the normal "server wins" logic, we'd end up violating hierarchy
+// constraints.  The hierarchy conflict must take precedence.  We can not allow
+// the update to be applied.  The item must remain in the conflict state.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest, HierarchyAndSimpleConflict) {
+  // Create a simply-conflicting item.  It will start with valid parent ids.
+  int64 handle = entry_factory()->CreateUnappliedAndUnsyncedBookmarkItem(
+      "orphaned_by_server");
+  {
+    // Manually set the SERVER_PARENT_ID to bad value.
+    // A bad parent indicates a hierarchy conflict.
+    syncable::WriteTransaction trans(FROM_HERE, UNITTEST, directory());
+    syncable::MutableEntry entry(&trans, syncable::GET_BY_HANDLE, handle);
+    ASSERT_TRUE(entry.good());
+
+    entry.PutServerParentId(TestIdFactory::MakeServer("bogus_parent"));
+  }
+
+  sessions::StatusController status;
+  ApplyBookmarkUpdates(&status);
+  EXPECT_EQ(0, status.num_updates_applied());
+  EXPECT_EQ(0, status.num_server_overwrites());
+  EXPECT_EQ(1, status.num_hierarchy_conflicts());
+}
+
+// Attempt to apply an udpate that would create a bookmark folder loop.  This
+// application should fail.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest, BookmarkFolderLoop) {
+  // Item 'X' locally has parent of 'root'.  Server is updating it to have
+  // parent of 'Y'.
+  {
+    // Create it as a child of root node.
+    int64 handle = entry_factory()->CreateSyncedItem("X", BOOKMARKS, true);
+
+    syncable::WriteTransaction trans(FROM_HERE, UNITTEST, directory());
+    syncable::MutableEntry entry(&trans, syncable::GET_BY_HANDLE, handle);
+    ASSERT_TRUE(entry.good());
+
+    // Re-parent from root to "Y"
+    entry.PutServerVersion(entry_factory()->GetNextRevision());
+    entry.PutIsUnappliedUpdate(true);
+    entry.PutServerParentId(TestIdFactory::MakeServer("Y"));
+  }
+
+  // Item 'Y' is child of 'X'.
+  entry_factory()->CreateUnsyncedItem(
+      TestIdFactory::MakeServer("Y"), TestIdFactory::MakeServer("X"), "Y", true,
+      BOOKMARKS, NULL);
+
+  // If the server's update were applied, we would have X be a child of Y, and Y
+  // as a child of X.  That's a directory loop.  The UpdateApplicator should
+  // prevent the update from being applied and note that this is a hierarchy
+  // conflict.
+
+  sessions::StatusController status;
+  ApplyBookmarkUpdates(&status);
+
+  // This should count as a hierarchy conflict.
+  EXPECT_EQ(1, status.num_hierarchy_conflicts());
+}
+
+// Test update application where the update has been orphaned by a local folder
+// deletion.  The update application attempt should fail.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest,
+       HierarchyConflictDeletedParent) {
+  // Create a locally deleted parent item.
+  int64 parent_handle;
+  entry_factory()->CreateUnsyncedItem(
+      syncable::Id::CreateFromServerId("parent"), TestIdFactory::root(),
+      "parent", true, BOOKMARKS, &parent_handle);
+  {
+    syncable::WriteTransaction trans(FROM_HERE, UNITTEST, directory());
+    syncable::MutableEntry entry(&trans,
+                                 syncable::GET_BY_HANDLE,
+                                 parent_handle);
+    entry.PutIsDel(true);
+  }
+
+  // Create an incoming child from the server.
+  entry_factory()->CreateUnappliedNewItemWithParent(
+      "child", DefaultBookmarkSpecifics(), "parent");
+
+  // The server's update may seem valid to some other client, but on this client
+  // that new item's parent no longer exists.  The update should not be applied
+  // and the update applicator should indicate this is a hierarchy conflict.
+
+  sessions::StatusController status;
+  ApplyBookmarkUpdates(&status);
+  EXPECT_EQ(1, status.num_hierarchy_conflicts());
+}
+
+// Attempt to apply an update that deletes a folder where the folder has
+// locally-created children.  The update application should fail.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest,
+       HierarchyConflictDeleteNonEmptyDirectory) {
+  // Create a server-deleted directory.
+  {
+    // Create it as a child of root node.
+    int64 handle = entry_factory()->CreateSyncedItem("parent", BOOKMARKS, true);
+
+    syncable::WriteTransaction trans(FROM_HERE, UNITTEST, directory());
+    syncable::MutableEntry entry(&trans, syncable::GET_BY_HANDLE, handle);
+    ASSERT_TRUE(entry.good());
+
+    // Delete it on the server.
+    entry.PutServerVersion(entry_factory()->GetNextRevision());
+    entry.PutIsUnappliedUpdate(true);
+    entry.PutServerParentId(TestIdFactory::root());
+    entry.PutServerIsDel(true);
+  }
+
+  // Create a local child of the server-deleted directory.
+  entry_factory()->CreateUnsyncedItem(
+      TestIdFactory::MakeServer("child"), TestIdFactory::MakeServer("parent"),
+      "child", false, BOOKMARKS, NULL);
+
+  // The server's request to delete the directory must be ignored, otherwise our
+  // unsynced new child would be orphaned.  This is a hierarchy conflict.
+
+  sessions::StatusController status;
+  ApplyBookmarkUpdates(&status);
+
+  // This should count as a hierarchy conflict.
+  EXPECT_EQ(1, status.num_hierarchy_conflicts());
+}
+
+// Attempt to apply updates where the updated item's parent is not known to this
+// client.  The update application attempt should fail.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest,
+       HierarchyConflictUnknownParent) {
+  // We shouldn't be able to do anything with either of these items.
+  entry_factory()->CreateUnappliedNewItemWithParent(
+      "some_item", DefaultBookmarkSpecifics(), "unknown_parent");
+  entry_factory()->CreateUnappliedNewItemWithParent(
+      "some_other_item", DefaultBookmarkSpecifics(), "some_item");
+
+  sessions::StatusController status;
+  ApplyBookmarkUpdates(&status);
+
+  EXPECT_EQ(2, status.num_hierarchy_conflicts())
+      << "All updates with an unknown ancestors should be in conflict";
+  EXPECT_EQ(0, status.num_updates_applied())
+      << "No item with an unknown ancestor should be applied";
+}
+
+// Attempt application of a mix of items.  Some update application attempts will
+// fail due to hierarchy conflicts.  Others should succeed.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest, ItemsBothKnownAndUnknown) {
+  // See what happens when there's a mixture of good and bad updates.
+  std::string root_server_id = syncable::GetNullId().GetServerId();
+  entry_factory()->CreateUnappliedNewItemWithParent(
+      "first_unknown_item", DefaultBookmarkSpecifics(), "unknown_parent");
+  entry_factory()->CreateUnappliedNewItemWithParent(
+      "first_known_item", DefaultBookmarkSpecifics(), root_server_id);
+  entry_factory()->CreateUnappliedNewItemWithParent(
+      "second_unknown_item", DefaultBookmarkSpecifics(), "unknown_parent");
+  entry_factory()->CreateUnappliedNewItemWithParent(
+      "second_known_item", DefaultBookmarkSpecifics(), "first_known_item");
+  entry_factory()->CreateUnappliedNewItemWithParent(
+      "third_known_item", DefaultBookmarkSpecifics(), "fourth_known_item");
+  entry_factory()->CreateUnappliedNewItemWithParent(
+      "fourth_known_item", DefaultBookmarkSpecifics(), root_server_id);
+
+  sessions::StatusController status;
+  ApplyBookmarkUpdates(&status);
+
+  EXPECT_EQ(2, status.num_hierarchy_conflicts())
+      << "The updates with unknown ancestors should be in conflict";
+  EXPECT_EQ(4, status.num_updates_applied())
+      << "The updates with known ancestors should be successfully applied";
+}
+
+// Attempt application of password upates where the passphrase is known.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest, DecryptablePassword) {
+  // Decryptable password updates should be applied.
+  Cryptographer* cryptographer;
+  {
+    // Storing the cryptographer separately is bad, but for this test we
+    // know it's safe.
+    syncable::ReadTransaction trans(FROM_HERE, directory());
+    cryptographer = directory()->GetCryptographer(&trans);
+  }
+
+  KeyParams params = {"localhost", "dummy", "foobar"};
+  cryptographer->AddKey(params);
+
+  sync_pb::EntitySpecifics specifics;
+  sync_pb::PasswordSpecificsData data;
+  data.set_origin("http://example.com");
+
+  cryptographer->Encrypt(data,
+                         specifics.mutable_password()->mutable_encrypted());
+  entry_factory()->CreateUnappliedNewItem("item", specifics, false);
+
+  sessions::StatusController status;
+  ApplyPasswordUpdates(&status);
+
+  EXPECT_EQ(1, status.num_updates_applied())
+      << "The updates that can be decrypted should be applied";
+}
+
+// Attempt application of encrypted items when the passphrase is not known.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest, UndecryptableData) {
+  // Undecryptable updates should not be applied.
+  sync_pb::EntitySpecifics encrypted_bookmark;
+  encrypted_bookmark.mutable_encrypted();
+  AddDefaultFieldValue(BOOKMARKS, &encrypted_bookmark);
+  std::string root_server_id = syncable::GetNullId().GetServerId();
+  entry_factory()->CreateUnappliedNewItemWithParent(
+      "folder", encrypted_bookmark, root_server_id);
+  entry_factory()->CreateUnappliedNewItem("item2", encrypted_bookmark, false);
+  sync_pb::EntitySpecifics encrypted_password;
+  encrypted_password.mutable_password();
+  entry_factory()->CreateUnappliedNewItem("item3", encrypted_password, false);
+
+  sessions::StatusController status;
+  ApplyBookmarkUpdates(&status);
+  ApplyPasswordUpdates(&status);
+
+  EXPECT_EQ(3, status.num_encryption_conflicts())
+      << "Updates that can't be decrypted should be in encryption conflict";
+  EXPECT_EQ(0, status.num_updates_applied())
+      << "No update that can't be decrypted should be applied";
+}
+
+// Test a mix of decryptable and undecryptable updates.
+TEST_F(SyncDirectoryUpdateHandlerApplyUpdateTest, SomeUndecryptablePassword) {
+  Cryptographer* cryptographer;
+  // Only decryptable password updates should be applied.
+  {
+    sync_pb::EntitySpecifics specifics;
+    sync_pb::PasswordSpecificsData data;
+    data.set_origin("http://example.com/1");
+    {
+      syncable::ReadTransaction trans(FROM_HERE, directory());
+      cryptographer = directory()->GetCryptographer(&trans);
+
+      KeyParams params = {"localhost", "dummy", "foobar"};
+      cryptographer->AddKey(params);
+
+      cryptographer->Encrypt(data,
+          specifics.mutable_password()->mutable_encrypted());
+    }
+    entry_factory()->CreateUnappliedNewItem("item1", specifics, false);
+  }
+  {
+    // Create a new cryptographer, independent of the one in the session.
+    Cryptographer other_cryptographer(cryptographer->encryptor());
+    KeyParams params = {"localhost", "dummy", "bazqux"};
+    other_cryptographer.AddKey(params);
+
+    sync_pb::EntitySpecifics specifics;
+    sync_pb::PasswordSpecificsData data;
+    data.set_origin("http://example.com/2");
+
+    other_cryptographer.Encrypt(data,
+        specifics.mutable_password()->mutable_encrypted());
+    entry_factory()->CreateUnappliedNewItem("item2", specifics, false);
+  }
+
+  sessions::StatusController status;
+  ApplyPasswordUpdates(&status);
+
+  EXPECT_EQ(1, status.num_encryption_conflicts())
+      << "The updates that can't be decrypted should be in encryption "
+      << "conflict";
+  EXPECT_EQ(1, status.num_updates_applied())
+      << "The undecryptable password update shouldn't be applied";
+}
+
 }  // namespace syncer