Logs all reasons card upload was not offered in UKM and UMA.

components/autofill/core/browser/autofill_metrics.h

Index: components/autofill/core/browser/autofill_metrics.h
diff --git a/components/autofill/core/browser/autofill_metrics.h b/components/autofill/core/browser/autofill_metrics.h
index ac50c90e59d8e6c4efb4dc79b92b03dc1c7722c0..6146e858333eddc0b6c934f311ec99912d707ea8 100644
--- a/components/autofill/core/browser/autofill_metrics.h
+++ b/components/autofill/core/browser/autofill_metrics.h
@@ -97,38 +97,38 @@ class AutofillMetrics {
   enum CardUploadDecisionMetric {
     // All the required conditions were satisfied and the card upload prompt was
     // triggered.
-    UPLOAD_OFFERED,
+    UPLOAD_OFFERED = 1 << 0,
     // No CVC was detected. We don't know whether any addresses were available
     // nor whether we would have been able to get upload details.
-    UPLOAD_NOT_OFFERED_NO_CVC,
+    UPLOAD_NOT_OFFERED_NO_CVC = 1 << 1,
     // A CVC was detected but no recently created or used address was available.
     // We don't know whether we would have been able to get upload details.
-    UPLOAD_NOT_OFFERED_NO_ADDRESS,
+    UPLOAD_NOT_OFFERED_NO_ADDRESS = 1 << 2,

[Steven Holte, 2017/05/02 23:50:29]

What about the UPLOAD_NOT_OFFERED part of UPLOAD_NOT_OFFERED_NO_ADDRESS?  Is
that needed?

[csashi, 2017/05/02 23:54:43]

I don't follow this comment. We log this bit if the upload was not offered
because we could not find an address - i.e.
UPLOAD_NOT_OFFERED_NO_ADDRESS => UPLOAD_NOT_OFFERED.

I removed the separate UPLOAD_NOT_OFFERED bit.

Can you clarify? Thanks.

[Steven Holte, 2017/05/03 01:04:05]

For logging this to UKM, it seems like the bit is redundant, since the fact that
you are not setting UPLOAD_OFFERED means upload was not offered. It seems like
it's nice if each bit's meaning is fairly independent, even if one is the cause
for another.

For UMA it seems like you are not actually trying to co-occurances, like whether
NO_ZIP_CODE is occuring with NO_ADDRESS, with the exception of OFFERED and
NO_CVC, which is the only "recoverable" error?  And it seems like you are
planning to compare counts of buckets against another bucket as a baseline,
rather than total counts since you recording multiple times for a single
opportunity.  I guess you also need to update the descriptions of these values
here?

There are maybe a couple other ways you might encode this:

1. If you don't actually have that many distinct combinations of errors, and it
seems like some of them are exclusive/dependent, you could probably get away
with using a SPARSE_HISTOGRAM and just sending the same mask to UMA as you send
to UMA as your buckets, though you'd have to encode the combinations in
histograms.xml

2. If there are more combinations than you care about, and you just care about
co-occurence with Upload offered or not for UMA, you could probably record two
histograms, with the different errors as values, one for offered one for not.

It seems like that might allow you to disentangle some of these bits from a
logic, but it will probably work fine as is too.

[csashi, 2017/05/03 01:20:41]

Unfortunately, not setting UPLOAD_OFFERED does not mean upload was not offered.
The reason is we also have UPLOAD_OFFERED_NO_CVC and we hope to imminently add
additional such bits like UPLOAD_OFFERED_NAME_NORMALIZED etc.

In retrospect, I should have renamed UPLOAD_OFFERED to
UPLOAD_OFFERED_WITHOUT_ANY_FIX (or some such equivalent). I can do that in a
follow up CL or in this CL if the commit fails anyway for some reason.

I agreed that we lose the total count of failures in UMA because we are
recording multiple failures for a single opportunity. Jared also pointed this
out offline. But we will be able to find the total count of failures in UKM.

My primary goal for this change for UMA was to find out if there are big counts
in any of the UPLOAD_NOT_OFFERED bits. This will help us prioritize fixing that
error. Co-occurrences are useful but we will get that from UKM.

Reg. Sparse histograms - Sebastien also suggested this but we don't know if we
will have many distinct combinations or not - UKM will tell us that but right
now we don't know.

     // A CVC and one or more addresses were available but no name was found on
     // either the card or the adress(es). We don't know whether the address(es)
     // were otherwise valid nor whether we would have been able to get upload
     // details.
-    UPLOAD_NOT_OFFERED_NO_NAME,
+    UPLOAD_NOT_OFFERED_NO_NAME = 1 << 3,
     // A CVC, multiple addresses, and a name were available but the adresses had
     // conflicting zip codes. We don't know whether we would have been able to
     // get upload details.
-    UPLOAD_NOT_OFFERED_CONFLICTING_ZIPS,
+    UPLOAD_NOT_OFFERED_CONFLICTING_ZIPS = 1 << 4,
     // A CVC, one or more addresses, and a name were available but no zip code
     // was found on any of the adress(es). We don't know whether we would have
     // been able to get upload details.
-    UPLOAD_NOT_OFFERED_NO_ZIP_CODE,
+    UPLOAD_NOT_OFFERED_NO_ZIP_CODE = 1 << 5,
     // A CVC, one or more valid addresses, and a name were available but the
     // request to Payments for upload details failed.
-    UPLOAD_NOT_OFFERED_GET_UPLOAD_DETAILS_FAILED,
+    UPLOAD_NOT_OFFERED_GET_UPLOAD_DETAILS_FAILED = 1 << 6,
     // A CVC and one or more addresses were available but the names on the card
     // and/or the addresses didn't match. We don't know whether the address(es)
     // were otherwise valid nor whether we would have been able to get upload
     // details.
-    UPLOAD_NOT_OFFERED_CONFLICTING_NAMES,
+    UPLOAD_NOT_OFFERED_CONFLICTING_NAMES = 1 << 7,
     // No CVC was detected, but valid addresses and names were.  Upload is still
     // possible if the user manually enters CVC, so upload was offered.
-    UPLOAD_OFFERED_NO_CVC,
-    NUM_CARD_UPLOAD_DECISION_METRICS,
+    UPLOAD_OFFERED_NO_CVC = 1 << 8,
+    // Update |kNumCardUploadDecisionMetrics| when adding new enum here.
   };
 
   enum DeveloperEngagementMetric {
@@ -645,7 +645,8 @@ class AutofillMetrics {
     base::TimeTicks form_loaded_timestamp_;
   };
 
-  static void LogCardUploadDecisionMetric(CardUploadDecisionMetric metric);
+  // |upload_decision_metrics| is a bitmask of |CardUploadDecisionMetric|.
+  static void LogCardUploadDecisionMetrics(int upload_decision_metrics);
   static void LogCreditCardInfoBarMetric(
       InfoBarMetric metric,
       bool is_uploading,
@@ -804,19 +805,18 @@ class AutofillMetrics {
   // suggestion to show an explanation of the warning.
   static void LogShowedHttpNotSecureExplanation();
 
-  // Logs the card upload decision ukm based on the specified |url| and
-  // |upload_decision|.
-  static void LogCardUploadDecisionUkm(
-      ukm::UkmService* ukm_service,
-      const GURL& url,
-      AutofillMetrics::CardUploadDecisionMetric upload_decision);
+  // Logs the card upload decisions ukm for the specified |url|.
+  // |upload_decision_metrics| is a bitmask of |CardUploadDecisionMetric|.
+  static void LogCardUploadDecisionsUkm(ukm::UkmService* ukm_service,
+                                        const GURL& url,
+                                        int upload_decision_metrics);
 
   // Logs the developer engagement ukm for the specified |url| and autofill
-  // fields in the form structure.
-  static void LogDeveloperEngagementUkm(
-      ukm::UkmService* ukm_service,
-      const GURL& url,
-      std::vector<AutofillMetrics::DeveloperEngagementMetric> metrics);
+  // fields in the form structure. |developer_engagement_metrics| is a bitmask
+  // of |AutofillMetrics::DeveloperEngagementMetric|.
+  static void LogDeveloperEngagementUkm(ukm::UkmService* ukm_service,
+                                        const GURL& url,
+                                        int developer_engagement_metrics);
 
   // Logs the the |ukm_entry_name| with the specified |url| and the specified
   // |metrics|. Returns whether the ukm was sucessfully logged.
@@ -886,6 +886,8 @@ class AutofillMetrics {
   };
 
  private:
+  static const int kNumCardUploadDecisionMetrics = 9;
+
   DISALLOW_IMPLICIT_CONSTRUCTORS(AutofillMetrics);
 };