Lock-free shutdown of profile sync service. Changes include:

* Disconnect non-frontend processor/associator to stop accessing directory
so that sync backend can be shut down without waiting.
* Change non-frontend controller so that creation/destruction of 
processor/associator doesn't depend on valid controller. So
scoped wait on stopping controller can be removed.
* Move sync thread to PSS. It's created when starting first backend and 
destroyed on last browser thread.

BUG=19757

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=210333

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=210955

Review URL: https://chromiumcodereview.appspot.com/16770005

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

16 files changed, 100 insertions, 58 deletions

diff --git a/sync/internal_api/internal_components_factory_impl.cc b/sync/internal_api/internal_components_factory_impl.cc
index 1cc3b11..d5fc102 100644
--- a/sync/internal_api/internal_components_factory_impl.cc
+++ b/sync/internal_api/internal_components_factory_impl.cc
@@ -37,14 +37,14 @@ InternalComponentsFactoryImpl::BuildContext(
     ServerConnectionManager* connection_manager,
     syncable::Directory* directory,
     const std::vector<ModelSafeWorker*>& workers,
-    ExtensionsActivityMonitor* monitor,
+    ExtensionsActivity* extensions_activity,
     const std::vector<SyncEngineEventListener*>& listeners,
     sessions::DebugInfoGetter* debug_info_getter,
     TrafficRecorder* traffic_recorder,
     const std::string& invalidation_client_id) {
   return scoped_ptr<sessions::SyncSessionContext>(
       new sessions::SyncSessionContext(
-          connection_manager, directory, workers, monitor,
+          connection_manager, directory, workers, extensions_activity,
           listeners, debug_info_getter,
           traffic_recorder,
           switches_.encryption_method == ENCRYPTION_KEYSTORE,
diff --git a/sync/internal_api/public/engine/model_safe_worker.cc b/sync/internal_api/public/engine/model_safe_worker.cc
index 44e0a24..7179ed5 100644
--- a/sync/internal_api/public/engine/model_safe_worker.cc
+++ b/sync/internal_api/public/engine/model_safe_worker.cc
@@ -4,6 +4,7 @@
 
 #include "sync/internal_api/public/engine/model_safe_worker.h"
 
+#include "base/bind.h"
 #include "base/json/json_writer.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/values.h"
@@ -84,7 +85,9 @@ std::string ModelSafeGroupToString(ModelSafeGroup group) {
 ModelSafeWorker::ModelSafeWorker(WorkerLoopDestructionObserver* observer)
     : stopped_(false),
       work_done_or_stopped_(false, false),
-      observer_(observer) {}
+      observer_(observer),
+      working_loop_(NULL),
+      working_loop_set_wait_(true, false) {}
 
 ModelSafeWorker::~ModelSafeWorker() {}
 
@@ -135,4 +138,33 @@ void ModelSafeWorker::WillDestroyCurrentMessageLoop() {
     observer_->OnWorkerLoopDestroyed(GetModelSafeGroup());
 }
 
+void ModelSafeWorker::SetWorkingLoopToCurrent() {
+  DCHECK(!working_loop_);
+  working_loop_ = base::MessageLoop::current();
+  working_loop_set_wait_.Signal();
+}
+
+void ModelSafeWorker::UnregisterForLoopDestruction(
+    base::Callback<void(ModelSafeGroup)> unregister_done_callback) {
+  // Ok to wait until |working_loop_| is set because this is called on sync
+  // loop.
+  working_loop_set_wait_.Wait();
+
+  // Should be called on sync loop.
+  DCHECK_NE(base::MessageLoop::current(), working_loop_);
+  DCHECK(working_loop_);
+  working_loop_->PostTask(
+      FROM_HERE,
+      base::Bind(&ModelSafeWorker::UnregisterForLoopDestructionAsync,
+                 this, unregister_done_callback));
+}
+
+void ModelSafeWorker::UnregisterForLoopDestructionAsync(
+    base::Callback<void(ModelSafeGroup)> unregister_done_callback) {
+  DCHECK(stopped_);
+  DCHECK_EQ(base::MessageLoop::current(), working_loop_);
+  base::MessageLoop::current()->RemoveDestructionObserver(this);
+  unregister_done_callback.Run(GetModelSafeGroup());
+}
+
 }  // namespace syncer
diff --git a/sync/internal_api/public/engine/model_safe_worker.h b/sync/internal_api/public/engine/model_safe_worker.h
index 680c493..f6b7ea6 100644
--- a/sync/internal_api/public/engine/model_safe_worker.h
+++ b/sync/internal_api/public/engine/model_safe_worker.h
@@ -69,9 +69,16 @@ class SYNC_EXPORT ModelSafeWorker
       public base::MessageLoop::DestructionObserver {
  public:
   // Subclass should implement to observe destruction of the loop where
-  // it actually does work.
+  // it actually does work. Called on UI thread immediately after worker is
+  // created.
   virtual void RegisterForLoopDestruction() = 0;
 
+  // Called on sync loop from SyncBackendRegistrar::ShutDown(). Post task to
+  // working loop to stop observing loop destruction and invoke
+  // |unregister_done_callback|.
+  virtual void UnregisterForLoopDestruction(
+      base::Callback<void(ModelSafeGroup)> unregister_done_callback);
+
   // If not stopped, call DoWorkAndWaitUntilDoneImpl() to do work. Otherwise
   // return CANNOT_DO_WORK.
   SyncerError DoWorkAndWaitUntilDone(const WorkCallback& work);
@@ -103,7 +110,14 @@ class SYNC_EXPORT ModelSafeWorker
   // Return true if the worker was stopped. Thread safe.
   bool IsStopped();
 
+  // Subclass should call this in RegisterForLoopDestruction() from the loop
+  // where work is done.
+  void SetWorkingLoopToCurrent();
+
  private:
+  void UnregisterForLoopDestructionAsync(
+      base::Callback<void(ModelSafeGroup)> unregister_done_callback);
+
   // Whether the worker should/can do more work. Set when sync is disabled or
   // when the worker's working thread is to be destroyed.
   base::Lock stopped_lock_;
@@ -115,6 +129,11 @@ class SYNC_EXPORT ModelSafeWorker
 
   // Notified when working thread of the worker is to be destroyed.
   WorkerLoopDestructionObserver* observer_;
+
+  // Remember working loop for posting task to unregister destruction
+  // observation from sync thread when shutting down sync.
+  base::MessageLoop* working_loop_;
+  base::WaitableEvent working_loop_set_wait_;
 };
 
 // A map that details which ModelSafeGroup each ModelType
diff --git a/sync/internal_api/public/engine/passive_model_worker.cc b/sync/internal_api/public/engine/passive_model_worker.cc
index 9bd5a35..02036d5 100644
--- a/sync/internal_api/public/engine/passive_model_worker.cc
+++ b/sync/internal_api/public/engine/passive_model_worker.cc
@@ -18,7 +18,8 @@ PassiveModelWorker::~PassiveModelWorker() {
 }
 
 void PassiveModelWorker::RegisterForLoopDestruction() {
-  NOTREACHED();
+  base::MessageLoop::current()->AddDestructionObserver(this);
+  SetWorkingLoopToCurrent();
 }
 
 SyncerError PassiveModelWorker::DoWorkAndWaitUntilDoneImpl(
diff --git a/sync/internal_api/public/engine/passive_model_worker.h b/sync/internal_api/public/engine/passive_model_worker.h
index f834c83..783731b 100644
--- a/sync/internal_api/public/engine/passive_model_worker.h
+++ b/sync/internal_api/public/engine/passive_model_worker.h
@@ -11,10 +11,6 @@
 #include "sync/internal_api/public/engine/model_safe_worker.h"
 #include "sync/internal_api/public/util/syncer_error.h"
 
-namespace base {
-class MessageLoop;
-}
-
 namespace syncer {
 
 // Implementation of ModelSafeWorker for passive types.  All work is
diff --git a/sync/internal_api/public/internal_components_factory.h b/sync/internal_api/public/internal_components_factory.h
index 96fd640..86509ec 100644
--- a/sync/internal_api/public/internal_components_factory.h
+++ b/sync/internal_api/public/internal_components_factory.h
@@ -18,7 +18,7 @@
 
 namespace syncer {
 
-class ExtensionsActivityMonitor;
+class ExtensionsActivity;
 class ServerConnectionManager;
 class SyncEngineEventListener;
 class SyncScheduler;
@@ -81,7 +81,7 @@ class SYNC_EXPORT InternalComponentsFactory {
       ServerConnectionManager* connection_manager,
       syncable::Directory* directory,
       const std::vector<ModelSafeWorker*>& workers,
-      ExtensionsActivityMonitor* monitor,
+      ExtensionsActivity* extensions_activity,
       const std::vector<SyncEngineEventListener*>& listeners,
       sessions::DebugInfoGetter* debug_info_getter,
       TrafficRecorder* traffic_recorder,
diff --git a/sync/internal_api/public/internal_components_factory_impl.h b/sync/internal_api/public/internal_components_factory_impl.h
index 6fec27f..148dd07 100644
--- a/sync/internal_api/public/internal_components_factory_impl.h
+++ b/sync/internal_api/public/internal_components_factory_impl.h
@@ -27,7 +27,7 @@ class SYNC_EXPORT InternalComponentsFactoryImpl
       ServerConnectionManager* connection_manager,
       syncable::Directory* directory,
       const std::vector<ModelSafeWorker*>& workers,
-      ExtensionsActivityMonitor* monitor,
+      ExtensionsActivity* extensions_activity,
       const std::vector<SyncEngineEventListener*>& listeners,
       sessions::DebugInfoGetter* debug_info_getter,
       TrafficRecorder* traffic_recorder,
diff --git a/sync/internal_api/public/sync_manager.h b/sync/internal_api/public/sync_manager.h
index bc6a137..3f5316b 100644
--- a/sync/internal_api/public/sync_manager.h
+++ b/sync/internal_api/public/sync_manager.h
@@ -22,6 +22,7 @@
 #include "sync/internal_api/public/engine/sync_status.h"
 #include "sync/internal_api/public/sync_encryption_handler.h"
 #include "sync/internal_api/public/util/report_unrecoverable_error_function.h"
+#include "sync/internal_api/public/util/unrecoverable_error_handler.h"
 #include "sync/internal_api/public/util/weak_handle.h"
 #include "sync/notifier/invalidation_handler.h"
 #include "sync/protocol/sync_protocol_error.h"
@@ -36,14 +37,13 @@ class BaseTransaction;
 class DataTypeDebugInfoListener;
 class Encryptor;
 struct Experiments;
-class ExtensionsActivityMonitor;
+class ExtensionsActivity;
 class HttpPostProviderFactory;
 class InternalComponentsFactory;
 class JsBackend;
 class JsEventHandler;
 class SyncEncryptionHandler;
 class SyncScheduler;
-class UnrecoverableErrorHandler;
 struct UserShare;
 
 namespace sessions {
@@ -310,15 +310,15 @@ class SYNC_EXPORT SyncManager : public syncer::InvalidationHandler {
       bool use_ssl,
       scoped_ptr<HttpPostProviderFactory> post_factory,
       const std::vector<ModelSafeWorker*>& workers,
-      ExtensionsActivityMonitor* extensions_activity_monitor,
+      ExtensionsActivity* extensions_activity,
       ChangeDelegate* change_delegate,
       const SyncCredentials& credentials,
       const std::string& invalidator_client_id,
       const std::string& restored_key_for_bootstrapping,
       const std::string& restored_keystore_key_for_bootstrapping,
-      scoped_ptr<InternalComponentsFactory> internal_components_factory,
+      InternalComponentsFactory* internal_components_factory,
       Encryptor* encryptor,
-      UnrecoverableErrorHandler* unrecoverable_error_handler,
+      scoped_ptr<UnrecoverableErrorHandler> unrecoverable_error_handler,
       ReportUnrecoverableErrorFunction report_unrecoverable_error_function,
       bool use_oauth2_token) = 0;
 
@@ -401,7 +401,7 @@ class SYNC_EXPORT SyncManager : public syncer::InvalidationHandler {
   // If no scheduler exists, the callback is run immediately (from the loop
   // this was created on, which is the sync loop), as sync is effectively
   // stopped.
-  virtual void StopSyncingForShutdown(const base::Closure& callback) = 0;
+  virtual void StopSyncingForShutdown() = 0;
 
   // Issue a final SaveChanges, and close sqlite handles.
   virtual void ShutdownOnSyncThread() = 0;
diff --git a/sync/internal_api/public/test/fake_sync_manager.h b/sync/internal_api/public/test/fake_sync_manager.h
index ed69a52..be06ea5 100644
--- a/sync/internal_api/public/test/fake_sync_manager.h
+++ b/sync/internal_api/public/test/fake_sync_manager.h
@@ -82,17 +82,16 @@ class FakeSyncManager : public SyncManager {
       bool use_ssl,
       scoped_ptr<HttpPostProviderFactory> post_factory,
       const std::vector<ModelSafeWorker*>& workers,
-      ExtensionsActivityMonitor* extensions_activity_monitor,
+      ExtensionsActivity* extensions_activity,
       ChangeDelegate* change_delegate,
       const SyncCredentials& credentials,
       const std::string& invalidator_client_id,
       const std::string& restored_key_for_bootstrapping,
       const std::string& restored_keystore_key_for_bootstrapping,
-      scoped_ptr<InternalComponentsFactory> internal_components_factory,
+      InternalComponentsFactory* internal_components_factory,
       Encryptor* encryptor,
-      UnrecoverableErrorHandler* unrecoverable_error_handler,
-      ReportUnrecoverableErrorFunction
-          report_unrecoverable_error_function,
+      scoped_ptr<UnrecoverableErrorHandler> unrecoverable_error_handler,
+      ReportUnrecoverableErrorFunction report_unrecoverable_error_function,
       bool use_oauth2_token) OVERRIDE;
   virtual void ThrowUnrecoverableError() OVERRIDE;
   virtual ModelTypeSet InitialSyncEndedTypes() OVERRIDE;
@@ -115,7 +114,7 @@ class FakeSyncManager : public SyncManager {
   virtual void RemoveObserver(Observer* observer) OVERRIDE;
   virtual SyncStatus GetDetailedStatus() const OVERRIDE;
   virtual void SaveChanges() OVERRIDE;
-  virtual void StopSyncingForShutdown(const base::Closure& callback) OVERRIDE;
+  virtual void StopSyncingForShutdown() OVERRIDE;
   virtual void ShutdownOnSyncThread() OVERRIDE;
   virtual UserShare* GetUserShare() OVERRIDE;
   virtual const std::string cache_guid() OVERRIDE;
diff --git a/sync/internal_api/public/test/test_internal_components_factory.h b/sync/internal_api/public/test/test_internal_components_factory.h
index b608154..d846094 100644
--- a/sync/internal_api/public/test/test_internal_components_factory.h
+++ b/sync/internal_api/public/test/test_internal_components_factory.h
@@ -33,7 +33,7 @@ class TestInternalComponentsFactory : public InternalComponentsFactory {
       ServerConnectionManager* connection_manager,
       syncable::Directory* directory,
       const std::vector<ModelSafeWorker*>& workers,
-      ExtensionsActivityMonitor* monitor,
+      ExtensionsActivity* monitor,
       const std::vector<SyncEngineEventListener*>& listeners,
       sessions::DebugInfoGetter* debug_info_getter,
       TrafficRecorder* traffic_recorder,
diff --git a/sync/internal_api/public/util/unrecoverable_error_handler.h b/sync/internal_api/public/util/unrecoverable_error_handler.h
index db2f2c0..2bd2475 100644
--- a/sync/internal_api/public/util/unrecoverable_error_handler.h
+++ b/sync/internal_api/public/util/unrecoverable_error_handler.h
@@ -19,8 +19,7 @@ class UnrecoverableErrorHandler {
   // further, and will report an error status if queried.
   virtual void OnUnrecoverableError(const tracked_objects::Location& from_here,
                                     const std::string& message) = 0;
- protected:
-  virtual ~UnrecoverableErrorHandler() { }
+  virtual ~UnrecoverableErrorHandler() {}
 };
 
 }  // namespace syncer
diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc
index 3d6158d..a541032 100644
--- a/sync/internal_api/sync_manager_impl.cc
+++ b/sync/internal_api/sync_manager_impl.cc
@@ -175,7 +175,6 @@ SyncManagerImpl::SyncManagerImpl(const std::string& name)
       invalidator_state_(DEFAULT_INVALIDATION_ERROR),
       traffic_recorder_(kMaxMessagesToRecord, kMaxMessageSizeToRecord),
       encryptor_(NULL),
-      unrecoverable_error_handler_(NULL),
       report_unrecoverable_error_function_(NULL) {
   // Pre-fill |notification_info_map_|.
   for (int i = FIRST_REAL_MODEL_TYPE; i < MODEL_TYPE_COUNT; ++i) {
@@ -346,15 +345,15 @@ void SyncManagerImpl::Init(
     bool use_ssl,
     scoped_ptr<HttpPostProviderFactory> post_factory,
     const std::vector<ModelSafeWorker*>& workers,
-    ExtensionsActivityMonitor* extensions_activity_monitor,
+    ExtensionsActivity* extensions_activity,
     SyncManager::ChangeDelegate* change_delegate,
     const SyncCredentials& credentials,
     const std::string& invalidator_client_id,
     const std::string& restored_key_for_bootstrapping,
     const std::string& restored_keystore_key_for_bootstrapping,
-    scoped_ptr<InternalComponentsFactory> internal_components_factory,
+    InternalComponentsFactory* internal_components_factory,
     Encryptor* encryptor,
-    UnrecoverableErrorHandler* unrecoverable_error_handler,
+    scoped_ptr<UnrecoverableErrorHandler> unrecoverable_error_handler,
     ReportUnrecoverableErrorFunction report_unrecoverable_error_function,
     bool use_oauth2_token) {
   CHECK(!initialized_);
@@ -376,7 +375,7 @@ void SyncManagerImpl::Init(
   database_path_ = database_location.Append(
       syncable::Directory::kSyncDatabaseFilename);
   encryptor_ = encryptor;
-  unrecoverable_error_handler_ = unrecoverable_error_handler;
+  unrecoverable_error_handler_ = unrecoverable_error_handler.Pass();
   report_unrecoverable_error_function_ = report_unrecoverable_error_function;
 
   allstatus_.SetHasKeystoreKey(
@@ -402,7 +401,7 @@ void SyncManagerImpl::Init(
   share_.directory.reset(
       new syncable::Directory(
           backing_store.release(),
-          unrecoverable_error_handler_,
+          unrecoverable_error_handler_.get(),
           report_unrecoverable_error_function_,
           sync_encryption_handler_.get(),
           sync_encryption_handler_->GetCryptographerUnsafe()));
@@ -442,7 +441,7 @@ void SyncManagerImpl::Init(
       connection_manager_.get(),
       directory(),
       workers,
-      extensions_activity_monitor,
+      extensions_activity,
       listeners,
       &debug_info_event_listener_,
       &traffic_recorder_,
@@ -620,9 +619,9 @@ void SyncManagerImpl::RemoveObserver(SyncManager::Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
-void SyncManagerImpl::StopSyncingForShutdown(const base::Closure& callback) {
+void SyncManagerImpl::StopSyncingForShutdown() {
   DVLOG(2) << "StopSyncingForShutdown";
-  scheduler_->RequestStop(callback);
+  scheduler_->RequestStop();
   if (connection_manager_)
     connection_manager_->TerminateAllIO();
 }
diff --git a/sync/internal_api/sync_manager_impl.h b/sync/internal_api/sync_manager_impl.h
index 153df81..712c268 100644
--- a/sync/internal_api/sync_manager_impl.h
+++ b/sync/internal_api/sync_manager_impl.h
@@ -70,15 +70,15 @@ class SYNC_EXPORT_PRIVATE SyncManagerImpl :
       bool use_ssl,
       scoped_ptr<HttpPostProviderFactory> post_factory,
       const std::vector<ModelSafeWorker*>& workers,
-      ExtensionsActivityMonitor* extensions_activity_monitor,
+      ExtensionsActivity* extensions_activity,
       SyncManager::ChangeDelegate* change_delegate,
       const SyncCredentials& credentials,
       const std::string& invalidator_client_id,
       const std::string& restored_key_for_bootstrapping,
       const std::string& restored_keystore_key_for_bootstrapping,
-      scoped_ptr<InternalComponentsFactory> internal_components_factory,
+      InternalComponentsFactory* internal_components_factory,
       Encryptor* encryptor,
-      UnrecoverableErrorHandler* unrecoverable_error_handler,
+      scoped_ptr<UnrecoverableErrorHandler> unrecoverable_error_handler,
       ReportUnrecoverableErrorFunction
           report_unrecoverable_error_function,
       bool use_oauth2_token) OVERRIDE;
@@ -106,7 +106,7 @@ class SYNC_EXPORT_PRIVATE SyncManagerImpl :
   virtual void RemoveObserver(SyncManager::Observer* observer) OVERRIDE;
   virtual SyncStatus GetDetailedStatus() const OVERRIDE;
   virtual void SaveChanges() OVERRIDE;
-  virtual void StopSyncingForShutdown(const base::Closure& callback) OVERRIDE;
+  virtual void StopSyncingForShutdown() OVERRIDE;
   virtual void ShutdownOnSyncThread() OVERRIDE;
   virtual UserShare* GetUserShare() OVERRIDE;
   virtual const std::string cache_guid() OVERRIDE;
@@ -360,7 +360,7 @@ class SYNC_EXPORT_PRIVATE SyncManagerImpl :
   TrafficRecorder traffic_recorder_;
 
   Encryptor* encryptor_;
-  UnrecoverableErrorHandler* unrecoverable_error_handler_;
+  scoped_ptr<UnrecoverableErrorHandler> unrecoverable_error_handler_;
   ReportUnrecoverableErrorFunction report_unrecoverable_error_function_;
 
   // Sync's encryption handler. It tracks the set of encrypted types, manages
diff --git a/sync/internal_api/sync_manager_impl_unittest.cc b/sync/internal_api/sync_manager_impl_unittest.cc
index 51144b6..06bc783 100644
--- a/sync/internal_api/sync_manager_impl_unittest.cc
+++ b/sync/internal_api/sync_manager_impl_unittest.cc
@@ -68,9 +68,8 @@
 #include "sync/test/engine/fake_sync_scheduler.h"
 #include "sync/test/engine/test_id_factory.h"
 #include "sync/test/fake_encryptor.h"
-#include "sync/test/fake_extensions_activity_monitor.h"
 #include "sync/util/cryptographer.h"
-#include "sync/util/extensions_activity_monitor.h"
+#include "sync/util/extensions_activity.h"
 #include "sync/util/test_unrecoverable_error_handler.h"
 #include "sync/util/time.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -800,6 +799,8 @@ class SyncManagerTest : public testing::Test,
   void SetUp() {
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
 
+    extensions_activity_ = new ExtensionsActivity();
+
     SyncCredentials credentials;
     credentials.email = "foo@bar.com";
     credentials.sync_token = "sometoken";
@@ -823,15 +824,16 @@ class SyncManagerTest : public testing::Test,
         false,
         scoped_ptr<HttpPostProviderFactory>(new TestHttpPostProviderFactory()),
         workers,
-        &extensions_activity_monitor_,
+        extensions_activity_.get(),
         this,
         credentials,
         "fake_invalidator_client_id",
         std::string(),
         std::string(),  // bootstrap tokens
-        scoped_ptr<InternalComponentsFactory>(GetFactory()),
+        scoped_ptr<InternalComponentsFactory>(GetFactory()).get(),
         &encryptor_,
-        &handler_,
+        scoped_ptr<UnrecoverableErrorHandler>(
+            new TestUnrecoverableErrorHandler).Pass(),
         NULL,
         false);
 
@@ -1010,11 +1012,10 @@ class SyncManagerTest : public testing::Test,
   base::ScopedTempDir temp_dir_;
   // Sync Id's for the roots of the enabled datatypes.
   std::map<ModelType, int64> type_roots_;
-  FakeExtensionsActivityMonitor extensions_activity_monitor_;
+  scoped_refptr<ExtensionsActivity> extensions_activity_;
 
  protected:
   FakeEncryptor encryptor_;
-  TestUnrecoverableErrorHandler handler_;
   SyncManagerImpl sync_manager_;
   WeakHandle<JsBackend> js_backend_;
   StrictMock<SyncManagerObserverMock> manager_observer_;
diff --git a/sync/internal_api/test/fake_sync_manager.cc b/sync/internal_api/test/fake_sync_manager.cc
index 7a6d6a4..cac310b 100644
--- a/sync/internal_api/test/fake_sync_manager.cc
+++ b/sync/internal_api/test/fake_sync_manager.cc
@@ -81,17 +81,16 @@ void FakeSyncManager::Init(
     bool use_ssl,
     scoped_ptr<HttpPostProviderFactory> post_factory,
     const std::vector<ModelSafeWorker*>& workers,
-    ExtensionsActivityMonitor* extensions_activity_monitor,
+    ExtensionsActivity* extensions_activity,
     ChangeDelegate* change_delegate,
     const SyncCredentials& credentials,
     const std::string& invalidator_client_id,
     const std::string& restored_key_for_bootstrapping,
     const std::string& restored_keystore_key_for_bootstrapping,
-    scoped_ptr<InternalComponentsFactory> internal_components_factory,
+    InternalComponentsFactory* internal_components_factory,
     Encryptor* encryptor,
-    UnrecoverableErrorHandler* unrecoverable_error_handler,
-    ReportUnrecoverableErrorFunction
-        report_unrecoverable_error_function,
+    scoped_ptr<UnrecoverableErrorHandler> unrecoverable_error_handler,
+    ReportUnrecoverableErrorFunction report_unrecoverable_error_function,
     bool use_oauth2_token) {
   sync_task_runner_ = base::ThreadTaskRunnerHandle::Get();
   PurgePartiallySyncedTypes();
@@ -210,10 +209,7 @@ void FakeSyncManager::SaveChanges() {
   // Do nothing.
 }
 
-void FakeSyncManager::StopSyncingForShutdown(const base::Closure& callback) {
-  if (!sync_task_runner_->PostTask(FROM_HERE, callback)) {
-    NOTREACHED();
-  }
+void FakeSyncManager::StopSyncingForShutdown() {
 }
 
 void FakeSyncManager::ShutdownOnSyncThread() {
diff --git a/sync/internal_api/test/test_internal_components_factory.cc b/sync/internal_api/test/test_internal_components_factory.cc
index 154c58cc..03d2d82 100644
--- a/sync/internal_api/test/test_internal_components_factory.cc
+++ b/sync/internal_api/test/test_internal_components_factory.cc
@@ -31,7 +31,7 @@ TestInternalComponentsFactory::BuildContext(
     ServerConnectionManager* connection_manager,
     syncable::Directory* directory,
     const std::vector<ModelSafeWorker*>& workers,
-    ExtensionsActivityMonitor* monitor,
+    ExtensionsActivity* monitor,
     const std::vector<SyncEngineEventListener*>& listeners,
     sessions::DebugInfoGetter* debug_info_getter,
     TrafficRecorder* traffic_recorder,