[Sync] Fix thread-ordering-dependent NULL dereference in NewNonFrontendDTC

chrome/browser/sync/glue/new_non_frontend_data_type_controller.h

Index: chrome/browser/sync/glue/new_non_frontend_data_type_controller.h
diff --git a/chrome/browser/sync/glue/new_non_frontend_data_type_controller.h b/chrome/browser/sync/glue/new_non_frontend_data_type_controller.h
index 64e65d6537af4460a74cbf0bfbe1a213d92f972e..2dd3ffd81069856aa856cb8cefb5fd8c2615fbcc 100644
--- a/chrome/browser/sync/glue/new_non_frontend_data_type_controller.h
+++ b/chrome/browser/sync/glue/new_non_frontend_data_type_controller.h
@@ -30,13 +30,13 @@ class NewNonFrontendDataTypeController : public NonFrontendDataTypeController {
   NewNonFrontendDataTypeController();
 
   // Overrides of NonFrontendDataTypeController methods.
-  virtual void StartAssociation() OVERRIDE;
   virtual void StartDone(DataTypeController::StartResult result,
                          DataTypeController::State new_state,
                          const SyncError& error) OVERRIDE;
   virtual void StartDoneImpl(DataTypeController::StartResult result,
                              DataTypeController::State new_state,
                              const SyncError& error) OVERRIDE;
+  virtual bool StartAssociationAsync() OVERRIDE;
 
   // Extract/create the syncable service from the profile and return a
   // WeakHandle to it.
@@ -44,34 +44,46 @@ class NewNonFrontendDataTypeController : public NonFrontendDataTypeController {
       const = 0;
 
  private:
+  // Posted on the backend thread by StartAssociationAsync().
+  void StartAssociationWithSharedChangeProcessor(
+      const scoped_refptr<SharedChangeProcessor>& shared_change_processor);
+
+  // Calls Disconnect() on |shared_change_processor_|, then sets it to
+  // NULL.  Must be called only by StartDoneImpl() or Stop() (on the
+  // UI thread) and only after a call to Start() (i.e.,
+  // |shared_change_processor_| must be non-NULL).
+  void ClearSharedChangeProcessor();
+
   // Posts StopLocalService() to the datatype's thread.
   void StopLocalServiceAsync();
 
-  // Calls local_service_->StopSyncing() and releases our references to it and
-  // |shared_change_processor_|.
+  // Calls local_service_->StopSyncing() and releases our references to it.
   void StopLocalService();
 
   // Deprecated.
   virtual void CreateSyncComponents() OVERRIDE;
 
+  // The shared change processor is the thread-safe interface to the
+  // datatype.  We hold a reference to it from the UI thread so that
+  // we can call Disconnect() on it from Stop()/StartDoneImpl().  Most
+  // of the work is done on the backend thread, and in
+  // StartAssociationWithSharedChangeProcessor() for this class in
+  // particular.
+  //
+  // Lifetime: The SharedChangeProcessor object is created on the UI
+  // thread and passed on to the backend thread.  This reference is
+  // released on the UI thread in Stop()/StartDoneImpl(), but the
+  // backend thread may still have references to it (which is okay,
+  // since we call Disconnect() before releasing the UI thread
+  // reference).
+  scoped_refptr<SharedChangeProcessor> shared_change_processor_;
+
   // A weak pointer to the actual local syncable service, which performs all the
   // real work. We do not own the object, and it is only safe to access on the
   // DataType's thread.
   // Lifetime: it gets set in StartAssociation() and released in
   // StopLocalService().
   base::WeakPtr<SyncableService> local_service_;
-
-  // The thread-safe layer between the datatype and the syncer. It allows us to
-  // disconnect both the datatype and ourselves from the syncer at shutdown
-  // time. All accesses to the syncer from any thread other than the UI thread
-  // should be through this. Once connected, it must be released on the
-  // datatype's thread (but if we never connect it we can destroy it on the UI
-  // thread as well).
-  // Lifetime: It gets created on the UI thread in Start(..), connected to
-  // the syncer in StartAssociation() on the datatype's thread, and if
-  // successfully connected released in StopLocalService() on the datatype's
-  // thread.
-  scoped_refptr<SharedChangeProcessor> shared_change_processor_;
 };
 
 }  // namespace browser_sync