[Sync] Don't crash for encryption errors

sync/internal_api/sync_manager_impl_unittest.cc

Index: sync/internal_api/sync_manager_impl_unittest.cc
diff --git a/sync/internal_api/sync_manager_impl_unittest.cc b/sync/internal_api/sync_manager_impl_unittest.cc
index 41b2e2df4b812fe06aa8eac08a32809131e66088..a4d92b312b60465cfa318b198c888287ae598e82 100644
--- a/sync/internal_api/sync_manager_impl_unittest.cc
+++ b/sync/internal_api/sync_manager_impl_unittest.cc
@@ -69,7 +69,7 @@
 #include "sync/test/fake_encryptor.h"
 #include "sync/util/cryptographer.h"
 #include "sync/util/extensions_activity.h"
-#include "sync/util/test_unrecoverable_error_handler.h"
+#include "sync/util/mock_unrecoverable_error_handler.h"
 #include "sync/util/time.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -836,7 +836,8 @@ class SyncManagerTest : public testing::Test,
   };
 
   SyncManagerTest()
-      : sync_manager_("Test sync manager") {
+      : sync_manager_("Test sync manager"),
+        mock_unrecoverable_error_handler_(NULL) {
     switches_.encryption_method =
         InternalComponentsFactory::ENCRYPTION_KEYSTORE;
   }
@@ -886,7 +887,8 @@ class SyncManagerTest : public testing::Test,
     args.invalidator_client_id = "fake_invalidator_client_id";
     args.internal_components_factory.reset(GetFactory());
     args.encryptor = &encryptor_;
-    args.unrecoverable_error_handler.reset(new TestUnrecoverableErrorHandler);
+    mock_unrecoverable_error_handler_ = new MockUnrecoverableErrorHandler();
+    args.unrecoverable_error_handler.reset(mock_unrecoverable_error_handler_);
     args.cancelation_signal = &cancelation_signal_;
     sync_manager_.Init(&args);
 
@@ -1080,6 +1082,12 @@ class SyncManagerTest : public testing::Test,
               sync_manager_.GetEncryptionHandler()->GetPassphraseType());
   }
 
+  bool HasUnrecoverableError() {
+    if (mock_unrecoverable_error_handler_)
+      return mock_unrecoverable_error_handler_->invocation_count() > 0;
+    return false;
+  }
+
  private:
   // Needed by |sync_manager_|.
   base::MessageLoop message_loop_;
@@ -1099,6 +1107,7 @@ class SyncManagerTest : public testing::Test,
   StrictMock<SyncEncryptionHandlerObserverMock> encryption_observer_;
   InternalComponentsFactory::Switches switches_;
   InternalComponentsFactory::StorageOption storage_used_;
+  MockUnrecoverableErrorHandler* mock_unrecoverable_error_handler_;

[stanisc, 2015/06/09 00:04:02]

Consider adding a comment about the ownership.

[Nicolas Zea, 2015/06/09 19:42:03]

Done.

 };
 
 TEST_F(SyncManagerTest, GetAllNodesForTypeTest) {
@@ -2058,6 +2067,93 @@ TEST_F(SyncManagerTest, UpdatePasswordReencryptEverything) {
   EXPECT_FALSE(ResetUnsyncedEntry(PASSWORDS, client_tag));
 }
 
+// Test that attempting to start up with corrupted password data triggers
+// an unrecoverable error (rather than crashing).
+TEST_F(SyncManagerTest, ReencryptEverythingWithUnrecoverableErrorPasswords) {
+  const char kClientTag[] = "client_tag";
+
+  EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, DEFAULT_ENCRYPTION));
+  sync_pb::EntitySpecifics entity_specifics;
+  {
+    // Create a synced bookmark with undecryptable data.
+    ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
+
+    Cryptographer other_cryptographer(&encryptor_);
+    KeyParams fake_params = {"localhost", "dummy", "fake_key"};
+    other_cryptographer.AddKey(fake_params);
+    sync_pb::PasswordSpecificsData data;
+    data.set_password_value("secret");
+    other_cryptographer.Encrypt(
+        data,
+        entity_specifics.mutable_password()->mutable_encrypted());
+
+    // Set up the real cryptographer with a different key.
+    KeyParams real_params = {"localhost", "username", "real_key"};
+    trans.GetCryptographer()->AddKey(real_params);
+  }
+  MakeServerNode(sync_manager_.GetUserShare(), PASSWORDS, kClientTag,
+                 syncable::GenerateSyncableHash(PASSWORDS,
+                                                kClientTag),
+                 entity_specifics);
+  EXPECT_FALSE(ResetUnsyncedEntry(PASSWORDS, kClientTag));
+
+  // Force a re-encrypt everything. Should trigger an unrecoverable error due
+  // to being unable to decrypt the data that was previously applied.
+  testing::Mock::VerifyAndClearExpectations(&encryption_observer_);
+  EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
+  EXPECT_CALL(encryption_observer_, OnCryptographerStateChanged(_));
+  EXPECT_CALL(encryption_observer_, OnEncryptedTypesChanged(_, false));
+  EXPECT_FALSE(HasUnrecoverableError());
+  sync_manager_.GetEncryptionHandler()->Init();
+  PumpLoop();
+  EXPECT_TRUE(HasUnrecoverableError());
+}
+
+// Test that attempting to start up with corrupted bookmark data triggers
+// an unrecoverable error (rather than crashing).
+TEST_F(SyncManagerTest, ReencryptEverythingWithUnrecoverableErrorBookmarks) {
+  const char kClientTag[] = "client_tag";
+  EXPECT_CALL(encryption_observer_,
+              OnEncryptedTypesChanged(
+                  HasModelTypes(EncryptableUserTypes()), true));
+  EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, FULL_ENCRYPTION));
+  sync_pb::EntitySpecifics entity_specifics;
+  {
+    // Create a synced bookmark with undecryptable data.
+    ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
+
+    Cryptographer other_cryptographer(&encryptor_);
+    KeyParams fake_params = {"localhost", "dummy", "fake_key"};
+    other_cryptographer.AddKey(fake_params);
+    sync_pb::EntitySpecifics bm_specifics;
+    bm_specifics.mutable_bookmark()->set_title("title");
+    bm_specifics.mutable_bookmark()->set_url("url");
+    sync_pb::EncryptedData encrypted;
+    other_cryptographer.Encrypt(bm_specifics, &encrypted);
+    entity_specifics.mutable_encrypted()->CopyFrom(encrypted);
+
+    // Set up the real cryptographer with a different key.
+    KeyParams real_params = {"localhost", "username", "real_key"};
+    trans.GetCryptographer()->AddKey(real_params);
+  }
+  MakeServerNode(sync_manager_.GetUserShare(), BOOKMARKS, kClientTag,
+                 syncable::GenerateSyncableHash(BOOKMARKS,
+                                                kClientTag),
+                 entity_specifics);
+  EXPECT_FALSE(ResetUnsyncedEntry(BOOKMARKS, kClientTag));
+
+  // Force a re-encrypt everything. Should trigger an unrecoverable error due
+  // to being unable to decrypt the data that was previously applied.
+  testing::Mock::VerifyAndClearExpectations(&encryption_observer_);
+  EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
+  EXPECT_CALL(encryption_observer_, OnCryptographerStateChanged(_));
+  EXPECT_CALL(encryption_observer_, OnEncryptedTypesChanged(_, true));
+  EXPECT_FALSE(HasUnrecoverableError());
+  sync_manager_.GetEncryptionHandler()->Init();
+  PumpLoop();
+  EXPECT_TRUE(HasUnrecoverableError());
+}
+
 // Verify SetTitle(..) doesn't unnecessarily set IS_UNSYNCED for bookmarks
 // when we write the same data, but does set it when we write new data.
 TEST_F(SyncManagerTest, SetBookmarkTitle) {