Revert 120698 - [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
===================================================================
--- chrome/browser/sync/glue/new_non_frontend_data_type_controller.h	(revision 120699)
+++ chrome/browser/sync/glue/new_non_frontend_data_type_controller.h	(working copy)
@@ -30,13 +30,13 @@
   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,46 +44,34 @@
       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.
+  // Calls local_service_->StopSyncing() and releases our references to it and
+  // |shared_change_processor_|.
   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