Fix SyncManager initialization failure crash.

chrome/browser/sync/glue/sync_backend_host.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "build/build_config.h"
 
 #include "chrome/browser/sync/glue/sync_backend_host.h"
 
 #include <algorithm>
 #include <map>
@@ skipping 828 matching lines @@
       &SyncBackendHost::HandleSyncCycleCompletedOnFrontendLoop,
       snapshot);
 }
 
 
 void SyncBackendHost::Core::OnInitializationComplete(
     const syncer::WeakHandle<syncer::JsBackend>& js_backend,
     bool success,
     const syncer::ModelTypeSet restored_types) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
+
+  if (!success) {
+    sync_manager_->RemoveObserver(this);
+    sync_manager_.reset();

[tim (not reviewing), 2012/07/31 19:40:26]

The main danger with this is that SyncManager needs to make sure it tears down
what it needs to if it detects failure. We actually carefully tear things down
in StopSyncingForShutdown and ShutdownOnSyncThread, but this will mean we just
rely on declaration orders / destructors. 

I'd recommend we move the call to set up connection_manager_ to *after* we
return false for good measure -- if we created that and (for example) set up the
NetworkChangeNotifier, we'd be in danger of hanging on shutdown since we'd
return false but never call TerminateAllIO (because we'd never call
StopSyncingForShutdown).

I'd also recommend, if we can, that we call
sync_manager_->ShutdownOnSyncThread() before reset here.

[rlarocque, 2012/07/31 20:14:07]

Good idea.  Done.
Done.

+  }
+
   host_.Call(
       FROM_HERE,
       &SyncBackendHost::HandleSyncManagerInitializationOnFrontendLoop,
       js_backend, success, restored_types);
 
   if (success) {
     // Initialization is complete, so we can schedule recurring SaveChanges.
     sync_loop_->PostTask(FROM_HERE,
                          base::Bind(&Core::StartSavingChanges, this));
   }
@@ skipping 185 matching lines @@
 void SyncBackendHost::Core::DoRefreshNigori(
     const base::Closure& done_callback) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
   chrome::VersionInfo version_info;
   sync_manager_->RefreshNigori(version_info.CreateVersionString(),
                                done_callback);
 }
 
 void SyncBackendHost::Core::DoStopSyncManagerForShutdown(
     const base::Closure& closure) {
-  DCHECK(sync_manager_.get());
-  sync_manager_->StopSyncingForShutdown(closure);
+  if (sync_manager_.get()) {
+    sync_manager_->StopSyncingForShutdown(closure);
+  } else {
+    sync_loop_->PostTask(FROM_HERE, closure);
+  }
 }
 
 void SyncBackendHost::Core::DoShutdown(bool sync_disabled) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
-  if (!sync_manager_.get())
-    return;
+  if (sync_manager_.get()) {
+    save_changes_timer_.reset();
+    sync_manager_->ShutdownOnSyncThread();
+    sync_manager_->RemoveObserver(this);
+    sync_manager_.reset();
+  }
 
-  save_changes_timer_.reset();
-  sync_manager_->ShutdownOnSyncThread();
-  sync_manager_->RemoveObserver(this);
-  sync_manager_.reset();
   chrome_sync_notification_bridge_ = NULL;
   registrar_ = NULL;
 
   if (sync_disabled)
     DeleteSyncDataFolder();
 
   sync_loop_ = NULL;
 
   host_.Reset();
 }
@@ skipping 302 matching lines @@
       FROM_HERE,
       base::Bind(&SyncBackendHost::Core::DoRefreshNigori,
                  core_.get(), sync_thread_done_callback));
 }
 
 #undef SDVLOG
 
 #undef SLOG
 
 }  // namespace browser_sync