[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..9c915403ddfa5c8fc56a5cbdacc1061312f3711a 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);
 }
@@ -48,7 +45,6 @@ void SharedModelTypeProcessor::OnSyncStarting(
 }
 
 void SharedModelTypeProcessor::OnMetadataLoaded(
-    SyncError error,
     std::unique_ptr<MetadataBatch> batch) {
   DCHECK(CalledOnValidThread());
   DCHECK(entities_.empty());
@@ -59,12 +55,6 @@ 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()) {
-    start_error_ = error;
-    ConnectIfReady();
-    return;
-  }
-
   if (batch->GetModelTypeState().initial_sync_done()) {
     EntityMetadataMap metadata_map(batch->TakeAllMetadata());
     std::vector<std::string> entities_to_commit;
@@ -96,10 +86,14 @@ void SharedModelTypeProcessor::OnMetadataLoaded(
   ConnectIfReady();
 }
 
+bool SharedModelTypeProcessor::ConnectPreconditionsMet() const {
+  return is_metadata_loaded_ && is_initial_pending_data_loaded_ &&
+         error_handler_;
+}
+
 void SharedModelTypeProcessor::ConnectIfReady() {
   DCHECK(CalledOnValidThread());
-  if (!is_metadata_loaded_ || !is_initial_pending_data_loaded_ ||
-      start_callback_.is_null()) {
+  if (!ConnectPreconditionsMet()) {
     return;
   }
 
@@ -114,7 +108,8 @@ void SharedModelTypeProcessor::ConnectIfReady() {
             base::ThreadTaskRunnerHandle::Get());
   }
 
-  start_callback_.Run(start_error_, std::move(activation_context));
+  start_callback_.Run(ModelToSyncError(start_error_),
+                      std::move(activation_context));
   start_callback_.Reset();
 }
 
@@ -144,14 +139,28 @@ bool SharedModelTypeProcessor::IsTrackingMetadata() {
   return model_type_state_.initial_sync_done();
 }
 
-SyncError SharedModelTypeProcessor::CreateAndUploadError(
+void SharedModelTypeProcessor::ReportError(const ModelError& error) {
+  DCHECK(error.IsSet());
+
+  if (ConnectPreconditionsMet()) {
+    // If both model and sync are ready, then |start_callback_| was already
+    // called and this can't be treated as a start error.
+    DCHECK(error_handler_);
+    error_handler_->OnUnrecoverableError(ModelToSyncError(error));
+  } else if (!start_error_.IsSet()) {
+    start_error_ = error;
+    // An early model error means we're no longer expecting OnMetadataLoaded to
+    // be called.
+    is_metadata_loaded_ = true;
+    is_initial_pending_data_loaded_ = true;
+    ConnectIfReady();
+  }
+}
+
+void SharedModelTypeProcessor::ReportError(
     const tracked_objects::Location& location,
     const std::string& message) {
-  if (error_handler_) {
-    return error_handler_->CreateAndUploadError(location, message, type_);
-  } else {
-    return SyncError(location, SyncError::DATATYPE_ERROR, message, type_);
-  }
+  ReportError(ModelError(location, message));
 }
 
 void SharedModelTypeProcessor::ConnectSync(
@@ -295,10 +304,10 @@ void SharedModelTypeProcessor::OnCommitCompleted(
     }
   }
 
-  SyncError error =
+  ModelError error =
       bridge_->ApplySyncChanges(std::move(change_list), EntityChangeList());
   if (error.IsSet()) {
-    error_handler_->OnUnrecoverableError(error);
+    ReportError(error);
   }
 }
 
@@ -352,11 +361,11 @@ void SharedModelTypeProcessor::OnUpdateReceived(
   }
 
   // Inform the bridge of the new or updated data.
-  SyncError error =
+  ModelError error =
       bridge_->ApplySyncChanges(std::move(metadata_changes), entity_changes);
 
   if (error.IsSet()) {
-    error_handler_->OnUnrecoverableError(error);
+    ReportError(error);
   } else {
     // There may be new reasons to commit by the time this function is done.
     FlushPendingCommitRequests();
@@ -545,11 +554,11 @@ void SharedModelTypeProcessor::OnInitialUpdateReceived(
   }
 
   // Let the bridge handle associating and merging the data.
-  SyncError error =
+  ModelError error =
       bridge_->MergeSyncData(std::move(metadata_changes), data_map);
 
   if (error.IsSet()) {
-    error_handler_->OnUnrecoverableError(error);
+    ReportError(error);
   } else {
     // We may have new reasons to commit by the time this function is done.
     FlushPendingCommitRequests();
@@ -557,30 +566,19 @@ void SharedModelTypeProcessor::OnInitialUpdateReceived(
 }
 
 void SharedModelTypeProcessor::OnInitialPendingDataLoaded(
-    SyncError error,
     std::unique_ptr<DataBatch> data_batch) {
   DCHECK(!is_initial_pending_data_loaded_);
 
-  if (error.IsSet()) {
-    start_error_ = error;
-  } else {
-    ConsumeDataBatch(std::move(data_batch));
-  }
-
+  ConsumeDataBatch(std::move(data_batch));
   is_initial_pending_data_loaded_ = true;
+
   ConnectIfReady();
 }
 
 void SharedModelTypeProcessor::OnDataLoadedForReEncryption(
-    SyncError error,
     std::unique_ptr<DataBatch> data_batch) {
   DCHECK(is_initial_pending_data_loaded_);
 
-  if (error.IsSet()) {
-    error_handler_->OnUnrecoverableError(error);
-    return;
-  }
-
   ConsumeDataBatch(std::move(data_batch));
   FlushPendingCommitRequests();
 }
@@ -646,4 +644,13 @@ ProcessorEntityTracker* SharedModelTypeProcessor::CreateEntity(
   return CreateEntity(bridge_->GetStorageKey(data), data);
 }
 
+SyncError SharedModelTypeProcessor::ModelToSyncError(ModelError error) const {

[skym, 2017/01/09 18:20:28]

Is there a reason you cannot take a const ModelError& error?

Also, if this method took ModelType as a param it could be in the anonymous
namespace instead of an instanced method.

[maxbogue, 2017/01/09 21:29:33]

Made it const&. It'll be going away in a follow-up anyways so not gonna make it
anon.

+  if (error.IsSet()) {
+    return SyncError(error.location(), SyncError::DATATYPE_ERROR,
+                     error.message(), type_);
+  } else {
+    return SyncError();
+  }
+}
+
 }  // namespace syncer