sync: Gracefully handle very early shutdown

chrome/browser/sync/glue/sync_backend_host.cc

Index: chrome/browser/sync/glue/sync_backend_host.cc
diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc
index 2f91728d69096ca2a55838b0e751a67550093421..2db700111cd3bf5afa48833edbfefbbe1911ab32 100644
--- a/chrome/browser/sync/glue/sync_backend_host.cc
+++ b/chrome/browser/sync/glue/sync_backend_host.cc
@@ -198,12 +198,12 @@ class SyncBackendHost::Core
   void DoFinishInitialProcessControlTypes();
 
   // The shutdown order is a bit complicated:
-  // 1) Call DoStopSyncManagerForShutdown() from |frontend_loop_| to request
-  //    sync manager to stop as soon as possible.
+  // 1) Call the SyncManagerStopHandle's RequestStop() from |frontend_loop_| to
+  //    request sync manager to stop as soon as possible.
   // 2) Post DoShutdown() to sync loop to clean up backend state, save
   //    directory and destroy sync manager.
-  void DoStopSyncManagerForShutdown();
-  void DoShutdown(bool sync_disabled);
+  void DoShutdown(bool sync_disabled,
+                  scoped_ptr<syncer::CancelationSignal> signal);
   void DoDestroySyncManager();
 
   // Configuration methods that must execute on sync loop.
@@ -305,7 +305,8 @@ SyncBackendHost::SyncBackendHost(
       cached_passphrase_type_(syncer::IMPLICIT_PASSPHRASE),
       invalidator_(
           invalidation::InvalidationServiceFactory::GetForProfile(profile)),
-      invalidation_handler_registered_(false) {
+      invalidation_handler_registered_(false),
+      cancelation_signal_(new syncer::CancelationSignal()) {
 }
 
 SyncBackendHost::SyncBackendHost(Profile* profile)
@@ -401,7 +402,8 @@ void SyncBackendHost::Initialize(
           new InternalComponentsFactoryImpl(factory_switches)).Pass(),
       unrecoverable_error_handler.Pass(),
       report_unrecoverable_error_function,
-      !cl->HasSwitch(switches::kSyncDisableOAuth2Token)));
+      !cl->HasSwitch(switches::kSyncDisableOAuth2Token),
+      cancelation_signal_.get()));
   InitCore(init_opts.Pass());
 }
 
@@ -492,24 +494,9 @@ bool SyncBackendHost::SetDecryptionPassphrase(const std::string& passphrase) {
   return true;
 }
 
-void SyncBackendHost::StopSyncManagerForShutdown() {
-  DCHECK_GT(initialization_state_, NOT_ATTEMPTED);
-  if (initialization_state_ == CREATING_SYNC_MANAGER) {
-    // We post here to implicitly wait for the SyncManager to be created,
-    // if needed.  We have to wait, since we need to shutdown immediately,
-    // and we need to tell the SyncManager so it can abort any activity
-    // (net I/O, data application).
-    DCHECK(registrar_->sync_thread()->IsRunning());
-    registrar_->sync_thread()->message_loop()->PostTask(FROM_HERE,
-        base::Bind(&SyncBackendHost::Core::DoStopSyncManagerForShutdown,
-                   core_.get()));
-  } else {
-    core_->DoStopSyncManagerForShutdown();
-  }
-}
-
 void SyncBackendHost::StopSyncingForShutdown() {
   DCHECK_EQ(base::MessageLoop::current(), frontend_loop_);
+  DCHECK_GT(initialization_state_, NOT_ATTEMPTED);
 
   // Immediately stop sending messages to the frontend.
   frontend_ = NULL;
@@ -521,7 +508,7 @@ void SyncBackendHost::StopSyncingForShutdown() {
 
   registrar_->RequestWorkerStopOnUIThread();
 
-  StopSyncManagerForShutdown();
+  cancelation_signal_->RequestStop();
 }
 
 scoped_ptr<base::Thread> SyncBackendHost::Shutdown(ShutdownOption option) {
@@ -544,10 +531,22 @@ scoped_ptr<base::Thread> SyncBackendHost::Shutdown(ShutdownOption option) {
   invalidation_handler_registered_ = false;
 
   // Shut down and destroy sync manager.
+  //
+  // We're about to drop our reference to the |core_|.  The only thing keeping
+  // it alive will be the reference that lives inside this closure we're about
+  // to post to the sync thread.  If this closure doesn't run right away, it's
+  // possible that our Core will outlive us.
+  //
+  // This is a problem for the |cancelation_signal_|, which must outlive the
+  // SyncManager, SyncScheduler, etc.  It can be destroyed only after the call
+  // to DoDestroySyncManager.  The SyncBackendHost might not live that long, but
+  // this closure will.  We pass ownership of the |cancelation_signal_| to this
+  // closure to keep it alive.
   registrar_->sync_thread()->message_loop()->PostTask(
       FROM_HERE,
       base::Bind(&SyncBackendHost::Core::DoShutdown,
-                 core_.get(), sync_disabled));
+                 core_.get(), sync_disabled,
+                 base::Passed(&cancelation_signal_)));
   core_ = NULL;
 
   // Worker cleanup.
@@ -883,7 +882,8 @@ SyncBackendHost::DoInitializeOptions::DoInitializeOptions(
     scoped_ptr<syncer::UnrecoverableErrorHandler> unrecoverable_error_handler,
     syncer::ReportUnrecoverableErrorFunction
         report_unrecoverable_error_function,
-    bool use_oauth2_token)
+    bool use_oauth2_token,
+    syncer::CancelationSignal* const cancelation_signal)
     : sync_loop(sync_loop),
       registrar(registrar),
       routing_info(routing_info),
@@ -903,7 +903,8 @@ SyncBackendHost::DoInitializeOptions::DoInitializeOptions(
       unrecoverable_error_handler(unrecoverable_error_handler.Pass()),
       report_unrecoverable_error_function(
           report_unrecoverable_error_function),
-      use_oauth2_token(use_oauth2_token) {
+      use_oauth2_token(use_oauth2_token),
+      cancelation_signal(cancelation_signal) {
 }
 
 SyncBackendHost::DoInitializeOptions::~DoInitializeOptions() {}
@@ -1168,7 +1169,8 @@ void SyncBackendHost::Core::DoInitialize(
                       &encryptor_,
                       options->unrecoverable_error_handler.Pass(),
                       options->report_unrecoverable_error_function,
-                      options->use_oauth2_token);
+                      options->use_oauth2_token,
+                      options->cancelation_signal);
 
   // |sync_manager_| may end up being NULL here in tests (in
   // synchronous initialization mode).
@@ -1278,13 +1280,11 @@ void SyncBackendHost::Core::DoEnableEncryptEverything() {
   sync_manager_->GetEncryptionHandler()->EnableEncryptEverything();
 }
 
-void SyncBackendHost::Core::DoStopSyncManagerForShutdown() {
-  if (sync_manager_)
-    sync_manager_->StopSyncingForShutdown();
-}
-
-void SyncBackendHost::Core::DoShutdown(bool sync_disabled) {
+void SyncBackendHost::Core::DoShutdown(
+    bool sync_disabled,
+    scoped_ptr<syncer::CancelationSignal> signal) {
   DCHECK_EQ(base::MessageLoop::current(), sync_loop_);
+
   // It's safe to do this even if the type was never activated.
   registrar_->DeactivateDataType(syncer::DEVICE_INFO);
   synced_device_tracker_.reset();