[Sync] Introduce ModelError for USS error handling.

components/sync/device_info/device_info_sync_bridge.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/sync/device_info/device_info_sync_bridge.h"
 
 #include <stdint.h>
 
 #include <algorithm>
 #include <set>
@@ skipping 367 matching lines @@
 
 void DeviceInfoSyncBridge::OnReadAllMetadata(
     SyncError error,
     std::unique_ptr<MetadataBatch> metadata_batch) {
   change_processor()->OnMetadataLoaded(error, std::move(metadata_batch));
   ReconcileLocalAndStored();
 }
 
 void DeviceInfoSyncBridge::OnCommit(Result result) {
   if (result != Result::SUCCESS) {
-    change_processor()->CreateAndUploadError(FROM_HERE,
-                                             "Failed a write to store.");
+    change_processor()->OnUnrecoverableError(
+        SyncError(FROM_HERE, SyncError::DATATYPE_ERROR,
+                  "Failed a write to store.", type()));
   }
 }
 
 void DeviceInfoSyncBridge::ReconcileLocalAndStored() {
   // On initial syncing we will have a change processor here, but it will not be
   // tracking changes. We need to persist a copy of our local device info to
   // disk, but the Put call to the processor will be ignored. That should be
   // fine however, as the discrepancy will be picked up later in merge. We don't
   // bother trying to track this case and act intelligently because simply not
   // much of a benefit in doing so.
@@ skipping 69 matching lines @@
                        [now](ClientIdToSpecifics::const_reference pair) {
                          return DeviceInfoUtil::IsActive(
                              GetLastUpdateTime(*pair.second), now);
                        });
 }
 
 void DeviceInfoSyncBridge::ReportStartupErrorToSync(const std::string& msg) {
   // TODO(skym): Shouldn't need to log this here, reporting should always log.
   LOG(WARNING) << msg;
   change_processor()->OnMetadataLoaded(
-      change_processor()->CreateAndUploadError(FROM_HERE, msg), nullptr);
+      change_processor()->CreateAndUploadError(FROM_HERE, msg, type()),

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

What if, we instead abandon the old error interfaces and write new ones. We
don't have to NOTREACHED in Copy(). We don't have to supply a type to
CreateAndUploadError(). I don't really like the existence of
CreateAndUploadError(). First, it shouldn't upload. that should be done in
OnUnrecoverableError() or OnMetadataLoaded(). Except AndUpload is in the name.

But, I don't think CreateAndUploadError() is even needed if we were to remove
SyncErrors from OnMetadataLoaded(). Can we instead have the bridge just call
OnUnrecoverableError() and never get around to OnMetadataLoaded? Making sure
OnMetadataLoaded() is always called is a hassle for the bridge, and can be error
prone when encountering errors.

Hmm, you don't even need to start with a new interface. Just create a one off
method in the Proc.

void SharedModelTypeProcessor::ReportUnrecoverableError(const
tracked_objects::Location& location, const std::string& message) {
  ...
}

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

Done!

+      nullptr);
 }
 
 }  // namespace syncer