Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
When determining what field type to upload, if both PHONE_HOME_CITY_A…
Browse files Browse the repository at this point in the history 
…ND_NUMBER and PHONE_HOME_WHOLE_NUMBER are matched, it means there is no country code in the profile's phone number. In that case, only match PHONE_HOME_CITY_AND_NUMBER because it's more precise.

BUG=580629
TEST=AutofillManagerTest

Review URL: https://codereview.chromium.org/1623803002

Cr-Commit-Position: refs/heads/master@{#371867}
  • Loading branch information
@sebsg
sebsg authored and Commit bot committed Jan 27, 2016
1 parent 16ae028 commit 03c71f5
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 70 deletions.
144 changes: 84 additions & 60 deletions components/autofill/core/browser/autofill_manager_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3276,14 +3276,20 @@ TEST_F(AutofillManagerTest, DeterminePossibleFieldTypesForUpload) {
test::SetProfileInfo(&profile, "Elvis", "Aaron", "Presley",
"theking@gmail.com", "RCA", "3734 Elvis Presley Blvd.",
"Apt. 10", "Memphis", "Tennessee", "38116", "US",
"12345678901");
"+1 (234) 567-8901");
profile.set_guid("00000000-0000-0000-0000-000000000001");
profiles.push_back(profile);
test::SetProfileInfo(&profile, "Charles", "", "Holley", "buddy@gmail.com",
"Decca", "123 Apple St.", "unit 6", "Lubbock", "Texas",
"79401", "US", "23456789012");
"79401", "US", "5142821292");
profile.set_guid("00000000-0000-0000-0000-000000000002");
profiles.push_back(profile);
test::SetProfileInfo(&profile, "Charles", "", "Baudelaire",
"lesfleursdumal@gmail.com", "", "108 Rue Saint-Lazare",
"Apt. 10", "Paris", "Ile de France", "75008", "FR",
"+33 2 49 19 70 70");
profile.set_guid("00000000-0000-0000-0000-000000000001");
profiles.push_back(profile);

// Set up the test credit cards.
std::vector<CreditCard> credit_cards;
Expand All @@ -3299,64 +3305,80 @@ TEST_F(AutofillManagerTest, DeterminePossibleFieldTypesForUpload) {
} TestCase;

TestCase test_cases[] = {
// Profile fields matches.
{"Elvis", NAME_FIRST},
{"Aaron", NAME_MIDDLE},
{"A", NAME_MIDDLE_INITIAL},
{"Presley", NAME_LAST},
{"Elvis Aaron Presley", NAME_FULL},
{"theking@gmail.com", EMAIL_ADDRESS},
{"RCA", COMPANY_NAME},
{"3734 Elvis Presley Blvd.", ADDRESS_HOME_LINE1},
{"Apt. 10", ADDRESS_HOME_LINE2},
{"Memphis", ADDRESS_HOME_CITY},
{"Tennessee", ADDRESS_HOME_STATE},
{"38116", ADDRESS_HOME_ZIP},
{"USA", ADDRESS_HOME_COUNTRY},
{"United States", ADDRESS_HOME_COUNTRY},
{"+1 (234) 567-8901", PHONE_HOME_WHOLE_NUMBER},
{"2345678901", PHONE_HOME_CITY_AND_NUMBER},
{"1", PHONE_HOME_COUNTRY_CODE},
{"234", PHONE_HOME_CITY_CODE},
{"5678901", PHONE_HOME_NUMBER},
{"567", PHONE_HOME_NUMBER},
{"8901", PHONE_HOME_NUMBER},

// Make sure matches for a second profile work.
{"Charles Holley", NAME_FULL},

// Credit card fields matches.
{"Elvis Presley", CREDIT_CARD_NAME},
{"4234-5678-9012-3456", CREDIT_CARD_NUMBER},
{"04", CREDIT_CARD_EXP_MONTH},
{"April", CREDIT_CARD_EXP_MONTH},
{"2012", CREDIT_CARD_EXP_4_DIGIT_YEAR},
{"12", CREDIT_CARD_EXP_2_DIGIT_YEAR},
{"04/2012", CREDIT_CARD_EXP_DATE_4_DIGIT_YEAR},

// Make sure whitespaces are trimmed properly.
{"", EMPTY_TYPE},
{" ", EMPTY_TYPE},
{" Elvis", NAME_FIRST},
{"Elvis ", NAME_FIRST},

// Make sure fields that differ by case match.
{"elvis ", NAME_FIRST},
{"UnItEd StAtEs", ADDRESS_HOME_COUNTRY},

// Make sure fields that differ by punctuation match.
{"3734 Elvis Presley Blvd", ADDRESS_HOME_LINE1},
{"3734, Elvis Presley Blvd.", ADDRESS_HOME_LINE1},

// Make sure unsupported variants do not match.
{"Elvis Aaron", UNKNOWN_TYPE},
{"Mr. Presley", UNKNOWN_TYPE},
{"3734 Elvis Presley", UNKNOWN_TYPE},
{"TN", UNKNOWN_TYPE},
{"38116-1023", UNKNOWN_TYPE},
{"5", UNKNOWN_TYPE},
{"56", UNKNOWN_TYPE},
{"901", UNKNOWN_TYPE}};
// Profile fields matches.
{"Elvis", NAME_FIRST},
{"Aaron", NAME_MIDDLE},
{"A", NAME_MIDDLE_INITIAL},
{"Presley", NAME_LAST},
{"Elvis Aaron Presley", NAME_FULL},
{"theking@gmail.com", EMAIL_ADDRESS},
{"RCA", COMPANY_NAME},
{"3734 Elvis Presley Blvd.", ADDRESS_HOME_LINE1},
{"Apt. 10", ADDRESS_HOME_LINE2},
{"Memphis", ADDRESS_HOME_CITY},
{"Tennessee", ADDRESS_HOME_STATE},
{"38116", ADDRESS_HOME_ZIP},
{"USA", ADDRESS_HOME_COUNTRY},
{"United States", ADDRESS_HOME_COUNTRY},
{"12345678901", PHONE_HOME_WHOLE_NUMBER},
{"+1 (234) 567-8901", PHONE_HOME_WHOLE_NUMBER},
{"(234)567-8901", PHONE_HOME_CITY_AND_NUMBER},
{"2345678901", PHONE_HOME_CITY_AND_NUMBER},
{"1", PHONE_HOME_COUNTRY_CODE},
{"234", PHONE_HOME_CITY_CODE},
{"5678901", PHONE_HOME_NUMBER},
{"567", PHONE_HOME_NUMBER},
{"8901", PHONE_HOME_NUMBER},

// Test an european profile.
{"Paris", ADDRESS_HOME_CITY},
{"Ile de France", ADDRESS_HOME_STATE},
{"75008", ADDRESS_HOME_ZIP},
{"FR", ADDRESS_HOME_COUNTRY},
{"France", ADDRESS_HOME_COUNTRY},
{"33249197070", PHONE_HOME_WHOLE_NUMBER},
{"+33 2 49 19 70 70", PHONE_HOME_WHOLE_NUMBER},
{"2 49 19 70 70", PHONE_HOME_CITY_AND_NUMBER},
{"249197070", PHONE_HOME_CITY_AND_NUMBER},
{"33", PHONE_HOME_COUNTRY_CODE},
{"2", PHONE_HOME_CITY_CODE},

// Credit card fields matches.
{"Elvis Presley", CREDIT_CARD_NAME},
{"4234-5678-9012-3456", CREDIT_CARD_NUMBER},
{"04", CREDIT_CARD_EXP_MONTH},
{"April", CREDIT_CARD_EXP_MONTH},
{"2012", CREDIT_CARD_EXP_4_DIGIT_YEAR},
{"12", CREDIT_CARD_EXP_2_DIGIT_YEAR},
{"04/2012", CREDIT_CARD_EXP_DATE_4_DIGIT_YEAR},

// Make sure whitespaces are trimmed properly.
{"", EMPTY_TYPE},
{" ", EMPTY_TYPE},
{" Elvis", NAME_FIRST},
{"Elvis ", NAME_FIRST},

// Make sure fields that differ by case match.
{"elvis ", NAME_FIRST},
{"UnItEd StAtEs", ADDRESS_HOME_COUNTRY},

// Make sure fields that differ by punctuation match.
{"3734 Elvis Presley Blvd", ADDRESS_HOME_LINE1},
{"3734, Elvis Presley Blvd.", ADDRESS_HOME_LINE1},

// Special phone number case. A profile with no country code should only
// match PHONE_HOME_CITY_AND_NUMBER.
{"5142821292", PHONE_HOME_CITY_AND_NUMBER},

// Make sure unsupported variants do not match.
{"Elvis Aaron", UNKNOWN_TYPE},
{"Mr. Presley", UNKNOWN_TYPE},
{"3734 Elvis Presley", UNKNOWN_TYPE},
{"TN", UNKNOWN_TYPE},
{"38116-1023", UNKNOWN_TYPE},
{"5", UNKNOWN_TYPE},
{"56", UNKNOWN_TYPE},
{"901", UNKNOWN_TYPE}};

for (TestCase test_case : test_cases) {
FormData form;
Expand All @@ -3380,6 +3402,8 @@ TEST_F(AutofillManagerTest, DeterminePossibleFieldTypesForUpload) {
ASSERT_EQ(1U, form_structure.field_count());
ServerFieldTypeSet possible_types =
form_structure.field(0)->possible_types();
EXPECT_EQ(1U, possible_types.size());

EXPECT_NE(possible_types.end(), possible_types.find(test_case.field_type));
}
}
Expand Down
6 changes: 3 additions & 3 deletions components/autofill/core/browser/autofill_metrics_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ TEST_F(AutofillMetricsTest, QualityMetrics) {
field.is_autofilled = true;
form.fields.push_back(field);
heuristic_types.push_back(PHONE_HOME_CITY_AND_NUMBER);
server_types.push_back(PHONE_HOME_WHOLE_NUMBER);
server_types.push_back(PHONE_HOME_CITY_AND_NUMBER);

// Simulate having seen this form on page load.
autofill_manager_->AddSeenForm(form, heuristic_types, server_types);
Expand All @@ -449,7 +449,7 @@ TEST_F(AutofillMetricsTest, QualityMetrics) {
GetFieldTypeGroupMetric(NAME_FULL, AutofillMetrics::TYPE_MATCH), 1);
histogram_tester.ExpectBucketCount(
"Autofill.Quality.HeuristicType.ByFieldType",
GetFieldTypeGroupMetric(PHONE_HOME_WHOLE_NUMBER,
GetFieldTypeGroupMetric(PHONE_HOME_CITY_AND_NUMBER,
AutofillMetrics::TYPE_MATCH),
1);
// Mismatch:
Expand Down Expand Up @@ -563,7 +563,7 @@ TEST_F(AutofillMetricsTest, QualityMetrics_NoSubmission) {
field.is_autofilled = true;
form.fields.push_back(field);
heuristic_types.push_back(PHONE_HOME_CITY_AND_NUMBER);
server_types.push_back(PHONE_HOME_WHOLE_NUMBER);
server_types.push_back(PHONE_HOME_CITY_AND_NUMBER);

// Simulate having seen this form on page load.
autofill_manager_->AddSeenForm(form, heuristic_types, server_types);
Expand Down
40 changes: 33 additions & 7 deletions components/autofill/core/browser/phone_number.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,11 @@ bool PhoneNumber::SetInfo(const AutofillType& type,
void PhoneNumber::GetMatchingTypes(const base::string16& text,
const std::string& app_locale,
ServerFieldTypeSet* matching_types) const {
// Strip the common phone number non numerical characters before calling the
// base matching type function. For example, the |text| "(514) 121-1523"
// would become the stripped text "5141211523". Since the base matching
// function only does simple canonicalization to match against the stored
// data, some domain specific cases will be covered below.
base::string16 stripped_text = text;
base::RemoveChars(stripped_text, base::ASCIIToUTF16(" .()-"), &stripped_text);
FormGroup::GetMatchingTypes(stripped_text, app_locale, matching_types);
Expand All @@ -164,13 +169,34 @@ void PhoneNumber::GetMatchingTypes(const base::string16& text,
matching_types->insert(PHONE_HOME_NUMBER);
}

base::string16 whole_number =
GetInfo(AutofillType(PHONE_HOME_WHOLE_NUMBER), app_locale);
if (!whole_number.empty()) {
base::string16 normalized_number =
i18n::NormalizePhoneNumber(text, GetRegion(*profile_, app_locale));
if (normalized_number == whole_number)
matching_types->insert(PHONE_HOME_WHOLE_NUMBER);
// TODO(crbug.com/581391): Investigate the use of PhoneNumberUtil when
// matching phone numbers for upload.
// If there is not already a match for PHONE_HOME_WHOLE_NUMBER, normalize the
// |text| based on the app_locale before comparing it to the whole number. For
// example, the France number "33 2 49 19 70 70" would be normalized to
// "+33249197070" whereas the US number "+1 (234) 567-8901" would be
// normalized to "12345678901".
if (matching_types->find(PHONE_HOME_WHOLE_NUMBER) == matching_types->end()) {
base::string16 whole_number =
GetInfo(AutofillType(PHONE_HOME_WHOLE_NUMBER), app_locale);
if (!whole_number.empty()) {
base::string16 normalized_number =
i18n::NormalizePhoneNumber(text, GetRegion(*profile_, app_locale));
if (normalized_number == whole_number)
matching_types->insert(PHONE_HOME_WHOLE_NUMBER);
}
}

// If both PHONE_HOME_CITY_AND_NUMBER and PHONE_HOME_WHOLE_NUMBER are matched,
// it means there is no country code in the profile's phone number. In that
// case, we should only return PHONE_HOME_CITY_AND_NUMBER because it's more
// precise.
ServerFieldTypeSet::iterator whole_number_iterator =
matching_types->find(PHONE_HOME_WHOLE_NUMBER);
if (whole_number_iterator != matching_types->end() &&
matching_types->find(PHONE_HOME_CITY_AND_NUMBER) !=
matching_types->end()) {
matching_types->erase(whole_number_iterator);
}
}

Expand Down

0 comments on commit 03c71f5

Please sign in to comment.