Add (fixed) Sync.PostedClientToServerMessageError2 histogram

There are currently 2 ways in which the ClientToServerResponse proto
may signal an error [1][2]. Sync.PostedClientToServerMessageError used
to consider only one of them, causing some buckets to be empty. This
CL introduces a new version of the histogram which records the metric
correctly.

The old histogram is kept around for the moment so the two can be
compared side by side once the new histogram reaches Stable. It will
be deprecated after this.

[1] https://source.chromium.org/chromium/chromium/src/+/b90d88871c6e4f9c44a1f19d244c3e1632ae26de:components/sync/protocol/sync.proto;l=975
[2] https://source.chromium.org/chromium/chromium/src/+/b90d88871c6e4f9c44a1f19d244c3e1632ae26de:components/sync/protocol/sync.proto;l=953

Bug: 1004302
Change-Id: I6d56061bc032664a1c0a3c303f8ccdd39f60e749
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2584003
Commit-Queue: Caitlin Fischer <caitlinfischer@google.com>
Reviewed-by: Caitlin Fischer <caitlinfischer@google.com>
Reviewed-by: Marc Treib <treib@chromium.org>
Auto-Submit: Victor Vianna <victorvianna@google.com>
Cr-Commit-Position: refs/heads/master@{#836596}

components/sync/engine_impl/syncer_proto_util.cc

@@ -375,10 +375,22 @@ bool SyncerProtoUtil::PostAndProcessHeaders(ServerConnectionManager* scm,
                              base::Time::Now() - start_time);
 
   if (response->error_code() != sync_pb::SyncEnums::SUCCESS) {
+    // TODO(crbug.com/1004302): Stop recording once
+    // Sync.PostedClientToServerMessageError2 (recorded below) has reached
+    // Stable. The reason is so the two can be compared for the same population.
     base::UmaHistogramSparse("Sync.PostedClientToServerMessageError",
                              response->error_code());
   }
 
+  // The error can be specified in 2 different fields, so consider both of them.
+  sync_pb::SyncEnums::ErrorType error_type =
+      response->has_error() ? response->error().error_type()
+                            : response->error_code();
+  if (error_type != sync_pb::SyncEnums::SUCCESS) {
+    base::UmaHistogramSparse("Sync.PostedClientToServerMessageError2",
+                             error_type);
+  }
+
   return true;
 }
 

tools/metrics/histograms/histograms_xml/sync/histograms.xml

@@ -738,7 +738,26 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
 
 <histogram name="Sync.PostedClientToServerMessageError" enum="SyncErrorType"
     expires_after="2021-06-06">
+<!-- TODO(crbug.com/1004302): Deprecate once
+Sync.PostedClientToServerMessageError2 has reached Stable (so the two histograms
+can be compared for the same population). -->
+
+  <owner>mastiz@chromium.org</owner>
+  <owner>treib@chromium.org</owner>
+  <summary>
+    WARNING: Some buckets in this histogram are not recorded due to a bug
+    (crbug.com/1004302), see Sync.PostedClientToServerMessageError2 instead.
+
+    A sync error code received from the sync server as a result of a
+    client-initiated request. Note that this excludes network errors (e.g.
+    client offline) and HTTP errors.
+  </summary>
+</histogram>
+
+<histogram name="Sync.PostedClientToServerMessageError2" enum="SyncErrorType"
+    expires_after="2021-12-14">
   <owner>mastiz@chromium.org</owner>
+  <owner>treib@chromium.org</owner>
   <summary>
     A sync error code received from the sync server as a result of a
     client-initiated request. Note that this excludes network errors (e.g.