[Payments] Move address ellision to after GetDetectedValues is computed.

A previous change misplaced the code to remove all information in addresses. It
is now moved to after GetDetectedValues is computed.

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

components/autofill/core/browser/credit_card_save_manager.cc

@@ -167,6 +167,20 @@ void CreditCardSaveManager::AttemptToOfferCardUploadSave(
     should_request_name_from_user_ = true;
   }
 
+  // If the relevant feature is enabled, only send the country of the
+  // recently-used addresses.
+  if (base::FeatureList::IsEnabled(
+          features::kAutofillSendOnlyCountryInGetUploadDetails)) {
+    for (size_t i = 0; i < upload_request_.profiles.size(); i++) {
+      AutofillProfile country_only;
+      country_only.SetInfo(ADDRESS_HOME_COUNTRY,
+                           upload_request_.profiles[i].GetInfo(
+                               ADDRESS_HOME_COUNTRY, app_locale_),
+                           app_locale_);
+      upload_request_.profiles[i] = std::move(country_only);
+    }
+  }
+
   // All required data is available, start the upload process.
   if (observer_for_testing_)
     observer_for_testing_->OnDecideToRequestUploadSave();
@@ -381,20 +395,6 @@ void CreditCardSaveManager::SetProfilesForCreditCardUpload(
   if (verified_zip.empty() && !candidate_profiles.empty())
     upload_decision_metrics_ |= AutofillMetrics::UPLOAD_NOT_OFFERED_NO_ZIP_CODE;
 
-  // If the relevant feature is enabled, only send the country of the
-  // recently-used addresses.
-  if (base::FeatureList::IsEnabled(
-          features::kAutofillSendOnlyCountryInGetUploadDetails)) {
-    for (size_t i = 0; i < candidate_profiles.size(); i++) {
-      AutofillProfile country_only;
-      country_only.SetInfo(
-          ADDRESS_HOME_COUNTRY,
-          candidate_profiles[i].GetInfo(ADDRESS_HOME_COUNTRY, app_locale_),
-          app_locale_);
-      candidate_profiles[i] = std::move(country_only);
-    }
-  }
-
   // Set up |upload_request->profiles|.
   upload_request->profiles.assign(candidate_profiles.begin(),
                                   candidate_profiles.end());

components/autofill/core/browser/credit_card_save_manager_unittest.cc

@@ -2342,7 +2342,28 @@ TEST_F(CreditCardSaveManagerTest, DuplicateMaskedCreditCard_NoUpload) {
   EXPECT_FALSE(credit_card_save_manager_->CreditCardWasUploaded());
 }
 
-TEST_F(CreditCardSaveManagerTest, GetDetectedValues_NothingIfNothingFound) {
+// This class is parametrized to allow running all the inheriting tests with and
+// without a specific feature enabled. See INSTANTIATE_TEST_CASE_P.
+class CreditCardSaveManagerGetDetectedValuesTest
+    : public CreditCardSaveManagerTest,
+      public ::testing::WithParamInterface<
+          /*enable_send_only_country_in_get_upload_details=*/bool> {
+ public:
+  CreditCardSaveManagerGetDetectedValuesTest() {}
+  ~CreditCardSaveManagerGetDetectedValuesTest() override {}
+
+  void SetUp() override {
+    CreditCardSaveManagerTest::SetUp();
+    scoped_feature_list_.InitWithFeatureState(
+        features::kAutofillSendOnlyCountryInGetUploadDetails,
+        /*enable_send_only_country_in_get_upload_details=*/GetParam());
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(CreditCardSaveManagerGetDetectedValuesTest);
+};
+
+TEST_P(CreditCardSaveManagerGetDetectedValuesTest, NothingIfNothingFound) {
   // Set up our credit card form data.
   FormData credit_card_form;
   CreateTestCreditCardFormData(&credit_card_form, true, false);
@@ -2360,7 +2381,7 @@ TEST_F(CreditCardSaveManagerTest, GetDetectedValues_NothingIfNothingFound) {
   EXPECT_EQ(payments_client_->detected_values_in_upload_details(), 0);
 }
 
-TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectCvc) {
+TEST_P(CreditCardSaveManagerGetDetectedValuesTest, DetectCvc) {
   // Set up our credit card form data.
   FormData credit_card_form;
   CreateTestCreditCardFormData(&credit_card_form, true, false);
@@ -2382,7 +2403,7 @@ TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectCvc) {
             expected_detected_value);
 }
 
-TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectCardholderName) {
+TEST_P(CreditCardSaveManagerGetDetectedValuesTest, DetectCardholderName) {
   // Set up our credit card form data.
   FormData credit_card_form;
   CreateTestCreditCardFormData(&credit_card_form, true, false);
@@ -2405,7 +2426,7 @@ TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectCardholderName) {
             expected_detected_value);
 }
 
-TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectAddressName) {
+TEST_P(CreditCardSaveManagerGetDetectedValuesTest, DetectAddressName) {
   // Set up a new address profile.
   AutofillProfile profile;
   profile.set_guid("00000000-0000-0000-0000-000000000200");
@@ -2434,8 +2455,8 @@ TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectAddressName) {
             expected_detected_value);
 }
 
-TEST_F(CreditCardSaveManagerTest,
-       GetDetectedValues_DetectCardholderAndAddressNameIfMatching) {
+TEST_P(CreditCardSaveManagerGetDetectedValuesTest,
+       DetectCardholderAndAddressNameIfMatching) {
   // Set up a new address profile.
   AutofillProfile profile;
   profile.set_guid("00000000-0000-0000-0000-000000000200");
@@ -2465,8 +2486,8 @@ TEST_F(CreditCardSaveManagerTest,
             expected_detected_values);
 }
 
-TEST_F(CreditCardSaveManagerTest,
-       GetDetectedValues_DetectNoUniqueNameIfNamesConflict) {
+TEST_P(CreditCardSaveManagerGetDetectedValuesTest,
+       DetectNoUniqueNameIfNamesConflict) {
   // Set up a new address profile.
   AutofillProfile profile;
   profile.set_guid("00000000-0000-0000-0000-000000000200");
@@ -2490,7 +2511,7 @@ TEST_F(CreditCardSaveManagerTest,
   EXPECT_EQ(payments_client_->detected_values_in_upload_details(), 0);
 }
 
-TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectPostalCode) {
+TEST_P(CreditCardSaveManagerGetDetectedValuesTest, DetectPostalCode) {
   // Set up a new address profile.
   AutofillProfile profile;
   profile.set_guid("00000000-0000-0000-0000-000000000200");
@@ -2519,8 +2540,8 @@ TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectPostalCode) {
             expected_detected_value);
 }
 
-TEST_F(CreditCardSaveManagerTest,
-       GetDetectedValues_DetectNoUniquePostalCodeIfZipsConflict) {
+TEST_P(CreditCardSaveManagerGetDetectedValuesTest,
+       DetectNoUniquePostalCodeIfZipsConflict) {
   // Set up two new address profiles with conflicting postal codes.
   AutofillProfile profile1;
   profile1.set_guid("00000000-0000-0000-0000-000000000200");
@@ -2548,7 +2569,7 @@ TEST_F(CreditCardSaveManagerTest,
   EXPECT_EQ(payments_client_->detected_values_in_upload_details(), 0);
 }
 
-TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectAddressLine) {
+TEST_P(CreditCardSaveManagerGetDetectedValuesTest, DetectAddressLine) {
   // Set up a new address profile.
   AutofillProfile profile;
   profile.set_guid("00000000-0000-0000-0000-000000000200");
@@ -2577,7 +2598,7 @@ TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectAddressLine) {
             expected_detected_value);
 }
 
-TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectLocality) {
+TEST_P(CreditCardSaveManagerGetDetectedValuesTest, DetectLocality) {
   // Set up a new address profile.
   AutofillProfile profile;
   profile.set_guid("00000000-0000-0000-0000-000000000200");
@@ -2605,7 +2626,7 @@ TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectLocality) {
             expected_detected_value);
 }
 
-TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectAdministrativeArea) {
+TEST_P(CreditCardSaveManagerGetDetectedValuesTest, DetectAdministrativeArea) {
   // Set up a new address profile.
   AutofillProfile profile;
   profile.set_guid("00000000-0000-0000-0000-000000000200");
@@ -2634,7 +2655,7 @@ TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectAdministrativeArea) {
             expected_detected_value);
 }
 
-TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectCountryCode) {
+TEST_P(CreditCardSaveManagerGetDetectedValuesTest, DetectCountryCode) {
   // Set up a new address profile.
   AutofillProfile profile;
   profile.set_guid("00000000-0000-0000-0000-000000000200");
@@ -2663,8 +2684,8 @@ TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectCountryCode) {
             expected_detected_value);
 }
 
-TEST_F(CreditCardSaveManagerTest,
-       GetDetectedValues_DetectHasGooglePaymentAccount) {
+TEST_P(CreditCardSaveManagerGetDetectedValuesTest,
+       DetectHasGooglePaymentAccount) {
   // Set the billing_customer_number Priority Preference to designate existence
   // of a Payments account.
   autofill_client_.GetPrefs()->SetDouble(prefs::kAutofillBillingCustomerNumber,
@@ -2692,7 +2713,7 @@ TEST_F(CreditCardSaveManagerTest,
             expected_detected_value);
 }
 
-TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectEverythingAtOnce) {
+TEST_P(CreditCardSaveManagerGetDetectedValuesTest, DetectEverythingAtOnce) {
   // Set up a new address profile.
   AutofillProfile profile;
   profile.set_guid("00000000-0000-0000-0000-000000000200");
@@ -2733,8 +2754,8 @@ TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectEverythingAtOnce) {
             expected_detected_values);
 }
 
-TEST_F(CreditCardSaveManagerTest,
-       GetDetectedValues_DetectSubsetOfPossibleFields) {
+TEST_P(CreditCardSaveManagerGetDetectedValuesTest,
+       DetectSubsetOfPossibleFields) {
   // Set up a new address profile, taking out address line and state.
   AutofillProfile profile;
   profile.set_guid("00000000-0000-0000-0000-000000000200");
@@ -2772,8 +2793,8 @@ TEST_F(CreditCardSaveManagerTest,
 // This test checks that ADDRESS_LINE, LOCALITY, ADMINISTRATIVE_AREA, and
 // COUNTRY_CODE don't care about possible conflicts or consistency and are
 // populated if even one address profile contains it.
-TEST_F(CreditCardSaveManagerTest,
-       GetDetectedValues_DetectAddressComponentsAcrossProfiles) {
+TEST_P(CreditCardSaveManagerGetDetectedValuesTest,
+       DetectAddressComponentsAcrossProfiles) {
   // Set up four new address profiles, each with a different address component.
   AutofillProfile profile1;
   profile1.set_guid("00000000-0000-0000-0000-000000000200");
@@ -2818,6 +2839,15 @@ TEST_F(CreditCardSaveManagerTest,
             expected_detected_values);
 }
 
+// Every test will appear with suffix /0 (param false) and /1 (param true), e.g.
+//  CreditCardSaveManagerGetDetectedValuesTest.NothingIfNothingFound/0:
+//    Feature disabled
+//  CreditCardSaveManagerGetDetectedValuesTest.NothingIfNothingFound/1:
+//    Feature enabled.
+INSTANTIATE_TEST_CASE_P(,  // Empty instatiation name.
+                        CreditCardSaveManagerGetDetectedValuesTest,
+                        ::testing::Values(false, true));
+
 TEST_F(CreditCardSaveManagerTest,
        UploadCreditCard_LogAdditionalErrorsWithUploadDetailsFailure) {
   // Anything other than "en-US" will cause GetUploadDetails to return a failure