Skip to content

Commit

Permalink
[Payments] Move address ellision to after GetDetectedValues is computed.
Browse files Browse the repository at this point in the history 
A previous change misplaced the code to remove all information in addresses. It
is now moved to after GetDetectedValues is computed.

TBR=mathp@chromium.org

(cherry picked from commit 5e05592)

Bug: 878416
Change-Id: I15fd8d8c97d2a051aab9313be98fff5a840fcbd9
Reviewed-on: https://chromium-review.googlesource.com/1207193
Reviewed-by: Jared Saul <jsaul@google.com>
Commit-Queue: Mathieu Perreault <mathp@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#588951}
Reviewed-on: https://chromium-review.googlesource.com/1211503
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#93}
Cr-Branched-From: 79f7c91-refs/heads/master@{#587811}
  • Loading branch information
Mathieu Perreault committed Sep 6, 2018
1 parent 6cabf66 commit 9756e17
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 36 deletions.
28 changes: 14 additions & 14 deletions components/autofill/core/browser/credit_card_save_manager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,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();
Expand Down Expand Up @@ -387,20 +401,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());
Expand Down
74 changes: 52 additions & 22 deletions components/autofill/core/browser/credit_card_save_manager_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2347,7 +2347,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);
Expand All @@ -2365,7 +2386,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);
Expand All @@ -2384,7 +2405,7 @@ TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectCvc) {
CreditCardSaveManager::DetectedValue::CVC);
}

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);
Expand All @@ -2403,7 +2424,7 @@ TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectCardholderName) {
CreditCardSaveManager::DetectedValue::CARDHOLDER_NAME);
}

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");
Expand All @@ -2428,8 +2449,8 @@ TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectAddressName) {
CreditCardSaveManager::DetectedValue::ADDRESS_NAME);
}

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");
Expand All @@ -2455,8 +2476,8 @@ TEST_F(CreditCardSaveManagerTest,
CreditCardSaveManager::DetectedValue::ADDRESS_NAME);
}

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");
Expand All @@ -2480,7 +2501,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");
Expand All @@ -2505,8 +2526,8 @@ TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectPostalCode) {
CreditCardSaveManager::DetectedValue::POSTAL_CODE);
}

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");
Expand Down Expand Up @@ -2534,7 +2555,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");
Expand All @@ -2559,7 +2580,7 @@ TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectAddressLine) {
CreditCardSaveManager::DetectedValue::ADDRESS_LINE);
}

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");
Expand All @@ -2584,7 +2605,7 @@ TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectLocality) {
CreditCardSaveManager::DetectedValue::LOCALITY);
}

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");
Expand All @@ -2609,7 +2630,7 @@ TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectAdministrativeArea) {
CreditCardSaveManager::DetectedValue::ADMINISTRATIVE_AREA);
}

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");
Expand All @@ -2634,8 +2655,8 @@ TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectCountryCode) {
CreditCardSaveManager::DetectedValue::COUNTRY_CODE);
}

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,
Expand All @@ -2659,7 +2680,7 @@ TEST_F(CreditCardSaveManagerTest,
CreditCardSaveManager::DetectedValue::HAS_GOOGLE_PAYMENTS_ACCOUNT);
}

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");
Expand Down Expand Up @@ -2696,8 +2717,8 @@ TEST_F(CreditCardSaveManagerTest, GetDetectedValues_DetectEverythingAtOnce) {
CreditCardSaveManager::DetectedValue::COUNTRY_CODE);
}

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");
Expand Down Expand Up @@ -2731,8 +2752,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");
Expand Down Expand Up @@ -2773,6 +2794,15 @@ TEST_F(CreditCardSaveManagerTest,
CreditCardSaveManager::DetectedValue::COUNTRY_CODE);
}

// 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
Expand Down

0 comments on commit 9756e17

Please sign in to comment.