Split bucket for auth errors in SyncSharingMessageCommitErrorCode

When interacting with the sync server, HTTP errors that represent auth
errors (401) get treated as SyncerError::SYNC_AUTH_ERROR. When reporting
these cases in metric like Sync.SharingMessage.CommitResult, they should
not contribute as "Sync server error", since the nature of the problem
is different and very likely transient (i.e. expired tokens).

In follow-up patches, the actual logic to handle this situation should
be improved.

Bug: 1097054

Change-Id: I2e806fab89a49e8840324f98d68f1aadd1c40080
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2253845
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Rushan Suleymanov <rushans@google.com>
Reviewed-by: Alex Chau <alexchau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#780273}

chrome/browser/sharing/sharing_fcm_sender.cc

@@ -317,7 +317,10 @@ void SharingFCMSender::OnMessageSentViaSync(
     case sync_pb::SharingMessageCommitError::UNAUTHENTICATED:
     case sync_pb::SharingMessageCommitError::PERMISSION_DENIED:
     case sync_pb::SharingMessageCommitError::SYNC_TURNED_OFF:
+    case sync_pb::SharingMessageCommitError::
+        DEPRECATED_SYNC_SERVER_OR_AUTH_ERROR:
     case sync_pb::SharingMessageCommitError::SYNC_SERVER_ERROR:
+    case sync_pb::SharingMessageCommitError::SYNC_AUTH_ERROR:
       send_message_result = SharingSendMessageResult::kInternalError;
       break;
     case sync_pb::SharingMessageCommitError::SYNC_NETWORK_ERROR:

chrome/browser/sharing/sharing_message_bridge_impl.cc

@@ -176,6 +176,10 @@ void SharingMessageBridgeImpl::OnCommitAttemptFailed(
       sharing_message_error_code =
           sync_pb::SharingMessageCommitError::SYNC_NETWORK_ERROR;
       break;
+    case syncer::SyncCommitError::kAuthError:
+      sharing_message_error_code =
+          sync_pb::SharingMessageCommitError::SYNC_AUTH_ERROR;
+      break;
     case syncer::SyncCommitError::kServerError:
     case syncer::SyncCommitError::kBadServerResponse:
       sharing_message_error_code =

components/sync/engine/non_blocking_sync_common.h

@@ -21,7 +21,12 @@ namespace syncer {
 
 static const int64_t kUncommittedVersion = -1;
 
-enum class SyncCommitError { kNetworkError, kServerError, kBadServerResponse };
+enum class SyncCommitError {
+  kNetworkError,
+  kAuthError,
+  kServerError,
+  kBadServerResponse
+};
 
 struct CommitRequestData {
   CommitRequestData();

components/sync/engine_impl/commit.cc

@@ -7,6 +7,7 @@
 #include <utility>
 
 #include "base/metrics/histogram_macros.h"
+#include "base/notreached.h"
 #include "base/rand_util.h"
 #include "base/trace_event/trace_event.h"
 #include "components/sync/base/data_type_histogram.h"
@@ -21,6 +22,7 @@
 namespace syncer {
 
 namespace {
+
 // The number of random ASCII bytes we'll add to CommitMessage. We choose 256
 // because it is not too large (to hurt performance and compression ratio), but
 // it is not too small to easily be canceled out using statistical analysis.
@@ -36,6 +38,43 @@ std::string RandASCIIString(size_t length) {
   return result;
 }
 
+SyncCommitError GetSyncCommitError(SyncerError syncer_error) {
+  switch (syncer_error.value()) {
+    case SyncerError::UNSET:
+    case SyncerError::CANNOT_DO_WORK:
+    case SyncerError::SYNCER_OK:
+    case SyncerError::DATATYPE_TRIGGERED_RETRY:
+    case SyncerError::SERVER_MORE_TO_DOWNLOAD:
+      NOTREACHED();
+      break;
+    case SyncerError::NETWORK_CONNECTION_UNAVAILABLE:
+    case SyncerError::NETWORK_IO_ERROR:
+      return SyncCommitError::kNetworkError;
+    case SyncerError::SYNC_AUTH_ERROR:
+    case SyncerError::SERVER_RETURN_INVALID_CREDENTIAL:
+      return SyncCommitError::kAuthError;
+    case SyncerError::SYNC_SERVER_ERROR:
+    case SyncerError::SERVER_RETURN_UNKNOWN_ERROR:
+    case SyncerError::SERVER_RETURN_THROTTLED:
+    case SyncerError::SERVER_RETURN_TRANSIENT_ERROR:
+    case SyncerError::SERVER_RETURN_MIGRATION_DONE:
+    case SyncerError::SERVER_RETURN_CLEAR_PENDING:
+    case SyncerError::SERVER_RETURN_NOT_MY_BIRTHDAY:
+    case SyncerError::SERVER_RETURN_CONFLICT:
+    case SyncerError::SERVER_RETURN_USER_ROLLBACK:
+    case SyncerError::SERVER_RETURN_PARTIAL_FAILURE:
+    case SyncerError::SERVER_RETURN_CLIENT_DATA_OBSOLETE:
+    case SyncerError::SERVER_RETURN_ENCRYPTION_OBSOLETE:
+    case SyncerError::SERVER_RETURN_DISABLED_BY_ADMIN:
+      return SyncCommitError::kServerError;
+    case SyncerError::SERVER_RESPONSE_VALIDATION_FAILED:
+      return SyncCommitError::kBadServerResponse;
+  }
+
+  NOTREACHED();
+  return SyncCommitError::kServerError;
+}
+
 }  // namespace
 
 Commit::Commit(ContributionMap contributions,
@@ -215,18 +254,7 @@ void Commit::CleanUp() {
 }
 
 void Commit::ReportFullCommitFailure(SyncerError syncer_error) {
-  SyncCommitError commit_error = SyncCommitError::kServerError;
-  switch (syncer_error.value()) {
-    case SyncerError::NETWORK_CONNECTION_UNAVAILABLE:
-    case SyncerError::NETWORK_IO_ERROR:
-      commit_error = SyncCommitError::kNetworkError;
-      break;
-    case SyncerError::SERVER_RESPONSE_VALIDATION_FAILED:
-      commit_error = SyncCommitError::kBadServerResponse;
-      break;
-    default:
-      commit_error = SyncCommitError::kServerError;
-  }
+  const SyncCommitError commit_error = GetSyncCommitError(syncer_error);
   for (auto& model_type_and_contribution : contributions_) {
     model_type_and_contribution.second->ProcessCommitFailure(commit_error);
   }

components/sync/protocol/sharing_message_specifics.proto

@@ -70,10 +70,15 @@ message SharingMessageCommitError {
     // Client-specific error codes.
     SYNC_TURNED_OFF = 8;
     SYNC_NETWORK_ERROR = 9;
-    // Error code for server error or unparsable server response.
-    SYNC_SERVER_ERROR = 10;
+    // Deprecated UMA bucket, prior to splitting between SYNC_SERVER_ERROR and
+    // SYNC_AUTH_ERROR.
+    DEPRECATED_SYNC_SERVER_OR_AUTH_ERROR = 10;
     // Message wasn't committed before timeout.
     SYNC_TIMEOUT = 11;
+    // Error code for server error or unparsable server response.
+    SYNC_SERVER_ERROR = 12;
+    // Auth error when communicating with the server.
+    SYNC_AUTH_ERROR = 13;
   }
 
   optional ErrorCode error_code = 1;

tools/metrics/histograms/enums.xml

@@ -67048,8 +67048,10 @@ would be helpful to identify which type is being sent.
   <int value="7" label="Permission denied"/>
   <int value="8" label="Sync turned off"/>
   <int value="9" label="Sync network error"/>
-  <int value="10" label="Sync server error"/>
+  <int value="10" label="[DEPRECATED] Sync server or auth error"/>
   <int value="11" label="Sync bridge timeout"/>
+  <int value="12" label="Sync server error"/>
+  <int value="13" label="Sync auth error"/>
 </enum>
 
 <enum name="SyncSimpleConflictResolutions">