Credit card upload save always asks Google payments if save is possible,...

Credit card upload save always asks Google payments if save is possible, provides metadata on detected values

Bug: 789645
Change-Id: I4c4709d9a4bd08134893dc64aa5a07e188bc2388
Reviewed-on: https://chromium-review.googlesource.com/816078
Commit-Queue: Jared Saul <jsaul@google.com>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523976}

chrome/browser/about_flags.cc

@@ -2940,6 +2940,11 @@ const FeatureEntry kFeatureEntries[] = {
      flag_descriptions::kEnableAutofillCreditCardUploadCvcPromptDescription,
      kOsDesktop,
      FEATURE_VALUE_TYPE(autofill::kAutofillUpstreamRequestCvcIfMissing)},
+    {"enable-autofill-credit-card-upload-send-detected-values",
+     flag_descriptions::kEnableAutofillCreditCardUploadSendDetectedValuesName,
+     flag_descriptions::
+         kEnableAutofillCreditCardUploadSendDetectedValuesDescription,
+     kOsAll, FEATURE_VALUE_TYPE(autofill::kAutofillUpstreamSendDetectedValues)},
     {"enable-autofill-credit-card-upload-send-pan-first-six",
      flag_descriptions::kEnableAutofillCreditCardUploadSendPanFirstSixName,
      flag_descriptions::

chrome/browser/flag_descriptions.cc

@@ -287,6 +287,13 @@ const char kEnableAutofillCreditCardUploadCvcPromptDescription[] =
     "If enabled, requests missing CVC when offering to upload credit cards to "
     "Google Payments.";
 
+const char kEnableAutofillCreditCardUploadSendDetectedValuesName[] =
+    "Always send metadata on detected form values for Autofill credit card "
+    "upload";
+const char kEnableAutofillCreditCardUploadSendDetectedValuesDescription[] =
+    "If enabled, always checks with Google Payments when deciding whether to "
+    "offer credit card upload, even if some data is missing.";
+
 const char kEnableAutofillCreditCardUploadSendPanFirstSixName[] =
     "Send first six digits of PAN when deciding whether to offer Autofill "
     "credit card upload";

chrome/browser/flag_descriptions.h

@@ -204,6 +204,10 @@ extern const char kEnableAutofillCreditCardLastUsedDateDisplayDescription[];
 extern const char kEnableAutofillCreditCardUploadCvcPromptName[];
 extern const char kEnableAutofillCreditCardUploadCvcPromptDescription[];
 
+extern const char kEnableAutofillCreditCardUploadSendDetectedValuesName[];
+extern const char
+    kEnableAutofillCreditCardUploadSendDetectedValuesDescription[];
+
 extern const char kEnableAutofillCreditCardUploadSendPanFirstSixName[];
 extern const char kEnableAutofillCreditCardUploadSendPanFirstSixDescription[];
 

components/autofill/core/browser/autofill_experiments.cc

@@ -65,6 +65,8 @@ const base::Feature kAutofillUpstreamAllowAllEmailDomains{
     "AutofillUpstreamAllowAllEmailDomains", base::FEATURE_DISABLED_BY_DEFAULT};
 const base::Feature kAutofillUpstreamRequestCvcIfMissing{
     "AutofillUpstreamRequestCvcIfMissing", base::FEATURE_DISABLED_BY_DEFAULT};
+const base::Feature kAutofillUpstreamSendDetectedValues{
+    "AutofillUpstreamSendDetectedValues", base::FEATURE_DISABLED_BY_DEFAULT};
 const base::Feature kAutofillUpstreamSendPanFirstSix{
     "AutofillUpstreamSendPanFirstSix", base::FEATURE_DISABLED_BY_DEFAULT};
 const base::Feature kAutofillUpstreamUseAutofillProfileComparator{
@@ -291,6 +293,10 @@ bool IsAutofillUpstreamRequestCvcIfMissingExperimentEnabled() {
 #endif
 }
 
+bool IsAutofillUpstreamSendDetectedValuesExperimentEnabled() {
+  return base::FeatureList::IsEnabled(kAutofillUpstreamSendDetectedValues);
+}
+
 bool IsAutofillUpstreamSendPanFirstSixExperimentEnabled() {
   return base::FeatureList::IsEnabled(kAutofillUpstreamSendPanFirstSix);
 }

components/autofill/core/browser/autofill_experiments.h

@@ -44,6 +44,7 @@ extern const base::Feature kAutofillSuppressDisusedCreditCards;
 extern const base::Feature kAutofillToolkitViewsCreditCardDialogsMac;
 extern const base::Feature kAutofillUpstreamAllowAllEmailDomains;
 extern const base::Feature kAutofillUpstreamRequestCvcIfMissing;
+extern const base::Feature kAutofillUpstreamSendDetectedValues;
 extern const base::Feature kAutofillUpstreamSendPanFirstSix;
 extern const base::Feature kAutofillUpstreamUseAutofillProfileComparator;
 extern const char kCreditCardSigninPromoImpressionLimitParamKey[];
@@ -140,6 +141,12 @@ bool IsAutofillSendBillingCustomerNumberExperimentEnabled();
 // in the offer to save bubble if it was not detected during the checkout flow.
 bool IsAutofillUpstreamRequestCvcIfMissingExperimentEnabled();
 
+// Returns whether the experiment is enabled where Chrome Upstream always checks
+// to see if it can offer to save (even though some data like name, address, and
+// CVC might be missing) by sending metadata on what form values were detected
+// along with whether the user is a Google Payments customer.
+bool IsAutofillUpstreamSendDetectedValuesExperimentEnabled();
+
 // Returns whether the experiment is enabled where Chrome Upstream sends the
 // first six digits of the card PAN to Google Payments to help determine whether
 // card upload is possible.

components/autofill/core/browser/credit_card_save_manager.h

@@ -26,6 +26,36 @@ namespace autofill {
 // save logic.  Owned by FormDataImporter.
 class CreditCardSaveManager : public payments::PaymentsClientSaveDelegate {
  public:
+  // Possible fields and values detected during credit card form submission, to
+  // be sent to Google Payments to better determine if upload credit card save
+  // should be offered.  These should stay consistent with the equivalent enum
+  // in Google Payments code.
+  enum DetectedValue {
+    // Set if a valid CVC was detected.  Will always be set if the CVC fix flow
+    // is enabled.
+    CVC = 1 << 0,
+    // Set if a cardholder name was found, *unless* conflicting names were
+    // found.
+    CARDHOLDER_NAME = 1 << 1,
+    // Set if an address name was found, *unless* conflicting names were found.
+    ADDRESS_NAME = 1 << 2,
+    // Set if an address line was found in any address (regardless of
+    // conflicts).
+    ADDRESS_LINE = 1 << 3,
+    // Set if a locality was found in any address (regardless of conflicts).
+    LOCALITY = 1 << 4,
+    // Set if an administrative area was found in any address (regardless of
+    // conflicts).
+    ADMINISTRATIVE_AREA = 1 << 5,
+    // Set if a postal code was found in any address, *unless* conflicting
+    // postal codes were found.
+    POSTAL_CODE = 1 << 6,
+    // Set if a country code was found in any address (regardless of conflicts).
+    COUNTRY_CODE = 1 << 7,
+    // Set if the user is already syncing data from a Google Payments account.
+    HAS_GOOGLE_PAYMENTS_ACCOUNT = 1 << 8,
+  };
+
   // The parameters should outlive the CreditCardSaveManager.
   CreditCardSaveManager(AutofillClient* client,
                         payments::PaymentsClient* payments_client,
@@ -63,13 +93,18 @@ class CreditCardSaveManager : public payments::PaymentsClientSaveDelegate {
 
   // Examines |card| and the stored profiles and if a candidate set of profiles
   // is found that matches the client-side validation rules, assigns the values
-  // to |upload_request.profiles| and returns 0. If no valid set can be found,
-  // returns the failure reasons. Appends any experiments that were triggered to
-  // |upload_request.active_experiments|. The return value is a bitmask of
-  // |AutofillMetrics::CardUploadDecisionMetric|.
-  int SetProfilesForCreditCardUpload(
+  // to |upload_request.profiles|. If any problems are found when determining
+  // the candidate set of profiles, sets |upload_decision_metrics_| with the
+  // failure reasons. Appends any experiments that were triggered to
+  // |upload_request.active_experiments|.
+  void SetProfilesForCreditCardUpload(
       const CreditCard& card,
-      payments::PaymentsClient::UploadRequestDetails* upload_request) const;
+      payments::PaymentsClient::UploadRequestDetails* upload_request);
+
+  // Analyzes the decisions made while importing address profile and credit card
+  // data in preparation for upload credit card save, in order to determine what
+  // uploadable data is actually available.
+  int GetDetectedValues() const;
 
   // Sets |user_did_accept_upload_prompt_| and calls SendUploadCardRequest if
   // the risk data is available.
@@ -109,6 +144,12 @@ class CreditCardSaveManager : public payments::PaymentsClientSaveDelegate {
 
   // Collected information about a pending upload request.
   payments::PaymentsClient::UploadRequestDetails upload_request_;
+
+  // A bitmask of |AutofillMetrics::CardUploadDecisionMetric| representing the
+  // decisions made when determining if credit card upload save should be
+  // offered.
+  int upload_decision_metrics_ = 0;
+
   // |true| if the user has opted to upload save their credit card to Google.
   bool user_did_accept_upload_prompt_ = false;
 

components/autofill/core/browser/form_data_importer.cc

@@ -132,7 +132,8 @@ void FormDataImporter::ImportFormData(const FormStructure& submitted_form,
     // to ImportFormData, this block can be reached on observing either a new
     // card or one already stored locally and whose |TypeAndLastFourDigits| do
     // not match a masked server card. We can offer to upload either kind, but
-    // note that they must pass address/name/CVC validation requirements first.
+    // note that unless the "send detected values" experiment is enabled, they
+    // must pass address/name/CVC validation requirements first.
     credit_card_save_manager_->AttemptToOfferCardUploadSave(
         submitted_form, *imported_credit_card);
   }

components/autofill/core/browser/payments/payments_client.cc

@@ -261,11 +261,13 @@ class UnmaskCardRequest : public PaymentsRequest {
 class GetUploadDetailsRequest : public PaymentsRequest {
  public:
   GetUploadDetailsRequest(const std::vector<AutofillProfile>& addresses,
+                          const int detected_values,
                           const std::string& pan_first_six,
                           const std::vector<const char*>& active_experiments,
                           const std::string& app_locale,
                           PaymentsClientSaveDelegate* delegate)
       : addresses_(addresses),
+        detected_values_(detected_values),
         pan_first_six_(pan_first_six),
         active_experiments_(active_experiments),
         app_locale_(app_locale),
@@ -296,6 +298,13 @@ class GetUploadDetailsRequest : public PaymentsRequest {
     }
     request_dict.Set("address", std::move(addresses));
 
+    // If the "send detected values" experiment is enabled, it's possible we may
+    // not have found name/address/CVC. The detected_values_ bitmask tells
+    // Payments what was found, and Payments will decide if the provided data is
+    // enough to offer upload save.
+    if (IsAutofillUpstreamSendDetectedValuesExperimentEnabled())
+      request_dict.SetInteger("detected_values", detected_values_);
+
     if (IsAutofillUpstreamSendPanFirstSixExperimentEnabled() &&
         !pan_first_six_.empty())
       request_dict.SetString("pan_first6", pan_first_six_);
@@ -326,6 +335,7 @@ class GetUploadDetailsRequest : public PaymentsRequest {
 
  private:
   const std::vector<AutofillProfile> addresses_;
+  const int detected_values_;
   const std::string pan_first_six_;
   const std::vector<const char*> active_experiments_;
   std::string app_locale_;
@@ -484,13 +494,14 @@ void PaymentsClient::UnmaskCard(
 
 void PaymentsClient::GetUploadDetails(
     const std::vector<AutofillProfile>& addresses,
+    const int detected_values,
     const std::string& pan_first_six,
     const std::vector<const char*>& active_experiments,
     const std::string& app_locale) {
   DCHECK(save_delegate_);
   IssueRequest(std::make_unique<GetUploadDetailsRequest>(
-                   addresses, pan_first_six, active_experiments, app_locale,
-                   save_delegate_),
+                   addresses, detected_values, pan_first_six,
+                   active_experiments, app_locale, save_delegate_),
                false);
 }
 

components/autofill/core/browser/payments/payments_client.h

@@ -131,11 +131,15 @@ class PaymentsClient : public net::URLFetcherDelegate,
   // The service uses |addresses| (from which names and phone numbers are
   // removed) and |app_locale| to determine which legal message to display.
   // |pan_first_six| is the first six digits of the number of the credit card
-  // being considered for upload. If the conditions are met, the legal message
-  // will be returned via OnDidGetUploadDetails. |active_experiments| is used by
-  // Payments server to track requests that were triggered by enabled features.
+  // being considered for upload. |detected_values| is a bitmask of
+  // CreditCardSaveManager::DetectedValue values that relays what data is
+  // actually available for upload in order to make more informed upload
+  // decisions. If the conditions are met, the legal message will be returned
+  // via OnDidGetUploadDetails. |active_experiments| is used by Payments server
+  // to track requests that were triggered by enabled features.
   virtual void GetUploadDetails(
       const std::vector<AutofillProfile>& addresses,
+      const int detected_values,
       const std::string& pan_first_six,
       const std::vector<const char*>& active_experiments,
       const std::string& app_locale);

components/autofill/core/browser/payments/payments_client_unittest.cc

@@ -14,6 +14,7 @@
 #include "base/values.h"
 #include "components/autofill/core/browser/autofill_experiments.h"
 #include "components/autofill/core/browser/autofill_test_utils.h"
+#include "components/autofill/core/browser/credit_card_save_manager.h"
 #include "components/autofill/core/browser/payments/payments_client.h"
 #include "components/autofill/core/common/autofill_pref_names.h"
 #include "components/autofill/core/common/autofill_switches.h"
@@ -27,6 +28,20 @@
 
 namespace autofill {
 namespace payments {
+namespace {
+
+int kAllDetectableValues =
+    CreditCardSaveManager::DetectedValue::CVC |
+    CreditCardSaveManager::DetectedValue::CARDHOLDER_NAME |
+    CreditCardSaveManager::DetectedValue::ADDRESS_NAME |
+    CreditCardSaveManager::DetectedValue::ADDRESS_LINE |
+    CreditCardSaveManager::DetectedValue::LOCALITY |
+    CreditCardSaveManager::DetectedValue::ADMINISTRATIVE_AREA |
+    CreditCardSaveManager::DetectedValue::POSTAL_CODE |
+    CreditCardSaveManager::DetectedValue::COUNTRY_CODE |
+    CreditCardSaveManager::DetectedValue::HAS_GOOGLE_PAYMENTS_ACCOUNT;
+
+}  // namespace
 
 class PaymentsClientTest : public testing::Test,
                            public PaymentsClientUnmaskDelegate,
@@ -61,6 +76,11 @@ class PaymentsClientTest : public testing::Test,
         kAutofillSendBillingCustomerNumber);
   }
 
+  void EnableAutofillUpstreamSendDetectedValuesExperiment() {
+    scoped_feature_list_.InitAndEnableFeature(
+        kAutofillUpstreamSendDetectedValues);
+  }
+
   void EnableAutofillUpstreamSendPanFirstSixExperiment() {
     scoped_feature_list_.InitAndEnableFeature(kAutofillUpstreamSendPanFirstSix);
   }
@@ -103,7 +123,8 @@ class PaymentsClientTest : public testing::Test,
   void StartGettingUploadDetails() {
     token_service_->AddAccount("example@gmail.com");
     identity_provider_->LogIn("example@gmail.com");
-    client_->GetUploadDetails(BuildTestProfiles(), /*pan_first_six=*/"411111",
+    client_->GetUploadDetails(BuildTestProfiles(), kAllDetectableValues,
+                              /*pan_first_six=*/"411111",
                               std::vector<const char*>(), "language-LOCALE");
   }
 
@@ -268,6 +289,30 @@ TEST_F(PaymentsClientTest, GetDetailsRemovesNonLocationData) {
   EXPECT_TRUE(GetUploadData().find("0090") == std::string::npos);
 }
 
+TEST_F(PaymentsClientTest,
+       GetDetailsIncludesDetectedValuesInRequestIfExperimentOn) {
+  EnableAutofillUpstreamSendDetectedValuesExperiment();
+
+  StartGettingUploadDetails();
+
+  // Verify that the detected values were included in the request.
+  std::string detected_values_string =
+      "\"detected_values\":" + std::to_string(kAllDetectableValues);
+  EXPECT_TRUE(GetUploadData().find(detected_values_string) !=
+              std::string::npos);
+}
+
+TEST_F(PaymentsClientTest,
+       GetDetailsDoesNotIncludeDetectedValuesInRequestIfExperimentOff) {
+  StartGettingUploadDetails();
+
+  // Verify that the detected values were left out of the request.
+  std::string detected_values_string =
+      "\"detected_values\":" + std::to_string(kAllDetectableValues);
+  EXPECT_TRUE(GetUploadData().find(detected_values_string) ==
+              std::string::npos);
+}
+
 TEST_F(PaymentsClientTest,
        GetDetailsIncludesPanFirstSixInRequestIfExperimentOn) {
   EnableAutofillUpstreamSendPanFirstSixExperiment();

tools/metrics/histograms/enums.xml

@@ -25289,6 +25289,7 @@ from previous Chrome versions.
   <int value="-314910380" label="disable-distance-field-text"/>
   <int value="-314605926" label="protect-sync-credential-on-reauth:enabled"/>
   <int value="-311148335" label="v8-pac-mojo-out-of-process"/>
+  <int value="-305029910" label="AutofillUpstreamSendDetectedValues:disabled"/>
   <int value="-300018686" label="disable-cloud-import"/>
   <int value="-299841473" label="top-document-isolation:enabled"/>
   <int value="-297716805"
@@ -25296,6 +25297,7 @@ from previous Chrome versions.
   <int value="-290672626" label="enable-asm-wasm"/>
   <int value="-288316828" label="enable-delegated-renderer"/>
   <int value="-281844827" label="AutofillCreditCardAblationExperiment:enabled"/>
+  <int value="-280111192" label="AutofillUpstreamSendDetectedValues:enabled"/>
   <int value="-279920685" label="affiliation-based-matching:enabled"/>
   <int value="-279493876" label="WebVRExperimentalRendering:enabled"/>
   <int value="-278347667" label="default-tile-height"/>