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

components/autofill/core/browser/autofill_manager.cc

 // Copyright 2013 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/autofill/core/browser/autofill_manager.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 24 matching lines @@
 #include "build/build_config.h"
 #include "components/autofill/core/browser/autocomplete_history_manager.h"
 #include "components/autofill/core/browser/autofill_client.h"
 #include "components/autofill/core/browser/autofill_data_model.h"
 #include "components/autofill/core/browser/autofill_experiments.h"
 #include "components/autofill/core/browser/autofill_external_delegate.h"
 #include "components/autofill/core/browser/autofill_field.h"
 #include "components/autofill/core/browser/autofill_manager_test_delegate.h"
 #include "components/autofill/core/browser/autofill_metrics.h"
 #include "components/autofill/core/browser/autofill_profile.h"
+#include "components/autofill/core/browser/autofill_profile_comparator.h"
 #include "components/autofill/core/browser/autofill_type.h"
 #include "components/autofill/core/browser/country_names.h"
 #include "components/autofill/core/browser/credit_card.h"
 #include "components/autofill/core/browser/field_types.h"
 #include "components/autofill/core/browser/form_structure.h"
 #include "components/autofill/core/browser/personal_data_manager.h"
 #include "components/autofill/core/browser/phone_number.h"
 #include "components/autofill/core/browser/phone_number_i18n.h"
 #include "components/autofill/core/browser/popup_item_ids.h"
 #include "components/autofill/core/browser/ui/save_card_bubble_controller.h"
@@ skipping 979 matching lines @@
 void AutofillManager::OnDidGetRealPan(AutofillClient::PaymentsRpcResult result,
                                       const std::string& real_pan) {
   DCHECK(full_card_request_);
   full_card_request_->OnDidGetRealPan(result, real_pan);
 }
 
 void AutofillManager::OnDidGetUploadDetails(
     AutofillClient::PaymentsRpcResult result,
     const base::string16& context_token,
     std::unique_ptr<base::DictionaryValue> legal_message) {
-  // TODO(jdonnelly): Log duration.
+  int card_upload_decision_metrics;
   if (result == AutofillClient::SUCCESS) {
     // Do *not* call payments_client_->Prepare() here. We shouldn't send
     // credentials until the user has explicitly accepted a prompt to upload.
     upload_request_.context_token = context_token;
     user_did_accept_upload_prompt_ = false;
     client_->ConfirmSaveCreditCardToCloud(
         upload_request_.card, std::move(legal_message),
         should_cvc_be_requested_,
         base::Bind(&AutofillManager::OnUserDidAcceptUpload,
                    weak_ptr_factory_.GetWeakPtr()));
     client_->LoadRiskData(base::Bind(&AutofillManager::OnDidGetUploadRiskData,
                                      weak_ptr_factory_.GetWeakPtr()));
-    AutofillMetrics::CardUploadDecisionMetric card_upload_decision_metric =
-        should_cvc_be_requested_ ? AutofillMetrics::UPLOAD_OFFERED_NO_CVC
-                                 : AutofillMetrics::UPLOAD_OFFERED;
-    AutofillMetrics::LogCardUploadDecisionMetric(card_upload_decision_metric);
-    LogCardUploadDecisionUkm(card_upload_decision_metric);
+    AutofillMetrics::LogCardUploadDecisionMetric(
+        AutofillMetrics::UPLOAD_OFFERED);
+    card_upload_decision_metrics = 1 << AutofillMetrics::UPLOAD_OFFERED;

[sebsg, 2017/05/01 02:46:20]

For bitmasks like this, I usually prefer when they are defined with their bit
value. Since these enum values are only used in this context, I think it would
be possible.

For example, you could have upload offered be 0, so as soon as something else is
set, you know it was not offered.

Here is an example of when I use it, for the syntax. The "Event" enum in
JourneyLogger:
https://cs.chromium.org/chromium/src/components/payments/core/journey_logger....

[csashi, 2017/05/01 16:01:56]

Can you confirm that I can change the enum values even though these values are
being used in prior Chrome releases for UMA logging? Granted, I am logging more
metrics in the new release that we did not log before but they are the same
values.

For Card Upload Decision metric, we cannot conclude that if value != 0, then
upload was not offered. This is because we sometimes offer upload even if some
fields are missing (currently only CVC, but can be expanded).

[sebsg, 2017/05/01 16:10:17]

Ah I see, didn't know that you could offer upload in those cases too. 

And for the UMA, I think it would have to be a new histogram for the reasons you
mentioned, sorry for the confusion.

Creating a new one and deprecating the old one is not ideal, but I feel like the
new way gives us much more valuable information so I think it's worth it. you
will still be able to see the old data if you by accessing the old histogram. 

This will also allow you to change the enum values since they will only be used
in the new UMA and UKM way of logging.

Thanks you for your hard work.

+    if (should_cvc_be_requested_) {
+      AutofillMetrics::LogCardUploadDecisionMetric(
+          AutofillMetrics::UPLOAD_OFFERED_NO_CVC);
+      card_upload_decision_metrics |= 1
+                                      << AutofillMetrics::UPLOAD_OFFERED_NO_CVC;
+    }
   } else {
     // If the upload details request failed, fall back to a local save. The
     // reasoning here is as follows:
     // - This will sometimes fail intermittently, in which case it might be
     // better to not fall back, because sometimes offering upload and sometimes
     // offering local save is a poor user experience.
     // - However, in some cases, our local configuration limiting the feature to
     // countries that Payments is known to support will not match Payments' own
     // determination of what country the user is located in. In these cases,
     // the upload details request will consistently fail and if we don't fall
     // back to a local save then the user will never be offered any kind of
     // credit card save.
     client_->ConfirmSaveCreditCardLocally(
         upload_request_.card,
         base::Bind(
             base::IgnoreResult(&PersonalDataManager::SaveImportedCreditCard),
             base::Unretained(personal_data_), upload_request_.card));
     AutofillMetrics::LogCardUploadDecisionMetric(
+        AutofillMetrics::UPLOAD_NOT_OFFERED);
+    AutofillMetrics::LogCardUploadDecisionMetric(
         AutofillMetrics::UPLOAD_NOT_OFFERED_GET_UPLOAD_DETAILS_FAILED);
-    LogCardUploadDecisionUkm(
-        AutofillMetrics::UPLOAD_NOT_OFFERED_GET_UPLOAD_DETAILS_FAILED);
+    card_upload_decision_metrics =
+        (1 << AutofillMetrics::UPLOAD_NOT_OFFERED_GET_UPLOAD_DETAILS_FAILED) |
+        (1 << AutofillMetrics::UPLOAD_NOT_OFFERED);
   }
+  LogCardUploadDecisionsUkm(card_upload_decision_metrics);
   pending_upload_request_url_ = GURL();
 }
 
 void AutofillManager::OnDidUploadCard(AutofillClient::PaymentsRpcResult result,
                                       const std::string& server_id) {
   // We don't do anything user-visible if the upload attempt fails.
   // If the upload succeeds, we will keep a copy of the card as a full server
   // card on the device.
   if (result == AutofillClient::SUCCESS && !server_id.empty()) {
     upload_request_.card.set_record_type(CreditCard::FULL_SERVER_CARD);
@@ skipping 156 matching lines @@
       if (field->Type().GetStorableType() == CREDIT_CARD_VERIFICATION_CODE &&
           IsValidCreditCardSecurityCode(field->value,
                                         upload_request_.card.type())) {
         upload_request_.cvc = field->value;
         break;
       }
     }
 
     // Upload requires that recently used or modified addresses meet the
     // client-side validation rules.
-    autofill::AutofillMetrics::CardUploadDecisionMetric
-        get_profiles_decision_metric = AutofillMetrics::UPLOAD_OFFERED;
     std::string rappor_metric_name;
-    bool get_profiles_succeeded =
-        GetProfilesForCreditCardUpload(*imported_credit_card,
-                                       &upload_request_.profiles,
-                                       &get_profiles_decision_metric,
-                                       &rappor_metric_name);
+    int upload_decision_metrics = GetProfilesForCreditCardUpload(
+        *imported_credit_card, &upload_request_.profiles, &rappor_metric_name);
 
     pending_upload_request_url_ = GURL(submitted_form.source_url());
 
     // Both the CVC and address checks are done.  Conform to the legacy order of
     // reporting on CVC then address.
     should_cvc_be_requested_ = false;
     if (upload_request_.cvc.empty()) {
-      if (IsAutofillUpstreamRequestCvcIfMissingExperimentEnabled()) {
-        should_cvc_be_requested_ = true;
-      } else {
+      should_cvc_be_requested_ =
+          (!upload_decision_metrics &&
+           IsAutofillUpstreamRequestCvcIfMissingExperimentEnabled());
+      if (!should_cvc_be_requested_) {
         AutofillMetrics::LogCardUploadDecisionMetric(
             AutofillMetrics::UPLOAD_NOT_OFFERED_NO_CVC);
-        LogCardUploadDecisionUkm(AutofillMetrics::UPLOAD_NOT_OFFERED_NO_CVC);
-        pending_upload_request_url_ = GURL();
-        CollectRapportSample(submitted_form.source_url(),
-                             "Autofill.CardUploadNotOfferedNoCvc");
-        return;
+        upload_decision_metrics |=
+            (1 << AutofillMetrics::UPLOAD_NOT_OFFERED_NO_CVC);
+        rappor_metric_name = "Autofill.CardUploadNotOfferedNoCvc";
       }
     }
-    if (!get_profiles_succeeded) {
-      DCHECK(get_profiles_decision_metric != AutofillMetrics::UPLOAD_OFFERED);
+    if (upload_decision_metrics) {
       AutofillMetrics::LogCardUploadDecisionMetric(
-          get_profiles_decision_metric);
-      LogCardUploadDecisionUkm(get_profiles_decision_metric);
+          AutofillMetrics::UPLOAD_NOT_OFFERED);
+      upload_decision_metrics |= 1 << AutofillMetrics::UPLOAD_NOT_OFFERED;
+      LogCardUploadDecisionsUkm(upload_decision_metrics);
       pending_upload_request_url_ = GURL();
       if (!rappor_metric_name.empty()) {
-        CollectRapportSample(submitted_form.source_url(), rappor_metric_name);
+        CollectRapporSample(submitted_form.source_url(), rappor_metric_name);
       }
       return;
     }
 
     // All required data is available, start the upload process.
     payments_client_->GetUploadDetails(upload_request_.profiles, app_locale_);
   }
 }
 
-bool AutofillManager::GetProfilesForCreditCardUpload(
+int AutofillManager::GetProfilesForCreditCardUpload(
     const CreditCard& card,
     std::vector<AutofillProfile>* profiles,
-    autofill::AutofillMetrics::CardUploadDecisionMetric*
-        address_upload_decision_metric,
     std::string* rappor_metric_name) const {
   std::vector<AutofillProfile> candidate_profiles;
   const base::Time now = AutofillClock::Now();
   const base::TimeDelta fifteen_minutes = base::TimeDelta::FromMinutes(15);
+  int upload_decision_metrics = 0;
 
   // First, collect all of the addresses used recently.
   for (AutofillProfile* profile : personal_data_->GetProfiles()) {
     if ((now - profile->use_date()) < fifteen_minutes ||
         (now - profile->modification_date()) < fifteen_minutes) {
       candidate_profiles.push_back(*profile);
     }
   }
   if (candidate_profiles.empty()) {
-    *address_upload_decision_metric =
-        AutofillMetrics::UPLOAD_NOT_OFFERED_NO_ADDRESS;
+    AutofillMetrics::LogCardUploadDecisionMetric(
+        AutofillMetrics::UPLOAD_NOT_OFFERED_NO_ADDRESS);
+    upload_decision_metrics |=
+        (1 << AutofillMetrics::UPLOAD_NOT_OFFERED_NO_ADDRESS);
     *rappor_metric_name = "Autofill.CardUploadNotOfferedNoAddress";
-    return false;
   }
 
   // If any of the names on the card or the addresses don't match (where
   // matching is case insensitive and ignores middle initials if present), the
   // candidate set is invalid. This matches the rules for name matching applied
   // server-side by Google Payments and ensures that we don't send upload
   // requests that are guaranteed to fail.
   base::string16 verified_name;
-  base::string16 card_name =
+  const base::string16 card_name =
       card.GetInfo(AutofillType(CREDIT_CARD_NAME_FULL), app_locale_);
   if (!card_name.empty()) {
     verified_name = RemoveMiddleInitial(card_name);
   }
   for (const AutofillProfile& profile : candidate_profiles) {
-    base::string16 address_name =
+    const base::string16 address_name =
         profile.GetInfo(AutofillType(NAME_FULL), app_locale_);
     if (!address_name.empty()) {
       if (verified_name.empty()) {
         verified_name = RemoveMiddleInitial(address_name);
       } else {
-        // TODO(crbug.com/590307): We only use ASCII case insensitivity here
-        // because the feature is launching US-only and middle initials only
-        // make sense for Western-style names anyway. As we launch in more
-        // countries, we'll need to make the name comparison more sophisticated.
+        // TODO(crbug.com/590307): We'll need to make the name comparison more
+        // sophisticated.
         if (!base::EqualsCaseInsensitiveASCII(
                 verified_name, RemoveMiddleInitial(address_name))) {
-          *address_upload_decision_metric =
-              AutofillMetrics::UPLOAD_NOT_OFFERED_CONFLICTING_NAMES;
-          *rappor_metric_name = "Autofill.CardUploadNotOfferedConflictingNames";
-          return false;
+          if (!upload_decision_metrics)
+            *rappor_metric_name =
+                "Autofill.CardUploadNotOfferedConflictingNames";
+          AutofillMetrics::LogCardUploadDecisionMetric(
+              AutofillMetrics::UPLOAD_NOT_OFFERED_CONFLICTING_NAMES);
+          upload_decision_metrics |=
+              (1 << AutofillMetrics::UPLOAD_NOT_OFFERED_CONFLICTING_NAMES);
+          break;
         }
       }
     }
   }
   // If neither the card nor any of the addresses have a name associated with
   // them, the candidate set is invalid.
   if (verified_name.empty()) {
-    *address_upload_decision_metric =
-        AutofillMetrics::UPLOAD_NOT_OFFERED_NO_NAME;
-    *rappor_metric_name = "Autofill.CardUploadNotOfferedNoName";
-    return false;
+    if (!upload_decision_metrics)
+      *rappor_metric_name = "Autofill.CardUploadNotOfferedNoName";
+    AutofillMetrics::LogCardUploadDecisionMetric(
+        AutofillMetrics::UPLOAD_NOT_OFFERED_NO_NAME);
+    upload_decision_metrics |=
+        (1 << AutofillMetrics::UPLOAD_NOT_OFFERED_NO_NAME);
   }
 
   // If any of the candidate addresses have a non-empty zip that doesn't match
   // any other non-empty zip, then the candidate set is invalid.
   base::string16 verified_zip;
   for (const AutofillProfile& profile : candidate_profiles) {
     // TODO(jdonnelly): Use GetInfo instead of GetRawInfo once zip codes are
     // canonicalized. See http://crbug.com/587465.
-    base::string16 zip = profile.GetRawInfo(ADDRESS_HOME_ZIP);
+    const base::string16 zip = profile.GetRawInfo(ADDRESS_HOME_ZIP);
     if (!zip.empty()) {
       if (verified_zip.empty()) {
         verified_zip = zip;
       } else {
         // To compare two zips, we check to see if either is a prefix of the
         // other. This allows us to consider a 5-digit zip and a zip+4 to be a
         // match if the first 5 digits are the same without hardcoding any
         // specifics of how postal codes are represented. (They can be numeric
         // or alphanumeric and vary from 3 to 10 digits long by country. See
         // https://en.wikipedia.org/wiki/Postal_code#Presentation.) The Payments
         // backend will apply a more sophisticated address-matching procedure.
         // This check is simply meant to avoid offering upload in cases that are
         // likely to fail.
         if (!(StartsWith(verified_zip, zip, base::CompareCase::SENSITIVE) ||
               StartsWith(zip, verified_zip, base::CompareCase::SENSITIVE))) {
-          *address_upload_decision_metric =
-              AutofillMetrics::UPLOAD_NOT_OFFERED_CONFLICTING_ZIPS;
-          return false;
+          AutofillMetrics::LogCardUploadDecisionMetric(
+              AutofillMetrics::UPLOAD_NOT_OFFERED_CONFLICTING_ZIPS);
+          upload_decision_metrics |=
+              (1 << AutofillMetrics::UPLOAD_NOT_OFFERED_CONFLICTING_ZIPS);
+          break;
         }
       }
     }
   }
 
   // If none of the candidate addresses have a zip, the candidate set is
   // invalid.
   if (verified_zip.empty()) {
-    *address_upload_decision_metric =
-        AutofillMetrics::UPLOAD_NOT_OFFERED_NO_ZIP_CODE;
-    return false;
+    AutofillMetrics::LogCardUploadDecisionMetric(
+        AutofillMetrics::UPLOAD_NOT_OFFERED_NO_ZIP_CODE);
+    upload_decision_metrics |=
+        (1 << AutofillMetrics::UPLOAD_NOT_OFFERED_NO_ZIP_CODE);
   }
 
-  profiles->assign(candidate_profiles.begin(), candidate_profiles.end());
-  return true;
+  if (!upload_decision_metrics)
+    profiles->assign(candidate_profiles.begin(), candidate_profiles.end());
+
+  return upload_decision_metrics;
 }
 
-void AutofillManager::CollectRapportSample(const GURL& source_url,
-                                           const std::string& metric_name)
-    const {
+void AutofillManager::CollectRapporSample(
+    const GURL& source_url,
+    const std::string& metric_name) const {
   if (source_url.is_valid() && client_->GetRapporServiceImpl()) {
     rappor::SampleDomainAndRegistryFromGURL(client_->GetRapporServiceImpl(),
                                             metric_name, source_url);
   }
 }
 
 // Note that |submitted_form| is passed as a pointer rather than as a reference
 // so that we can get memory management right across threads.  Note also that we
 // explicitly pass in all the time stamps of interest, as the cached ones might
 // get reset before this method executes.
@@ skipping 831 matching lines @@
     }
     if (i > 0)
       fputs("Next oldest form:\n", file);
   }
   fputs("\n", file);
 
   fclose(file);
 }
 #endif  // ENABLE_FORM_DEBUG_DUMP
 
-void AutofillManager::LogCardUploadDecisionUkm(
-    AutofillMetrics::CardUploadDecisionMetric upload_decision) {
-  AutofillMetrics::LogCardUploadDecisionUkm(
-      client_->GetUkmService(), pending_upload_request_url_, upload_decision);
+void AutofillManager::LogCardUploadDecisionsUkm(int upload_decision_metrics) {
+  AutofillMetrics::LogCardUploadDecisionsUkm(client_->GetUkmService(),
+                                             pending_upload_request_url_,
+                                             upload_decision_metrics);
 }
 
 }  // namespace autofill