Move control-type logic from SyncEngineBackend to SyncManager

No behavioral changes are expected, although it's not trivial to claim
so due to subtle ordering changes.

In particular, the registration of control types is now taken care of
in SyncManagerImpl, immediately before NotifyInitializationSuccess() is
run, which continues in SyncEngineBackend where the old code lived.

Somewhat related, the mechanism to brodcast SyncEncryptionHandler-
related events via a proxy observer is also moved to SyncManagerImpl.

We do this in preparation for changing the behavior in future patches.

Bug: 922900
Change-Id: Ie43d06e40403e1bb0476ffec6f82d2596e018588
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1619813
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662582}

components/sync/driver/glue/sync_engine_backend.cc

@@ -115,14 +115,6 @@ void SyncEngineBackend::OnInitializationComplete(
     return;
   }
 
-  // Register for encryption related changes now. We have to do this before
-  // the initializing downloading control types or initializing the encryption
-  // handler in order to receive notifications triggered during encryption
-  // startup.
-  DCHECK(encryption_observer_proxy_);
-  sync_manager_->GetEncryptionHandler()->AddObserver(
-      encryption_observer_proxy_.get());
-
   // Sync manager initialization is complete, so we can schedule recurring
   // SaveChanges.
   base::SequencedTaskRunnerHandle::Get()->PostTask(
@@ -149,15 +141,6 @@ void SyncEngineBackend::OnInitializationComplete(
   ModelTypeSet new_control_types =
       registrar_->ConfigureDataTypes(ControlTypes(), ModelTypeSet());
 
-  // Control types don't have DataTypeControllers, but they need to have
-  // update handlers registered in ModelTypeRegistry. Register them here.
-  ModelTypeConnector* model_type_connector =
-      sync_manager_->GetModelTypeConnector();
-  ModelTypeSet control_types = ControlTypes();
-  for (ModelType type : control_types) {
-    model_type_connector->RegisterDirectoryType(type, GROUP_PASSIVE);
-  }
-
   ModelSafeRoutingInfo routing_info;
   registrar_->GetModelSafeRoutingInfo(&routing_info);
   SDVLOG(1) << "Control Types " << ModelTypeSetToString(new_control_types)
@@ -326,10 +309,6 @@ void SyncEngineBackend::DoInitialize(SyncEngine::InitParams params) {
   DCHECK(params.registrar);
   registrar_ = std::move(params.registrar);
 
-  DCHECK(!encryption_observer_proxy_);
-  DCHECK(params.encryption_observer_proxy);
-  encryption_observer_proxy_ = std::move(params.encryption_observer_proxy);
-
   sync_manager_ = params.sync_manager_factory->CreateSyncManager(name_);
   sync_manager_->AddObserver(this);
 
@@ -345,6 +324,7 @@ void SyncEngineBackend::DoInitialize(SyncEngine::InitParams params) {
   // building the user agent may block on some platforms.
   args.post_factory->Init(params.sync_user_agent);
   registrar_->GetWorkers(&args.workers);
+  args.encryption_observer_proxy = std::move(params.encryption_observer_proxy);
   args.extensions_activity = params.extensions_activity.get();
   args.change_delegate = registrar_.get();  // as SyncManager::ChangeDelegate
   args.authenticated_account_id = params.authenticated_account_id;

components/sync/driver/glue/sync_engine_backend.h

@@ -206,9 +206,6 @@ class SyncEngineBackend : public base::RefCountedThreadSafe<SyncEngineBackend>,
   // Non-null only between calls to DoInitialize() and DoShutdown().
   std::unique_ptr<SyncBackendRegistrar> registrar_;
 
-  // Non-null only between calls to DoInitialize() and DoShutdown().
-  std::unique_ptr<SyncEncryptionHandler::Observer> encryption_observer_proxy_;
-
   // The timer used to periodically call SaveChanges.
   std::unique_ptr<base::RepeatingTimer> save_changes_timer_;
 

components/sync/driver/glue/sync_engine_impl_unittest.cc

@@ -51,10 +51,6 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
-using ::testing::_;
-using ::testing::InvokeWithoutArgs;
-using ::testing::StrictMock;
-
 namespace syncer {
 
 namespace {
@@ -142,23 +138,6 @@ class FakeSyncManagerFactory : public SyncManagerFactory {
   FakeSyncManager** fake_manager_;
 };
 
-class NullEncryptionObserver : public SyncEncryptionHandler::Observer {
- public:
-  void OnPassphraseRequired(
-      PassphraseRequiredReason reason,
-      const KeyDerivationParams& key_derivation_params,
-      const sync_pb::EncryptedData& pending_keys) override {}
-  void OnPassphraseAccepted() override {}
-  void OnBootstrapTokenUpdated(const std::string& bootstrap_token,
-                               BootstrapTokenType type) override {}
-  void OnEncryptedTypesChanged(ModelTypeSet encrypted_types,
-                               bool encrypt_everything) override {}
-  void OnEncryptionComplete() override {}
-  void OnCryptographerStateChanged(Cryptographer* cryptographer) override {}
-  void OnPassphraseTypeChanged(PassphraseType type,
-                               base::Time passphrase_time) override {}
-};
-
 class MockInvalidationService : public invalidation::InvalidationService {
  public:
   MockInvalidationService() = default;
@@ -246,8 +225,6 @@ class SyncEngineImplTest : public testing::Test {
     params.host = &host_;
     params.registrar = std::make_unique<SyncBackendRegistrar>(
         std::string(), base::Bind(&CreateModelWorkerForGroup));
-    params.encryption_observer_proxy =
-        std::make_unique<NullEncryptionObserver>();
     params.http_factory_getter = std::move(http_post_provider_factory_getter);
     params.authenticated_account_id = "user@example.com";
     params.sync_manager_factory = std::move(fake_manager_factory_);

components/sync/engine/fake_sync_manager.cc

@@ -15,6 +15,7 @@
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "components/sync/base/weak_handle.h"
 #include "components/sync/engine/engine_components_factory.h"
+#include "components/sync/engine/fake_model_type_connector.h"
 #include "components/sync/engine/net/http_post_provider_factory.h"
 #include "components/sync/syncable/directory.h"
 
@@ -197,10 +198,6 @@ UserShare* FakeSyncManager::GetUserShare() {
   return test_user_share_.user_share();
 }
 
-ModelTypeConnector* FakeSyncManager::GetModelTypeConnector() {
-  return &fake_model_type_connector_;
-}
-
 std::unique_ptr<ModelTypeConnector>
 FakeSyncManager::GetModelTypeConnectorProxy() {
   return std::make_unique<FakeModelTypeConnector>();

components/sync/engine/fake_sync_manager.h

@@ -13,7 +13,6 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/observer_list.h"
-#include "components/sync/engine/fake_model_type_connector.h"
 #include "components/sync/engine/sync_manager.h"
 #include "components/sync/syncable/test_user_share.h"
 #include "components/sync/test/fake_sync_encryption_handler.h"
@@ -100,7 +99,6 @@ class FakeSyncManager : public SyncManager {
   void SaveChanges() override;
   void ShutdownOnSyncThread() override;
   UserShare* GetUserShare() override;
-  ModelTypeConnector* GetModelTypeConnector() override;
   std::unique_ptr<ModelTypeConnector> GetModelTypeConnectorProxy() override;
   std::string cache_guid() override;
   std::string birthday() override;
@@ -149,8 +147,6 @@ class FakeSyncManager : public SyncManager {
   // The most recent configure reason.
   ConfigureReason last_configure_reason_;
 
-  FakeModelTypeConnector fake_model_type_connector_;
-
   FakeSyncEncryptionHandler fake_encryption_handler_;
 
   TestUserShare test_user_share_;

components/sync/engine/sync_manager.h

@@ -53,7 +53,6 @@ class JsEventHandler;
 class ModelTypeControllerDelegate;
 class ProtocolEvent;
 class SyncCycleSnapshot;
-class SyncEncryptionHandler;
 class TypeDebugInfoObserver;
 class UnrecoverableErrorHandler;
 struct UserShare;
@@ -215,6 +214,8 @@ class SyncManager {
 
     std::vector<scoped_refptr<ModelSafeWorker>> workers;
 
+    std::unique_ptr<SyncEncryptionHandler::Observer> encryption_observer_proxy;
+
     // Must outlive SyncManager.
     ExtensionsActivity* extensions_activity;
 
@@ -341,11 +342,6 @@ class SyncManager {
   // May be called from any thread.
   virtual UserShare* GetUserShare() = 0;
 
-  // Returns non-owning pointer to ModelTypeConnector. In contrast with
-  // ModelTypeConnectorProxy all calls are executed synchronously, thus the
-  // pointer should be used on sync thread.
-  virtual ModelTypeConnector* GetModelTypeConnector() = 0;
-
   // Returns an instance of the main interface for registering sync types with
   // sync engine.
   virtual std::unique_ptr<ModelTypeConnector> GetModelTypeConnectorProxy() = 0;

components/sync/engine_impl/sync_manager_impl.cc

@@ -283,6 +283,9 @@ void SyncManagerImpl::Init(InitArgs* args) {
 
   change_delegate_ = args->change_delegate;
 
+  DCHECK(args->encryption_observer_proxy);
+  encryption_observer_proxy_ = std::move(args->encryption_observer_proxy);
+
   AddObserver(&js_sync_manager_observer_);
   SetJsEventHandler(args->event_handler);
 
@@ -320,7 +323,13 @@ void SyncManagerImpl::Init(InitArgs* args) {
     keystore_keys_handler = sync_encryption_handler_impl.get();
     sync_encryption_handler_ = std::move(sync_encryption_handler_impl);
   }
+
+  // Register for encryption related changes now. We have to do this before
+  // the initial download of control types or initializing the encryption
+  // handler in order to receive notifications triggered during encryption
+  // startup.
   sync_encryption_handler_->AddObserver(this);
+  sync_encryption_handler_->AddObserver(encryption_observer_proxy_.get());
   sync_encryption_handler_->AddObserver(&debug_info_event_listener_);
   sync_encryption_handler_->AddObserver(&js_sync_encryption_handler_observer_);
 
@@ -417,6 +426,12 @@ void SyncManagerImpl::Init(InitArgs* args) {
     scheduler_->OnCredentialsUpdated();
   }
 
+  // Control types don't have DataTypeControllers, but they need to have
+  // update handlers registered in ModelTypeRegistry.
+  for (ModelType control_type : ControlTypes()) {
+    model_type_registry_->RegisterDirectoryType(control_type, GROUP_PASSIVE);
+  }
+
   NotifyInitializationSuccess();
 }
 
@@ -1001,11 +1016,6 @@ UserShare* SyncManagerImpl::GetUserShare() {
   return &share_;
 }
 
-ModelTypeConnector* SyncManagerImpl::GetModelTypeConnector() {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  return model_type_registry_.get();
-}
-
 std::unique_ptr<ModelTypeConnector>
 SyncManagerImpl::GetModelTypeConnectorProxy() {
   DCHECK(initialized_);

components/sync/engine_impl/sync_manager_impl.h

@@ -92,7 +92,6 @@ class SyncManagerImpl
   void SaveChanges() override;
   void ShutdownOnSyncThread() override;
   UserShare* GetUserShare() override;
-  ModelTypeConnector* GetModelTypeConnector() override;
   std::unique_ptr<ModelTypeConnector> GetModelTypeConnectorProxy() override;
   std::string cache_guid() override;
   std::string birthday() override;
@@ -325,6 +324,8 @@ class SyncManagerImpl
   // Initialized iff USS implementation of Nigori is enabled.
   base::WeakPtr<ModelTypeControllerDelegate> nigori_controller_delegate_;
 
+  std::unique_ptr<SyncEncryptionHandler::Observer> encryption_observer_proxy_;
+
   base::WeakPtrFactory<SyncManagerImpl> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(SyncManagerImpl);

components/sync/engine_impl/sync_manager_impl_unittest.cc

@@ -948,12 +948,16 @@ class SyncManagerTest : public testing::Test,
     scoped_refptr<ModelSafeWorker> worker = new FakeModelWorker(GROUP_PASSIVE);
     workers.push_back(worker);
 
+    auto encryption_observer =
+        std::make_unique<StrictMock<SyncEncryptionHandlerObserverMock>>();
+    encryption_observer_ = encryption_observer.get();
+
     SyncManager::InitArgs args;
     args.database_location = temp_dir_.GetPath();
     args.service_url = GURL("https://example.com/");
-    args.post_factory = std::unique_ptr<HttpPostProviderFactory>(
-        new TestHttpPostProviderFactory());
+    args.post_factory = std::make_unique<TestHttpPostProviderFactory>();
     args.workers = workers;
+    args.encryption_observer_proxy = std::move(encryption_observer);
     args.extensions_activity = extensions_activity_.get(),
     args.change_delegate = this;
     if (!enable_local_sync_backend)
@@ -970,8 +974,6 @@ class SyncManagerTest : public testing::Test,
     args.poll_interval = base::TimeDelta::FromMinutes(60);
     sync_manager_.Init(&args);
 
-    sync_manager_.GetEncryptionHandler()->AddObserver(&encryption_observer_);
-
     EXPECT_TRUE(js_backend_.IsInitialized());
     EXPECT_EQ(EngineComponentsFactory::STORAGE_ON_DISK, storage_used_);
 
@@ -1134,13 +1136,13 @@ class SyncManagerTest : public testing::Test,
   EngineComponentsFactory::Switches GetSwitches() const { return switches_; }
 
   void ExpectPassphraseAcceptance() {
-    EXPECT_CALL(encryption_observer_, OnPassphraseAccepted());
-    EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
-    EXPECT_CALL(encryption_observer_, OnCryptographerStateChanged(_));
+    EXPECT_CALL(*encryption_observer_, OnPassphraseAccepted());
+    EXPECT_CALL(*encryption_observer_, OnEncryptionComplete());
+    EXPECT_CALL(*encryption_observer_, OnCryptographerStateChanged(_));
   }
 
   void SetCustomPassphraseAndCheck(const std::string& passphrase) {
-    EXPECT_CALL(encryption_observer_,
+    EXPECT_CALL(*encryption_observer_,
                 OnPassphraseTypeChanged(PassphraseType::CUSTOM_PASSPHRASE, _));
     sync_manager_.GetEncryptionHandler()->SetEncryptionPassphrase(passphrase);
     EXPECT_EQ(PassphraseType::CUSTOM_PASSPHRASE, GetPassphraseType());
@@ -1166,7 +1168,8 @@ class SyncManagerTest : public testing::Test,
   WeakHandle<JsBackend> js_backend_;
   bool initialization_succeeded_;
   StrictMock<SyncManagerObserverMock> manager_observer_;
-  StrictMock<SyncEncryptionHandlerObserverMock> encryption_observer_;
+  // Owned by |sync_manager_|.
+  StrictMock<SyncEncryptionHandlerObserverMock>* encryption_observer_;
   EngineComponentsFactory::Switches switches_;
   EngineComponentsFactory::StorageOption storage_used_;
   MockUnrecoverableErrorHandler mock_unrecoverable_error_handler_;
@@ -1174,9 +1177,9 @@ class SyncManagerTest : public testing::Test,
 
 TEST_F(SyncManagerTest, RefreshEncryptionReady) {
   EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, DEFAULT_ENCRYPTION));
-  EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
-  EXPECT_CALL(encryption_observer_, OnCryptographerStateChanged(_));
-  EXPECT_CALL(encryption_observer_, OnEncryptedTypesChanged(_, false));
+  EXPECT_CALL(*encryption_observer_, OnEncryptionComplete());
+  EXPECT_CALL(*encryption_observer_, OnCryptographerStateChanged(_));
+  EXPECT_CALL(*encryption_observer_, OnEncryptedTypesChanged(_, false));
 
   sync_manager_.GetEncryptionHandler()->Init();
   PumpLoop();
@@ -1203,9 +1206,9 @@ TEST_F(SyncManagerTest, RefreshEncryptionNotReady) {
 
   // Should fail. Triggers an OnPassphraseRequired because the cryptographer
   // is not ready.
-  EXPECT_CALL(encryption_observer_, OnPassphraseRequired(_, _, _)).Times(1);
-  EXPECT_CALL(encryption_observer_, OnCryptographerStateChanged(_));
-  EXPECT_CALL(encryption_observer_, OnEncryptedTypesChanged(_, false));
+  EXPECT_CALL(*encryption_observer_, OnPassphraseRequired(_, _, _)).Times(1);
+  EXPECT_CALL(*encryption_observer_, OnCryptographerStateChanged(_));
+  EXPECT_CALL(*encryption_observer_, OnEncryptedTypesChanged(_, false));
   sync_manager_.GetEncryptionHandler()->Init();
   PumpLoop();
 
@@ -1217,9 +1220,9 @@ TEST_F(SyncManagerTest, RefreshEncryptionNotReady) {
 // Attempt to refresh encryption when nigori is empty.
 TEST_F(SyncManagerTest, RefreshEncryptionEmptyNigori) {
   EXPECT_TRUE(SetUpEncryption(DONT_WRITE_NIGORI, DEFAULT_ENCRYPTION));
-  EXPECT_CALL(encryption_observer_, OnEncryptionComplete()).Times(1);
-  EXPECT_CALL(encryption_observer_, OnCryptographerStateChanged(_));
-  EXPECT_CALL(encryption_observer_, OnEncryptedTypesChanged(_, false));
+  EXPECT_CALL(*encryption_observer_, OnEncryptionComplete()).Times(1);
+  EXPECT_CALL(*encryption_observer_, OnCryptographerStateChanged(_));
+  EXPECT_CALL(*encryption_observer_, OnEncryptedTypesChanged(_, false));
 
   // Should write to nigori.
   sync_manager_.GetEncryptionHandler()->Init();
@@ -1244,9 +1247,9 @@ TEST_F(SyncManagerTest, RefreshEncryptionEmptyNigori) {
 TEST_F(SyncManagerTest, EncryptDataTypesWithNoData) {
   EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, DEFAULT_ENCRYPTION));
   EXPECT_CALL(
-      encryption_observer_,
+      *encryption_observer_,
       OnEncryptedTypesChanged(HasModelTypes(EncryptableUserTypes()), true));
-  EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
+  EXPECT_CALL(*encryption_observer_, OnEncryptionComplete());
   sync_manager_.GetEncryptionHandler()->EnableEncryptEverything();
   EXPECT_TRUE(IsEncryptEverythingEnabledForTest());
 }
@@ -1287,9 +1290,9 @@ TEST_F(SyncManagerTest, EncryptDataTypesWithData) {
   }
 
   EXPECT_CALL(
-      encryption_observer_,
+      *encryption_observer_,
       OnEncryptedTypesChanged(HasModelTypes(EncryptableUserTypes()), true));
-  EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
+  EXPECT_CALL(*encryption_observer_, OnEncryptionComplete());
   sync_manager_.GetEncryptionHandler()->EnableEncryptEverything();
   EXPECT_TRUE(IsEncryptEverythingEnabledForTest());
   {
@@ -1305,7 +1308,7 @@ TEST_F(SyncManagerTest, EncryptDataTypesWithData) {
 
   // Trigger's a ReEncryptEverything with new passphrase.
   testing::Mock::VerifyAndClearExpectations(&encryption_observer_);
-  EXPECT_CALL(encryption_observer_,
+  EXPECT_CALL(*encryption_observer_,
               OnBootstrapTokenUpdated(_, PASSPHRASE_BOOTSTRAP_TOKEN));
   ExpectPassphraseAcceptance();
   SetCustomPassphraseAndCheck("new_passphrase");
@@ -1323,11 +1326,11 @@ TEST_F(SyncManagerTest, EncryptDataTypesWithData) {
   // Calling EncryptDataTypes with an empty encrypted types should not trigger
   // a reencryption and should just notify immediately.
   testing::Mock::VerifyAndClearExpectations(&encryption_observer_);
-  EXPECT_CALL(encryption_observer_,
+  EXPECT_CALL(*encryption_observer_,
               OnBootstrapTokenUpdated(_, PASSPHRASE_BOOTSTRAP_TOKEN))
       .Times(0);
-  EXPECT_CALL(encryption_observer_, OnPassphraseAccepted()).Times(0);
-  EXPECT_CALL(encryption_observer_, OnEncryptionComplete()).Times(0);
+  EXPECT_CALL(*encryption_observer_, OnPassphraseAccepted()).Times(0);
+  EXPECT_CALL(*encryption_observer_, OnEncryptionComplete()).Times(0);
   sync_manager_.GetEncryptionHandler()->EnableEncryptEverything();
 }
 
@@ -1356,7 +1359,7 @@ TEST_F(SyncManagerTest, SetPassphraseWithPassword) {
     data.set_password_value("secret");
     password_node.SetPasswordSpecifics(data);
   }
-  EXPECT_CALL(encryption_observer_,
+  EXPECT_CALL(*encryption_observer_,
               OnBootstrapTokenUpdated(_, PASSPHRASE_BOOTSTRAP_TOKEN));
   ExpectPassphraseAcceptance();
   SetCustomPassphraseAndCheck("new_passphrase");
@@ -1405,7 +1408,7 @@ TEST_F(SyncManagerTest, SupplyPendingGAIAPass) {
     EXPECT_TRUE(cryptographer->has_pending_keys());
     node.SetNigoriSpecifics(nigori);
   }
-  EXPECT_CALL(encryption_observer_,
+  EXPECT_CALL(*encryption_observer_,
               OnBootstrapTokenUpdated(_, PASSPHRASE_BOOTSTRAP_TOKEN));
   ExpectPassphraseAcceptance();
   sync_manager_.GetEncryptionHandler()->SetDecryptionPassphrase("passphrase2");
@@ -1449,13 +1452,13 @@ TEST_F(SyncManagerTest, SupplyPendingExplicitPass) {
     nigori.set_keybag_is_frozen(true);
     node.SetNigoriSpecifics(nigori);
   }
-  EXPECT_CALL(encryption_observer_, OnCryptographerStateChanged(_));
-  EXPECT_CALL(encryption_observer_,
+  EXPECT_CALL(*encryption_observer_, OnCryptographerStateChanged(_));
+  EXPECT_CALL(*encryption_observer_,
               OnPassphraseTypeChanged(PassphraseType::CUSTOM_PASSPHRASE, _));
-  EXPECT_CALL(encryption_observer_, OnPassphraseRequired(_, _, _));
-  EXPECT_CALL(encryption_observer_, OnEncryptedTypesChanged(_, false));
+  EXPECT_CALL(*encryption_observer_, OnPassphraseRequired(_, _, _));
+  EXPECT_CALL(*encryption_observer_, OnEncryptedTypesChanged(_, false));
   sync_manager_.GetEncryptionHandler()->Init();
-  EXPECT_CALL(encryption_observer_,
+  EXPECT_CALL(*encryption_observer_,
               OnBootstrapTokenUpdated(_, PASSPHRASE_BOOTSTRAP_TOKEN));
   ExpectPassphraseAcceptance();
   sync_manager_.GetEncryptionHandler()->SetDecryptionPassphrase("explicit");
@@ -1487,7 +1490,7 @@ TEST_F(SyncManagerTest, SetPassphraseWithEmptyPasswordNode) {
     EXPECT_EQ(WriteNode::INIT_SUCCESS, result);
     node_id = password_node.GetId();
   }
-  EXPECT_CALL(encryption_observer_,
+  EXPECT_CALL(*encryption_observer_,
               OnBootstrapTokenUpdated(_, PASSPHRASE_BOOTSTRAP_TOKEN));
   ExpectPassphraseAcceptance();
   SetCustomPassphraseAndCheck("new_passphrase");
@@ -1573,9 +1576,9 @@ TEST_F(SyncManagerTest, EncryptBookmarksWithLegacyData) {
   }
 
   EXPECT_CALL(
-      encryption_observer_,
+      *encryption_observer_,
       OnEncryptedTypesChanged(HasModelTypes(EncryptableUserTypes()), true));
-  EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
+  EXPECT_CALL(*encryption_observer_, OnEncryptionComplete());
   sync_manager_.GetEncryptionHandler()->EnableEncryptEverything();
   EXPECT_TRUE(IsEncryptEverythingEnabledForTest());
 
@@ -1659,13 +1662,13 @@ TEST_F(SyncManagerTest, UpdateEntryWithEncryption) {
 
   // Encrypt the datatatype, should set is_unsynced.
   EXPECT_CALL(
-      encryption_observer_,
+      *encryption_observer_,
       OnEncryptedTypesChanged(HasModelTypes(EncryptableUserTypes()), true));
-  EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
+  EXPECT_CALL(*encryption_observer_, OnEncryptionComplete());
   EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, FULL_ENCRYPTION));
 
-  EXPECT_CALL(encryption_observer_, OnCryptographerStateChanged(_));
-  EXPECT_CALL(encryption_observer_, OnEncryptedTypesChanged(_, true));
+  EXPECT_CALL(*encryption_observer_, OnCryptographerStateChanged(_));
+  EXPECT_CALL(*encryption_observer_, OnEncryptedTypesChanged(_, true));
   sync_manager_.GetEncryptionHandler()->Init();
   PumpLoop();
   {
@@ -1686,7 +1689,7 @@ TEST_F(SyncManagerTest, UpdateEntryWithEncryption) {
 
   // Set a new passphrase. Should set is_unsynced.
   testing::Mock::VerifyAndClearExpectations(&encryption_observer_);
-  EXPECT_CALL(encryption_observer_,
+  EXPECT_CALL(*encryption_observer_,
               OnBootstrapTokenUpdated(_, PASSPHRASE_BOOTSTRAP_TOKEN));
   ExpectPassphraseAcceptance();
   SetCustomPassphraseAndCheck("new_passphrase");
@@ -1708,9 +1711,9 @@ TEST_F(SyncManagerTest, UpdateEntryWithEncryption) {
 
   // Force a re-encrypt everything. Should not set is_unsynced.
   testing::Mock::VerifyAndClearExpectations(&encryption_observer_);
-  EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
-  EXPECT_CALL(encryption_observer_, OnCryptographerStateChanged(_));
-  EXPECT_CALL(encryption_observer_, OnEncryptedTypesChanged(_, true));
+  EXPECT_CALL(*encryption_observer_, OnEncryptionComplete());
+  EXPECT_CALL(*encryption_observer_, OnCryptographerStateChanged(_));
+  EXPECT_CALL(*encryption_observer_, OnEncryptedTypesChanged(_, true));
 
   sync_manager_.GetEncryptionHandler()->Init();
   PumpLoop();
@@ -1870,7 +1873,7 @@ TEST_F(SyncManagerTest, UpdatePasswordNewPassphrase) {
 
   // Set a new passphrase. Should set is_unsynced.
   testing::Mock::VerifyAndClearExpectations(&encryption_observer_);
-  EXPECT_CALL(encryption_observer_,
+  EXPECT_CALL(*encryption_observer_,
               OnBootstrapTokenUpdated(_, PASSPHRASE_BOOTSTRAP_TOKEN));
   ExpectPassphraseAcceptance();
   SetCustomPassphraseAndCheck("new_passphrase");
@@ -1942,9 +1945,9 @@ TEST_F(SyncManagerTest, UpdatePasswordReencryptEverything) {
 
   // Force a re-encrypt everything. Should not set is_unsynced.
   testing::Mock::VerifyAndClearExpectations(&encryption_observer_);
-  EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
-  EXPECT_CALL(encryption_observer_, OnCryptographerStateChanged(_));
-  EXPECT_CALL(encryption_observer_, OnEncryptedTypesChanged(_, false));
+  EXPECT_CALL(*encryption_observer_, OnEncryptionComplete());
+  EXPECT_CALL(*encryption_observer_, OnCryptographerStateChanged(_));
+  EXPECT_CALL(*encryption_observer_, OnEncryptedTypesChanged(_, false));
   sync_manager_.GetEncryptionHandler()->Init();
   PumpLoop();
   EXPECT_FALSE(ResetUnsyncedEntry(PASSWORDS, kClientTag));
@@ -1974,9 +1977,9 @@ TEST_F(SyncManagerTest, UpdatePasswordReencryptEverythingFillMetadata) {
 
   // Force a re-encrypt everything. Should set is_unsynced.
   testing::Mock::VerifyAndClearExpectations(&encryption_observer_);
-  EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
-  EXPECT_CALL(encryption_observer_, OnCryptographerStateChanged(_));
-  EXPECT_CALL(encryption_observer_, OnEncryptedTypesChanged(_, false));
+  EXPECT_CALL(*encryption_observer_, OnEncryptionComplete());
+  EXPECT_CALL(*encryption_observer_, OnCryptographerStateChanged(_));
+  EXPECT_CALL(*encryption_observer_, OnEncryptedTypesChanged(_, false));
   sync_manager_.GetEncryptionHandler()->Init();
   PumpLoop();
   // Check that unencrypted metadata field was set.
@@ -2022,9 +2025,9 @@ TEST_F(SyncManagerTest,
 
   // Force a re-encrypt everything. Should not set is_unsynced.
   testing::Mock::VerifyAndClearExpectations(&encryption_observer_);
-  EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
-  EXPECT_CALL(encryption_observer_, OnCryptographerStateChanged(_));
-  EXPECT_CALL(encryption_observer_, OnEncryptedTypesChanged(_, false));
+  EXPECT_CALL(*encryption_observer_, OnEncryptionComplete());
+  EXPECT_CALL(*encryption_observer_, OnCryptographerStateChanged(_));
+  EXPECT_CALL(*encryption_observer_, OnEncryptedTypesChanged(_, false));
   sync_manager_.GetEncryptionHandler()->Init();
   PumpLoop();
   EXPECT_FALSE(ResetUnsyncedEntry(PASSWORDS, kClientTag));
@@ -2062,9 +2065,9 @@ TEST_F(SyncManagerTest, ReencryptEverythingWithUnrecoverableErrorPasswords) {
   // 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_CALL(*encryption_observer_, OnEncryptionComplete());
+  EXPECT_CALL(*encryption_observer_, OnCryptographerStateChanged(_));
+  EXPECT_CALL(*encryption_observer_, OnEncryptedTypesChanged(_, false));
   EXPECT_FALSE(HasUnrecoverableError());
   sync_manager_.GetEncryptionHandler()->Init();
   PumpLoop();
@@ -2076,7 +2079,7 @@ TEST_F(SyncManagerTest, ReencryptEverythingWithUnrecoverableErrorPasswords) {
 TEST_F(SyncManagerTest, ReencryptEverythingWithUnrecoverableErrorBookmarks) {
   const char kClientTag[] = "client_tag";
   EXPECT_CALL(
-      encryption_observer_,
+      *encryption_observer_,
       OnEncryptedTypesChanged(HasModelTypes(EncryptableUserTypes()), true));
   EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, FULL_ENCRYPTION));
   sync_pb::EntitySpecifics entity_specifics;
@@ -2107,9 +2110,9 @@ TEST_F(SyncManagerTest, ReencryptEverythingWithUnrecoverableErrorBookmarks) {
   // 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_CALL(*encryption_observer_, OnEncryptionComplete());
+  EXPECT_CALL(*encryption_observer_, OnCryptographerStateChanged(_));
+  EXPECT_CALL(*encryption_observer_, OnEncryptedTypesChanged(_, true));
   EXPECT_FALSE(HasUnrecoverableError());
   sync_manager_.GetEncryptionHandler()->Init();
   PumpLoop();
@@ -2164,12 +2167,12 @@ TEST_F(SyncManagerTest, SetBookmarkTitleWithEncryption) {
 
   // Encrypt the datatatype, should set is_unsynced.
   EXPECT_CALL(
-      encryption_observer_,
+      *encryption_observer_,
       OnEncryptedTypesChanged(HasModelTypes(EncryptableUserTypes()), true));
-  EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
+  EXPECT_CALL(*encryption_observer_, OnEncryptionComplete());
   EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, FULL_ENCRYPTION));
-  EXPECT_CALL(encryption_observer_, OnCryptographerStateChanged(_));
-  EXPECT_CALL(encryption_observer_, OnEncryptedTypesChanged(_, true));
+  EXPECT_CALL(*encryption_observer_, OnCryptographerStateChanged(_));
+  EXPECT_CALL(*encryption_observer_, OnEncryptedTypesChanged(_, true));
   sync_manager_.GetEncryptionHandler()->Init();
   PumpLoop();
   EXPECT_TRUE(ResetUnsyncedEntry(BOOKMARKS, client_tag));
@@ -2255,12 +2258,12 @@ TEST_F(SyncManagerTest, SetNonBookmarkTitleWithEncryption) {
 
   // Encrypt the datatatype, should set is_unsynced.
   EXPECT_CALL(
-      encryption_observer_,
+      *encryption_observer_,
       OnEncryptedTypesChanged(HasModelTypes(EncryptableUserTypes()), true));
-  EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
+  EXPECT_CALL(*encryption_observer_, OnEncryptionComplete());
   EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, FULL_ENCRYPTION));
-  EXPECT_CALL(encryption_observer_, OnCryptographerStateChanged(_));
-  EXPECT_CALL(encryption_observer_, OnEncryptedTypesChanged(_, true));
+  EXPECT_CALL(*encryption_observer_, OnCryptographerStateChanged(_));
+  EXPECT_CALL(*encryption_observer_, OnEncryptedTypesChanged(_, true));
   sync_manager_.GetEncryptionHandler()->Init();
   PumpLoop();
   EXPECT_TRUE(ResetUnsyncedEntry(PREFERENCES, client_tag));
@@ -2448,9 +2451,9 @@ class SyncManagerWithLocalBackendTest : public SyncManagerTest {
 // the local backend case.
 TEST_F(SyncManagerWithLocalBackendTest, StartSyncInLocalMode) {
   EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, DEFAULT_ENCRYPTION));
-  EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
-  EXPECT_CALL(encryption_observer_, OnCryptographerStateChanged(_));
-  EXPECT_CALL(encryption_observer_, OnEncryptedTypesChanged(_, false));
+  EXPECT_CALL(*encryption_observer_, OnEncryptionComplete());
+  EXPECT_CALL(*encryption_observer_, OnCryptographerStateChanged(_));
+  EXPECT_CALL(*encryption_observer_, OnEncryptedTypesChanged(_, false));
 
   sync_manager_.GetEncryptionHandler()->Init();
   PumpLoop();