[Sync] Introduce ModelError for USS error handling.

components/sync/model_impl/shared_model_type_processor.cc

Index: components/sync/model_impl/shared_model_type_processor.cc
diff --git a/components/sync/model_impl/shared_model_type_processor.cc b/components/sync/model_impl/shared_model_type_processor.cc
index 6b5816f90a209628f7a6ad2a741908e70f139088..910e62d3b7706a32df1fbfdf455257e8fc932fca 100644
--- a/components/sync/model_impl/shared_model_type_processor.cc
+++ b/components/sync/model_impl/shared_model_type_processor.cc
@@ -23,10 +23,7 @@ namespace syncer {
 SharedModelTypeProcessor::SharedModelTypeProcessor(ModelType type,
                                                    ModelTypeSyncBridge* bridge)
     : type_(type),
-      is_metadata_loaded_(false),
-      is_initial_pending_data_loaded_(false),
       bridge_(bridge),
-      error_handler_(nullptr),
       weak_ptr_factory_(this) {
   DCHECK(bridge);
 }
@@ -59,7 +56,7 @@ void SharedModelTypeProcessor::OnMetadataLoaded(
   // Flip this flag here to cover all cases where we don't need to load data.
   is_initial_pending_data_loaded_ = true;
 
-  if (error.IsSet()) {
+  if (error.IsSet() && !start_error_.IsSet()) {
     start_error_ = error;
     ConnectIfReady();

[skym, 2017/01/05 17:03:49]

While you're in here, would be nice to explain why we connect if we're in an
error state. I think this is at first intuitively backwards.

[maxbogue, 2017/01/07 00:12:39]

n/a, I think?

     return;
@@ -144,9 +141,18 @@ bool SharedModelTypeProcessor::IsTrackingMetadata() {
   return model_type_state_.initial_sync_done();
 }
 
+void SharedModelTypeProcessor::OnUnrecoverableError(const SyncError& error) {
+  if (error_handler_) {
+    error_handler_->OnUnrecoverableError(error);
+  } else if (!start_error_.IsSet()) {
+    start_error_ = error;
+  }
+}
+
 SyncError SharedModelTypeProcessor::CreateAndUploadError(
     const tracked_objects::Location& location,
-    const std::string& message) {
+    const std::string& message,
+    ModelType type) {

[skym, 2017/01/05 17:03:49]

Doesn't seem like type is very useful to be part of this interface. I guess you
could DCHECK that type == type_.

[maxbogue, 2017/01/07 00:12:39]

n/a

   if (error_handler_) {
     return error_handler_->CreateAndUploadError(location, message, type_);
   } else {
@@ -154,6 +160,11 @@ SyncError SharedModelTypeProcessor::CreateAndUploadError(
   }
 }
 
+std::unique_ptr<DataTypeErrorHandler> SharedModelTypeProcessor::Copy() const {
+  NOTREACHED();

[skym, 2017/01/05 17:03:49]

I think this probably deserves a comment. Could explain why we're in this
situation, why Copy() doesn't make sense for this class.

[maxbogue, 2017/01/07 00:12:39]

n/a

+  return nullptr;
+}
+
 void SharedModelTypeProcessor::ConnectSync(
     std::unique_ptr<CommitQueue> worker) {
   DCHECK(CalledOnValidThread());
@@ -561,7 +572,7 @@ void SharedModelTypeProcessor::OnInitialPendingDataLoaded(
     std::unique_ptr<DataBatch> data_batch) {
   DCHECK(!is_initial_pending_data_loaded_);
 
-  if (error.IsSet()) {
+  if (error.IsSet() && !start_error_.IsSet()) {
     start_error_ = error;
   } else {
     ConsumeDataBatch(std::move(data_batch));