[Sync] Use OnSingleDataTypeUnrecoverableError for all errors

This removes the disable callback from all datatype controllers as well
as the DisableDataType method from the PSS, in favor of
OnSingleDataTypeUnrecoverable error (which now accepts a SyncError).

BUG=368834
TBR=bauerb@chromium.org

Review URL: https://codereview.chromium.org/436733002

Cr-Commit-Position: refs/heads/master@{#288557}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288557 0039d316-1c4b-4281-b951-d872f2087c98

18 files changed, 91 insertions, 159 deletions

diff --git a/components/sync_driver/change_processor_mock.h b/components/sync_driver/change_processor_mock.h
index aaa7e82d..7acf83f 100644
--- a/components/sync_driver/change_processor_mock.h
+++ b/components/sync_driver/change_processor_mock.h
@@ -25,9 +25,8 @@ class ChangeProcessorMock
   MOCK_CONST_METHOD0(IsRunning, bool());
   MOCK_METHOD2(OnUnrecoverableError, void(const tracked_objects::Location&,
                                           const std::string&));
-  MOCK_METHOD2(OnSingleDatatypeUnrecoverableError,
-                     void(const tracked_objects::Location&,
-                          const std::string&));
+  MOCK_METHOD1(OnSingleDataTypeUnrecoverableError,
+               void(const syncer::SyncError&));
   MOCK_METHOD3(CreateAndUploadError,
                    syncer::SyncError(const tracked_objects::Location&,
                              const std::string&,
diff --git a/components/sync_driver/data_type_controller.cc b/components/sync_driver/data_type_controller.cc
index 3f7a681..db26151 100644
--- a/components/sync_driver/data_type_controller.cc
+++ b/components/sync_driver/data_type_controller.cc
@@ -12,11 +12,9 @@ namespace sync_driver {
 
 DataTypeController::DataTypeController(
     scoped_refptr<base::MessageLoopProxy> ui_thread,
-    const base::Closure& error_callback,
-    const DisableTypeCallback& disable_callback)
+    const base::Closure& error_callback)
     : base::RefCountedDeleteOnMessageLoop<DataTypeController>(ui_thread),
       error_callback_(error_callback),
-      disable_callback_(disable_callback),
       user_share_(NULL) {
 }
 
@@ -51,11 +49,6 @@ syncer::UserShare* DataTypeController::user_share() const {
   return user_share_;
 }
 
-DataTypeController::DisableTypeCallback
-DataTypeController::disable_callback() {
-  return disable_callback_;
-}
-
 bool DataTypeController::ReadyForStart() const {
   return true;
 }
diff --git a/components/sync_driver/data_type_controller.h b/components/sync_driver/data_type_controller.h
index 7d8b67c..a673b1f 100644
--- a/components/sync_driver/data_type_controller.h
+++ b/components/sync_driver/data_type_controller.h
@@ -149,8 +149,7 @@ class DataTypeController
   friend class base::DeleteHelper<DataTypeController>;
 
   DataTypeController(scoped_refptr<base::MessageLoopProxy> ui_thread,
-                     const base::Closure& error_callback,
-                     const DisableTypeCallback& disable_callback);
+                     const base::Closure& error_callback);
 
   // If the DTC is waiting for models to load, once the models are
   // loaded the datatype service will call this function on DTC to let
@@ -160,16 +159,12 @@ class DataTypeController
   virtual ~DataTypeController();
 
   syncer::UserShare* user_share() const;
-  DisableTypeCallback disable_callback();
 
   // The callback that will be invoked when an unrecoverable error occurs.
   // TODO(sync): protected for use by legacy controllers.
   base::Closure error_callback_;
 
  private:
-  // TODO(tim): Bug 383480. Do we need two callbacks?
-  DisableTypeCallback disable_callback_;
-
   syncer::UserShare* user_share_;
 };
 
diff --git a/components/sync_driver/data_type_error_handler.h b/components/sync_driver/data_type_error_handler.h
index a33a151..174b9a8 100644
--- a/components/sync_driver/data_type_error_handler.h
+++ b/components/sync_driver/data_type_error_handler.h
@@ -18,9 +18,8 @@ class DataTypeErrorHandler {
  public:
   // Call this to disable a datatype while it is running. This is usually
   // called for a runtime failure that is specific to a datatype.
-  virtual void OnSingleDatatypeUnrecoverableError(
-      const tracked_objects::Location& from_here,
-      const std::string& message) = 0;
+  virtual void OnSingleDataTypeUnrecoverableError(
+      const syncer::SyncError& error) = 0;
 
   // This will create a syncer::SyncError object. This will also upload
   // a breakpad call stack to crash server. A sync error usually means
diff --git a/components/sync_driver/data_type_error_handler_mock.h b/components/sync_driver/data_type_error_handler_mock.h
index 6e70e2b..a5cd6d4 100644
--- a/components/sync_driver/data_type_error_handler_mock.h
+++ b/components/sync_driver/data_type_error_handler_mock.h
@@ -14,8 +14,8 @@ class DataTypeErrorHandlerMock : public DataTypeErrorHandler {
  public:
   DataTypeErrorHandlerMock();
   virtual ~DataTypeErrorHandlerMock();
-  MOCK_METHOD2(OnSingleDatatypeUnrecoverableError,
-               void(const tracked_objects::Location&, const std::string&));
+  MOCK_METHOD1(OnSingleDataTypeUnrecoverableError,
+               void(const syncer::SyncError&));
   MOCK_METHOD3(CreateAndUploadError,
                    syncer::SyncError(const tracked_objects::Location&,
                              const std::string&,
diff --git a/components/sync_driver/fake_data_type_controller.cc b/components/sync_driver/fake_data_type_controller.cc
index 5ba1c4c..897e77a 100644
--- a/components/sync_driver/fake_data_type_controller.cc
+++ b/components/sync_driver/fake_data_type_controller.cc
@@ -12,8 +12,7 @@ using syncer::ModelType;
 namespace sync_driver {
 
 FakeDataTypeController::FakeDataTypeController(ModelType type)
-      : DataTypeController(base::MessageLoopProxy::current(), base::Closure(),
-                           DisableTypeCallback()),
+      : DataTypeController(base::MessageLoopProxy::current(), base::Closure()),
         state_(NOT_RUNNING),
         model_load_delayed_(false),
         type_(type),
@@ -137,11 +136,8 @@ DataTypeController::State FakeDataTypeController::state() const {
   return state_;
 }
 
-void FakeDataTypeController::OnSingleDatatypeUnrecoverableError(
-    const tracked_objects::Location& from_here,
-    const std::string& message) {
-  syncer::SyncError error(
-      from_here, syncer::SyncError::DATATYPE_ERROR, message, type_);
+void FakeDataTypeController::OnSingleDataTypeUnrecoverableError(
+    const syncer::SyncError& error) {
   syncer::SyncMergeResult local_merge_result(type());
   local_merge_result.set_error(error);
   last_start_callback_.Run(
diff --git a/components/sync_driver/fake_data_type_controller.h b/components/sync_driver/fake_data_type_controller.h
index 840a5b2..a84bc27 100644
--- a/components/sync_driver/fake_data_type_controller.h
+++ b/components/sync_driver/fake_data_type_controller.h
@@ -33,9 +33,8 @@ class FakeDataTypeController : public DataTypeController {
   virtual syncer::ModelSafeGroup model_safe_group() const OVERRIDE;
   virtual ChangeProcessor* GetChangeProcessor() const OVERRIDE;
   virtual State state() const OVERRIDE;
-  virtual void OnSingleDatatypeUnrecoverableError(
-      const tracked_objects::Location& from_here,
-      const std::string& message) OVERRIDE;
+  virtual void OnSingleDataTypeUnrecoverableError(
+      const syncer::SyncError& error) OVERRIDE;
   virtual bool ReadyForStart() const OVERRIDE;
 
   void FinishStart(ConfigureResult result);
diff --git a/components/sync_driver/generic_change_processor.cc b/components/sync_driver/generic_change_processor.cc
index d7a5c6e..c44ce72 100644
--- a/components/sync_driver/generic_change_processor.cc
+++ b/components/sync_driver/generic_change_processor.cc
@@ -144,10 +144,13 @@ void GenericChangeProcessor::ApplyChangesFromSyncModel(
       // Need to load specifics from node.
       syncer::ReadNode read_node(trans);
       if (read_node.InitByIdLookup(it->id) != syncer::BaseNode::INIT_OK) {
-        error_handler()->OnSingleDatatypeUnrecoverableError(
+        syncer::SyncError error(
             FROM_HERE,
+            syncer::SyncError::DATATYPE_ERROR,
             "Failed to look up data for received change with id " +
-                base::Int64ToString(it->id));
+                base::Int64ToString(it->id),
+            syncer::GetModelTypeFromSpecifics(it->specifics));
+        error_handler()->OnSingleDataTypeUnrecoverableError(error);
         return;
       }
       syncer_changes_.push_back(syncer::SyncChange(
@@ -168,17 +171,14 @@ void GenericChangeProcessor::CommitChangesFromSyncModel() {
                             syncer::SyncError::DATATYPE_ERROR,
                             "Local service destroyed.",
                             type);
-    error_handler()->OnSingleDatatypeUnrecoverableError(error.location(),
-                                                        error.message());
+    error_handler()->OnSingleDataTypeUnrecoverableError(error);
     return;
   }
   syncer::SyncError error = local_service_->ProcessSyncChanges(FROM_HERE,
                                                        syncer_changes_);
   syncer_changes_.clear();
-  if (error.IsSet()) {
-    error_handler()->OnSingleDatatypeUnrecoverableError(
-        error.location(), error.message());
-  }
+  if (error.IsSet())
+    error_handler()->OnSingleDataTypeUnrecoverableError(error);
 }
 
 syncer::SyncDataList GenericChangeProcessor::GetAllSyncData(
@@ -285,9 +285,8 @@ int GenericChangeProcessor::GetSyncCountForType(syncer::ModelType type) {
 
 namespace {
 
-// TODO(isherman): Investigating http://crbug.com/121592
 // WARNING: this code is sensitive to compiler optimizations. Be careful
-// modifying any code around an OnSingleDatatypeUnrecoverableError call, else
+// modifying any code around an OnSingleDataTypeUnrecoverableError call, else
 // the compiler attempts to merge it with other calls, losing useful information
 // in breakpad uploads.
 syncer::SyncError LogLookupFailure(
@@ -303,24 +302,21 @@ syncer::SyncError LogLookupFailure(
                   error_prefix +
                       "could not find entry matching the lookup criteria.",
                   type);
-      error_handler->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-                                                        error.message());
+      error_handler->OnSingleDataTypeUnrecoverableError(error);
       LOG(ERROR) << "Delete: Bad entry.";
       return error;
     }
     case syncer::BaseNode::INIT_FAILED_ENTRY_IS_DEL: {
       syncer::SyncError error;
       error.Reset(from_here, error_prefix + "entry is already deleted.", type);
-      error_handler->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-                                                        error.message());
+      error_handler->OnSingleDataTypeUnrecoverableError(error);
       LOG(ERROR) << "Delete: Deleted entry.";
       return error;
     }
     case syncer::BaseNode::INIT_FAILED_DECRYPT_IF_NECESSARY: {
       syncer::SyncError error;
       error.Reset(from_here, error_prefix + "unable to decrypt", type);
-      error_handler->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-                                                        error.message());
+      error_handler->OnSingleDataTypeUnrecoverableError(error);
       LOG(ERROR) << "Delete: Undecryptable entry.";
       return error;
     }
@@ -329,8 +325,7 @@ syncer::SyncError LogLookupFailure(
       error.Reset(from_here,
                   error_prefix + "a precondition was not met for calling init.",
                   type);
-      error_handler->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-                                                        error.message());
+      error_handler->OnSingleDataTypeUnrecoverableError(error);
       LOG(ERROR) << "Delete: Failed precondition.";
       return error;
     }
@@ -338,8 +333,7 @@ syncer::SyncError LogLookupFailure(
       syncer::SyncError error;
       // Should have listed all the possible error cases above.
       error.Reset(from_here, error_prefix + "unknown error", type);
-      error_handler->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-                                                        error.message());
+      error_handler->OnSingleDataTypeUnrecoverableError(error);
       LOG(ERROR) << "Delete: Unknown error.";
       return error;
     }
@@ -361,8 +355,7 @@ syncer::SyncError AttemptDelete(const syncer::SyncChange& change,
           "Failed to delete " + type_str + " node. Local data, empty tag. " +
               change.location().ToString(),
           type);
-      error_handler->OnSingleDatatypeUnrecoverableError(error.location(),
-                                                        error.message());
+      error_handler->OnSingleDataTypeUnrecoverableError(error);
       NOTREACHED();
       return error;
     }
@@ -464,8 +457,7 @@ syncer::SyncError GenericChangeProcessor::ProcessSyncChanges(
           "Received unset SyncChange in the change processor, " +
               change.location().ToString(),
           type);
-      error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-                                                          error.message());
+      error_handler()->OnSingleDataTypeUnrecoverableError(error);
       NOTREACHED();
       LOG(ERROR) << "Unset sync change.";
       return error;
@@ -480,7 +472,7 @@ syncer::SyncError GenericChangeProcessor::ProcessSyncChanges(
 }
 
 // WARNING: this code is sensitive to compiler optimizations. Be careful
-// modifying any code around an OnSingleDatatypeUnrecoverableError call, else
+// modifying any code around an OnSingleDataTypeUnrecoverableError call, else
 // the compiler attempts to merge it with other calls, losing useful information
 // in breakpad uploads.
 syncer::SyncError GenericChangeProcessor::HandleActionAdd(
@@ -500,8 +492,7 @@ syncer::SyncError GenericChangeProcessor::HandleActionAdd(
                             syncer::SyncError::DATATYPE_ERROR,
                             "Failed to look up root node for type " + type_str,
                             type);
-    error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-                                                        error.message());
+    error_handler()->OnSingleDataTypeUnrecoverableError(error);
     NOTREACHED();
     LOG(ERROR) << "Create: no root node.";
     return error;
@@ -516,24 +507,21 @@ syncer::SyncError GenericChangeProcessor::HandleActionAdd(
       case syncer::WriteNode::INIT_FAILED_EMPTY_TAG: {
         syncer::SyncError error;
         error.Reset(FROM_HERE, error_prefix + "empty tag", type);
-        error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-                                                            error.message());
+        error_handler()->OnSingleDataTypeUnrecoverableError(error);
         LOG(ERROR) << "Create: Empty tag.";
         return error;
       }
       case syncer::WriteNode::INIT_FAILED_ENTRY_ALREADY_EXISTS: {
         syncer::SyncError error;
         error.Reset(FROM_HERE, error_prefix + "entry already exists", type);
-        error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-                                                            error.message());
+        error_handler()->OnSingleDataTypeUnrecoverableError(error);
         LOG(ERROR) << "Create: Entry exists.";
         return error;
       }
       case syncer::WriteNode::INIT_FAILED_COULD_NOT_CREATE_ENTRY: {
         syncer::SyncError error;
         error.Reset(FROM_HERE, error_prefix + "failed to create entry", type);
-        error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-                                                            error.message());
+        error_handler()->OnSingleDataTypeUnrecoverableError(error);
         LOG(ERROR) << "Create: Could not create entry.";
         return error;
       }
@@ -541,16 +529,14 @@ syncer::SyncError GenericChangeProcessor::HandleActionAdd(
         syncer::SyncError error;
         error.Reset(
             FROM_HERE, error_prefix + "failed to set predecessor", type);
-        error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-                                                            error.message());
+        error_handler()->OnSingleDataTypeUnrecoverableError(error);
         LOG(ERROR) << "Create: Bad predecessor.";
         return error;
       }
       default: {
         syncer::SyncError error;
         error.Reset(FROM_HERE, error_prefix + "unknown error", type);
-        error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-                                                            error.message());
+        error_handler()->OnSingleDataTypeUnrecoverableError(error);
         LOG(ERROR) << "Create: Unknown error.";
         return error;
       }
@@ -575,7 +561,7 @@ syncer::SyncError GenericChangeProcessor::HandleActionAdd(
   return syncer::SyncError();
 }
 // WARNING: this code is sensitive to compiler optimizations. Be careful
-// modifying any code around an OnSingleDatatypeUnrecoverableError call, else
+// modifying any code around an OnSingleDataTypeUnrecoverableError call, else
 // the compiler attempts to merge it with other calls, losing useful information
 // in breakpad uploads.
 syncer::SyncError GenericChangeProcessor::HandleActionUpdate(
@@ -597,22 +583,19 @@ syncer::SyncError GenericChangeProcessor::HandleActionUpdate(
     if (result == syncer::BaseNode::INIT_FAILED_PRECONDITION) {
       syncer::SyncError error;
       error.Reset(FROM_HERE, error_prefix + "empty tag", type);
-      error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-                                                          error.message());
+      error_handler()->OnSingleDataTypeUnrecoverableError(error);
       LOG(ERROR) << "Update: Empty tag.";
       return error;
     } else if (result == syncer::BaseNode::INIT_FAILED_ENTRY_NOT_GOOD) {
       syncer::SyncError error;
       error.Reset(FROM_HERE, error_prefix + "bad entry", type);
-      error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-                                                          error.message());
+      error_handler()->OnSingleDataTypeUnrecoverableError(error);
       LOG(ERROR) << "Update: bad entry.";
       return error;
     } else if (result == syncer::BaseNode::INIT_FAILED_ENTRY_IS_DEL) {
       syncer::SyncError error;
       error.Reset(FROM_HERE, error_prefix + "deleted entry", type);
-      error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-                                                          error.message());
+      error_handler()->OnSingleDataTypeUnrecoverableError(error);
       LOG(ERROR) << "Update: deleted entry.";
       return error;
     } else {
@@ -630,8 +613,7 @@ syncer::SyncError GenericChangeProcessor::HandleActionUpdate(
                     "nigori mismatch for " +
                         type_str + ".",
                     type);
-        error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-                                                            error.message());
+        error_handler()->OnSingleDataTypeUnrecoverableError(error);
         LOG(ERROR) << "Update: encr case 1.";
         return error;
       } else if (agreement && can_decrypt) {
@@ -641,8 +623,7 @@ syncer::SyncError GenericChangeProcessor::HandleActionUpdate(
                     "and the nigori matches (?!) for " +
                         type_str + ".",
                     type);
-        error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-                                                            error.message());
+        error_handler()->OnSingleDataTypeUnrecoverableError(error);
         LOG(ERROR) << "Update: encr case 2.";
         return error;
       } else if (agreement) {
@@ -652,8 +633,7 @@ syncer::SyncError GenericChangeProcessor::HandleActionUpdate(
                     "the nigori matches for " +
                         type_str + ".",
                     type);
-        error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-                                                            error.message());
+        error_handler()->OnSingleDataTypeUnrecoverableError(error);
         LOG(ERROR) << "Update: encr case 3.";
         return error;
       } else {
@@ -663,8 +643,7 @@ syncer::SyncError GenericChangeProcessor::HandleActionUpdate(
                     "(?!) and nigori mismatch for " +
                         type_str + ".",
                     type);
-        error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-                                                            error.message());
+        error_handler()->OnSingleDataTypeUnrecoverableError(error);
         LOG(ERROR) << "Update: encr case 4.";
         return error;
       }
diff --git a/components/sync_driver/model_association_manager_unittest.cc b/components/sync_driver/model_association_manager_unittest.cc
index 906d99b..5c26319 100644
--- a/components/sync_driver/model_association_manager_unittest.cc
+++ b/components/sync_driver/model_association_manager_unittest.cc
@@ -339,8 +339,12 @@ TEST_F(SyncModelAssociationManagerTest, StopAfterConfiguration) {
   testing::Mock::VerifyAndClearExpectations(&delegate_);
   EXPECT_CALL(delegate_,
               OnSingleDataTypeWillStop(syncer::BOOKMARKS, _));
+  syncer::SyncError error(FROM_HERE,
+                          syncer::SyncError::DATATYPE_ERROR,
+                          "error",
+                          syncer::BOOKMARKS);
   GetController(controllers_, syncer::BOOKMARKS)
-      ->OnSingleDatatypeUnrecoverableError(FROM_HERE, "runtime error");
+      ->OnSingleDataTypeUnrecoverableError(error);
 }
 
 }  // namespace sync_driver
diff --git a/components/sync_driver/non_ui_data_type_controller.cc b/components/sync_driver/non_ui_data_type_controller.cc
index 211f142..67edb40 100644
--- a/components/sync_driver/non_ui_data_type_controller.cc
+++ b/components/sync_driver/non_ui_data_type_controller.cc
@@ -24,9 +24,8 @@ NonUIDataTypeController::CreateSharedChangeProcessor() {
 NonUIDataTypeController::NonUIDataTypeController(
     scoped_refptr<base::MessageLoopProxy> ui_thread,
     const base::Closure& error_callback,
-    const DisableTypeCallback& disable_callback,
     SyncApiComponentFactory* sync_factory)
-    : DataTypeController(ui_thread, error_callback, disable_callback),
+    : DataTypeController(ui_thread, error_callback),
       sync_factory_(sync_factory),
       state_(NOT_RUNNING),
       ui_thread_(ui_thread) {
@@ -111,6 +110,7 @@ void NonUIDataTypeController::StartAssociating(
 
 void NonUIDataTypeController::Stop() {
   DCHECK(ui_thread_->BelongsToCurrentThread());
+
   if (state() == NOT_RUNNING)
     return;
 
@@ -166,22 +166,20 @@ DataTypeController::State NonUIDataTypeController::state() const {
   return state_;
 }
 
-void NonUIDataTypeController::OnSingleDatatypeUnrecoverableError(
-    const tracked_objects::Location& from_here, const std::string& message) {
+void NonUIDataTypeController::OnSingleDataTypeUnrecoverableError(
+    const syncer::SyncError& error) {
   DCHECK(!ui_thread_->BelongsToCurrentThread());
   // TODO(tim): We double-upload some errors.  See bug 383480.
   if (!error_callback_.is_null())
     error_callback_.Run();
-  ui_thread_->PostTask(from_here,
+  ui_thread_->PostTask(error.location(),
       base::Bind(&NonUIDataTypeController::DisableImpl,
                  this,
-                 from_here,
-                 message));
+                 error));
 }
 
 NonUIDataTypeController::NonUIDataTypeController()
-    : DataTypeController(base::MessageLoopProxy::current(), base::Closure(),
-                         DisableTypeCallback()),
+    : DataTypeController(base::MessageLoopProxy::current(), base::Closure()),
       sync_factory_(NULL) {}
 
 NonUIDataTypeController::~NonUIDataTypeController() {}
@@ -272,15 +270,12 @@ void NonUIDataTypeController::AbortModelLoad() {
 }
 
 void NonUIDataTypeController::DisableImpl(
-    const tracked_objects::Location& from_here,
-    const std::string& message) {
+    const syncer::SyncError& error) {
   DCHECK(ui_thread_->BelongsToCurrentThread());
   UMA_HISTOGRAM_ENUMERATION("Sync.DataTypeRunFailures",
                             ModelTypeToHistogramInt(type()),
                             syncer::MODEL_TYPE_COUNT);
   if (!start_callback_.is_null()) {
-    syncer::SyncError error(
-        from_here, syncer::SyncError::DATATYPE_ERROR, message, type());
     syncer::SyncMergeResult local_merge_result(type());
     local_merge_result.set_error(error);
     start_callback_.Run(RUNTIME_ERROR,
diff --git a/components/sync_driver/non_ui_data_type_controller.h b/components/sync_driver/non_ui_data_type_controller.h
index 17a0dbe..0f4fdab 100644
--- a/components/sync_driver/non_ui_data_type_controller.h
+++ b/components/sync_driver/non_ui_data_type_controller.h
@@ -27,7 +27,6 @@ class NonUIDataTypeController : public DataTypeController {
   NonUIDataTypeController(
       scoped_refptr<base::MessageLoopProxy> ui_thread,
       const base::Closure& error_callback,
-      const DisableTypeCallback& disable_callback,
       SyncApiComponentFactory* sync_factory);
 
   // DataTypeController interface.
@@ -40,9 +39,8 @@ class NonUIDataTypeController : public DataTypeController {
   virtual ChangeProcessor* GetChangeProcessor() const OVERRIDE;
   virtual std::string name() const OVERRIDE;
   virtual State state() const OVERRIDE;
-  virtual void OnSingleDatatypeUnrecoverableError(
-      const tracked_objects::Location& from_here,
-      const std::string& message) OVERRIDE;
+  virtual void OnSingleDataTypeUnrecoverableError(
+      const syncer::SyncError& error) OVERRIDE;
 
  protected:
   // For testing only.
@@ -125,8 +123,7 @@ class NonUIDataTypeController : public DataTypeController {
   // Disable this type with the sync service. Should only be invoked in case of
   // an unrecoverable error.
   // Note: this is performed on the UI thread.
-  void DisableImpl(const tracked_objects::Location& from_here,
-                   const std::string& message);
+  void DisableImpl(const syncer::SyncError& error);
 
   SyncApiComponentFactory* const sync_factory_;
 
diff --git a/components/sync_driver/non_ui_data_type_controller_mock.h b/components/sync_driver/non_ui_data_type_controller_mock.h
index 1369c33..3716a35 100644
--- a/components/sync_driver/non_ui_data_type_controller_mock.h
+++ b/components/sync_driver/non_ui_data_type_controller_mock.h
@@ -26,9 +26,8 @@ class NonUIDataTypeControllerMock
   MOCK_CONST_METHOD0(name, std::string());
   MOCK_CONST_METHOD0(model_safe_group, syncer::ModelSafeGroup());
   MOCK_CONST_METHOD0(state, State());
-  MOCK_METHOD2(OnSingleDataTypeUnrecoverableError,
-               void(const tracked_objects::Location&,
-                    const std::string&));
+  MOCK_METHOD1(OnSingleDataTypeUnrecoverableError,
+               void(const syncer::SyncError& error));
 
   // NonUIDataTypeController mocks.
   MOCK_METHOD0(StartModels, bool());
diff --git a/components/sync_driver/non_ui_data_type_controller_unittest.cc b/components/sync_driver/non_ui_data_type_controller_unittest.cc
index 021db41..875523d 100644
--- a/components/sync_driver/non_ui_data_type_controller_unittest.cc
+++ b/components/sync_driver/non_ui_data_type_controller_unittest.cc
@@ -96,12 +96,10 @@ class NonUIDataTypeControllerFake
       SyncApiComponentFactory* sync_factory,
       NonUIDataTypeControllerMock* mock,
       SharedChangeProcessor* change_processor,
-      const DisableTypeCallback& disable_callback,
       scoped_refptr<base::MessageLoopProxy> backend_loop)
       : NonUIDataTypeController(
           base::MessageLoopProxy::current(),
           base::Closure(),
-          disable_callback,
           sync_factory),
         blocked_(false),
         mock_(mock),
@@ -188,22 +186,17 @@ class NonUIDataTypeControllerFake
 class SyncNonUIDataTypeControllerTest : public testing::Test {
  public:
   SyncNonUIDataTypeControllerTest()
-      : backend_thread_("dbthread"),
-        disable_callback_invoked_(false) {}
+      : backend_thread_("dbthread") {}
 
   virtual void SetUp() OVERRIDE {
     backend_thread_.Start();
     change_processor_ = new SharedChangeProcessorMock();
     // All of these are refcounted, so don't need to be released.
     dtc_mock_ = new StrictMock<NonUIDataTypeControllerMock>();
-    DataTypeController::DisableTypeCallback disable_callback =
-        base::Bind(&SyncNonUIDataTypeControllerTest::DisableTypeCallback,
-                   base::Unretained(this));
     non_ui_dtc_ =
         new NonUIDataTypeControllerFake(NULL,
                                         dtc_mock_.get(),
                                         change_processor_,
-                                        disable_callback,
                                         backend_thread_.message_loop_proxy());
   }
 
@@ -271,12 +264,6 @@ class SyncNonUIDataTypeControllerTest : public testing::Test {
     done->Signal();
   }
 
-  void DisableTypeCallback(const tracked_objects::Location& location,
-                           const std::string& message) {
-    disable_callback_invoked_ = true;
-    non_ui_dtc_->Stop();
-  }
-
   base::MessageLoopForUI message_loop_;
   base::Thread backend_thread_;
 
@@ -288,8 +275,6 @@ class SyncNonUIDataTypeControllerTest : public testing::Test {
   scoped_refptr<NonUIDataTypeControllerMock> dtc_mock_;
   scoped_refptr<SharedChangeProcessorMock> change_processor_;
   scoped_ptr<syncer::SyncChangeProcessor> saved_change_processor_;
-
-  bool disable_callback_invoked_;
 };
 
 TEST_F(SyncNonUIDataTypeControllerTest, StartOk) {
@@ -496,7 +481,7 @@ TEST_F(SyncNonUIDataTypeControllerTest, StopStart) {
   EXPECT_EQ(DataTypeController::RUNNING, non_ui_dtc_->state());
 }
 
-TEST_F(SyncNonUIDataTypeControllerTest, OnSingleDatatypeUnrecoverableError) {
+TEST_F(SyncNonUIDataTypeControllerTest, OnSingleDataTypeUnrecoverableError) {
   SetStartExpectations();
   SetAssociateExpectations();
   SetActivateExpectations(DataTypeController::OK);
@@ -507,12 +492,15 @@ TEST_F(SyncNonUIDataTypeControllerTest, OnSingleDatatypeUnrecoverableError) {
 
   testing::Mock::VerifyAndClearExpectations(&start_callback_);
   EXPECT_CALL(start_callback_, Run(DataTypeController::RUNTIME_ERROR, _, _));
+  syncer::SyncError error(FROM_HERE,
+                          syncer::SyncError::DATATYPE_ERROR,
+                          "error",
+                          non_ui_dtc_->type());
   backend_thread_.message_loop_proxy()->PostTask(FROM_HERE, base::Bind(
       &NonUIDataTypeControllerFake::
-          OnSingleDatatypeUnrecoverableError,
+          OnSingleDataTypeUnrecoverableError,
       non_ui_dtc_.get(),
-      FROM_HERE,
-      std::string("Test")));
+      error));
   WaitForDTC();
 }
 
diff --git a/components/sync_driver/proxy_data_type_controller.cc b/components/sync_driver/proxy_data_type_controller.cc
index ae1ea53..c017802 100644
--- a/components/sync_driver/proxy_data_type_controller.cc
+++ b/components/sync_driver/proxy_data_type_controller.cc
@@ -9,7 +9,7 @@ namespace sync_driver {
 ProxyDataTypeController::ProxyDataTypeController(
     scoped_refptr<base::MessageLoopProxy> ui_thread,
     syncer::ModelType type)
-    : DataTypeController(ui_thread, base::Closure(), DisableTypeCallback()),
+    : DataTypeController(ui_thread, base::Closure()),
       state_(NOT_RUNNING),
       type_(type) {
   DCHECK(syncer::ProxyTypes().Has(type_));
@@ -61,8 +61,8 @@ DataTypeController::State ProxyDataTypeController::state() const {
   return state_;
 }
 
-void ProxyDataTypeController::OnSingleDatatypeUnrecoverableError(
-    const tracked_objects::Location& from_here, const std::string& message) {
+void ProxyDataTypeController::OnSingleDataTypeUnrecoverableError(
+    const syncer::SyncError& error) {
   NOTIMPLEMENTED();
 }
 
diff --git a/components/sync_driver/proxy_data_type_controller.h b/components/sync_driver/proxy_data_type_controller.h
index e90ee24..6e255c6 100644
--- a/components/sync_driver/proxy_data_type_controller.h
+++ b/components/sync_driver/proxy_data_type_controller.h
@@ -32,9 +32,8 @@ class ProxyDataTypeController : public DataTypeController {
   virtual State state() const OVERRIDE;
 
   // DataTypeErrorHandler interface.
-  virtual void OnSingleDatatypeUnrecoverableError(
-      const tracked_objects::Location& from_here,
-      const std::string& message) OVERRIDE;
+  virtual void OnSingleDataTypeUnrecoverableError(
+      const syncer::SyncError& error) OVERRIDE;
 
  protected:
   // DataTypeController is RefCounted.
diff --git a/components/sync_driver/ui_data_type_controller.cc b/components/sync_driver/ui_data_type_controller.cc
index 3b7945c..eca5779 100644
--- a/components/sync_driver/ui_data_type_controller.cc
+++ b/components/sync_driver/ui_data_type_controller.cc
@@ -17,8 +17,7 @@ namespace sync_driver {
 
 UIDataTypeController::UIDataTypeController()
     : DataTypeController(base::MessageLoopProxy::current(),
-                         base::Closure(),
-                         DisableTypeCallback()),
+                         base::Closure()),
       sync_factory_(NULL),
       state_(NOT_RUNNING),
       type_(syncer::UNSPECIFIED) {
@@ -27,10 +26,9 @@ UIDataTypeController::UIDataTypeController()
 UIDataTypeController::UIDataTypeController(
     scoped_refptr<base::MessageLoopProxy> ui_thread,
     const base::Closure& error_callback,
-    const DisableTypeCallback& disable_callback,
     syncer::ModelType type,
     SyncApiComponentFactory* sync_factory)
-    : DataTypeController(ui_thread, error_callback, disable_callback),
+    : DataTypeController(ui_thread, error_callback),
       sync_factory_(sync_factory),
       state_(NOT_RUNNING),
       type_(type),
@@ -269,6 +267,9 @@ void UIDataTypeController::Stop() {
   DCHECK(ui_thread_->BelongsToCurrentThread());
   DCHECK(syncer::IsRealDataType(type_));
 
+  if (state_ == NOT_RUNNING)
+    return;
+
   State prev_state = state_;
   state_ = STOPPING;
 
@@ -318,8 +319,9 @@ DataTypeController::State UIDataTypeController::state() const {
   return state_;
 }
 
-void UIDataTypeController::OnSingleDatatypeUnrecoverableError(
-    const tracked_objects::Location& from_here, const std::string& message) {
+void UIDataTypeController::OnSingleDataTypeUnrecoverableError(
+    const syncer::SyncError& error) {
+  DCHECK_EQ(type(), error.model_type());
   UMA_HISTOGRAM_ENUMERATION("Sync.DataTypeRunFailures",
                             ModelTypeToHistogramInt(type()),
                             syncer::MODEL_TYPE_COUNT);
@@ -327,8 +329,6 @@ void UIDataTypeController::OnSingleDatatypeUnrecoverableError(
   if (!error_callback_.is_null())
     error_callback_.Run();
   if (!start_callback_.is_null()) {
-    syncer::SyncError error(
-        from_here, syncer::SyncError::DATATYPE_ERROR, message, type());
     syncer::SyncMergeResult local_merge_result(type());
     local_merge_result.set_error(error);
     start_callback_.Run(RUNTIME_ERROR,
diff --git a/components/sync_driver/ui_data_type_controller.h b/components/sync_driver/ui_data_type_controller.h
index ae7983e..3028991 100644
--- a/components/sync_driver/ui_data_type_controller.h
+++ b/components/sync_driver/ui_data_type_controller.h
@@ -34,7 +34,6 @@ class UIDataTypeController : public DataTypeController {
   UIDataTypeController(
       scoped_refptr<base::MessageLoopProxy> ui_thread,
       const base::Closure& error_callback,
-      const DisableTypeCallback& disable_callback,
       syncer::ModelType type,
       SyncApiComponentFactory* sync_factory);
 
@@ -50,9 +49,8 @@ class UIDataTypeController : public DataTypeController {
   virtual State state() const OVERRIDE;
 
   // DataTypeErrorHandler interface.
-  virtual void OnSingleDatatypeUnrecoverableError(
-      const tracked_objects::Location& from_here,
-      const std::string& message) OVERRIDE;
+  virtual void OnSingleDataTypeUnrecoverableError(
+      const syncer::SyncError& error) OVERRIDE;
 
   // Used by tests to override the factory used to create
   // GenericChangeProcessors.
diff --git a/components/sync_driver/ui_data_type_controller_unittest.cc b/components/sync_driver/ui_data_type_controller_unittest.cc
index e1b6356..20bd304 100644
--- a/components/sync_driver/ui_data_type_controller_unittest.cc
+++ b/components/sync_driver/ui_data_type_controller_unittest.cc
@@ -31,17 +31,13 @@ class SyncUIDataTypeControllerTest : public testing::Test,
  public:
   SyncUIDataTypeControllerTest()
       : type_(syncer::PREFERENCES),
-        change_processor_(NULL),
-        disable_callback_invoked_(false) {}
+        change_processor_(NULL) {}
 
   virtual void SetUp() {
     preference_dtc_ =
         new UIDataTypeController(
             base::MessageLoopProxy::current(),
             base::Closure(),
-            base::Bind(&SyncUIDataTypeControllerTest::DisableTypeCallback,
-                       base::Unretained(this),
-                       type_),
             type_,
             this);
     SetStartExpectations();
@@ -89,13 +85,6 @@ class SyncUIDataTypeControllerTest : public testing::Test,
     message_loop_.RunUntilIdle();
   }
 
-  void DisableTypeCallback(syncer::ModelType type,
-                           const tracked_objects::Location& location,
-                           const std::string& message) {
-    disable_callback_invoked_ = true;
-    preference_dtc_->Stop();
-  }
-
   base::MessageLoopForUI message_loop_;
   const syncer::ModelType type_;
   StartCallbackMock start_callback_;
@@ -103,7 +92,6 @@ class SyncUIDataTypeControllerTest : public testing::Test,
   scoped_refptr<UIDataTypeController> preference_dtc_;
   FakeGenericChangeProcessor* change_processor_;
   syncer::FakeSyncableService syncable_service_;
-  bool disable_callback_invoked_;
 };
 
 // Start the DTC. Verify that the callback is called with OK, the
@@ -200,7 +188,11 @@ TEST_F(SyncUIDataTypeControllerTest, OnSingleDatatypeUnrecoverableError) {
 
   testing::Mock::VerifyAndClearExpectations(&start_callback_);
   EXPECT_CALL(start_callback_, Run(DataTypeController::RUNTIME_ERROR, _, _));
-  preference_dtc_->OnSingleDatatypeUnrecoverableError(FROM_HERE, "Test");
+  syncer::SyncError error(FROM_HERE,
+                          syncer::SyncError::DATATYPE_ERROR,
+                          "error",
+                          syncer::PREFERENCES);
+  preference_dtc_->OnSingleDataTypeUnrecoverableError(error);
 }
 
 }  // namespace