[Autofill] Ensure that clients pass the correct region info when parsing phone numbers.
BUG=100845
Review URL: https://chromiumcodereview.appspot.com/11783045
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@175975 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/autofill/autofill_ie_toolbar_import_win.cc b/chrome/browser/autofill/autofill_ie_toolbar_import_win.cc
index aa08f071..94e95f3 100644
--- a/chrome/browser/autofill/autofill_ie_toolbar_import_win.cc
+++ b/chrome/browser/autofill/autofill_ie_toolbar_import_win.cc
@@ -14,6 +14,7 @@
#include "base/logging.h"
#include "base/string16.h"
#include "base/win/registry.h"
+#include "chrome/browser/autofill/autofill_country.h"
#include "chrome/browser/autofill/autofill_profile.h"
#include "chrome/browser/autofill/credit_card.h"
#include "chrome/browser/autofill/crypto/rc4_decryptor.h"
@@ -65,15 +66,15 @@
return true;
}
-string16 ReadAndDecryptValue(RegKey* key, const wchar_t* value_name) {
+string16 ReadAndDecryptValue(const RegKey& key, const wchar_t* value_name) {
DWORD data_type = REG_BINARY;
DWORD data_size = 0;
- LONG result = key->ReadValue(value_name, NULL, &data_size, &data_type);
+ LONG result = key.ReadValue(value_name, NULL, &data_size, &data_type);
if ((result != ERROR_SUCCESS) || !data_size || data_type != REG_BINARY)
return string16();
std::vector<uint8> data;
data.resize(data_size);
- result = key->ReadValue(value_name, &(data[0]), &data_size, &data_type);
+ result = key.ReadValue(value_name, &(data[0]), &data_size, &data_type);
if (result == ERROR_SUCCESS) {
std::string out_data;
if (syncer::DecryptData(data, &out_data)) {
@@ -123,42 +124,61 @@
typedef std::map<std::wstring, AutofillFieldType> RegToFieldMap;
-bool ImportSingleProfile(FormGroup* profile,
- RegKey* key,
- const RegToFieldMap& reg_to_field ) {
- DCHECK(profile != NULL);
- if (!key->Valid())
+// Imports address or credit card data from the given registry |key| into the
+// given |form_group|, with the help of |reg_to_field|. When importing address
+// data, writes the phone data into |phone|; otherwise, |phone| should be null.
+// Returns true if any fields were set, false otherwise.
+bool ImportSingleFormGroup(const RegKey& key,
+ const RegToFieldMap& reg_to_field,
+ FormGroup* form_group,
+ PhoneNumber::PhoneCombineHelper* phone) {
+ if (!key.Valid())
return false;
bool has_non_empty_fields = false;
- // Phones need to be rebuilt.
- PhoneNumber::PhoneCombineHelper phone;
-
- for (uint32 i = 0; i < key->GetValueCount(); ++i) {
+ for (uint32 i = 0; i < key.GetValueCount(); ++i) {
std::wstring value_name;
- if (key->GetValueNameAt(i, &value_name) != ERROR_SUCCESS)
+ if (key.GetValueNameAt(i, &value_name) != ERROR_SUCCESS)
continue;
+
RegToFieldMap::const_iterator it = reg_to_field.find(value_name);
if (it == reg_to_field.end())
continue; // This field is not imported.
+
string16 field_value = ReadAndDecryptValue(key, value_name.c_str());
if (!field_value.empty()) {
- has_non_empty_fields = true;
if (it->second == CREDIT_CARD_NUMBER)
field_value = DecryptCCNumber(field_value);
- // We need to store phone data in |phone| before building the whole number
- // at the end. The rest of the fields are set "as is".
+ // Phone numbers are stored piece-by-piece, and then reconstructed from
+ // the pieces. The rest of the fields are set "as is".
// TODO(isherman): Call SetInfo(), rather than SetRawInfo().
- if (!phone.SetInfo(it->second, field_value))
- profile->SetRawInfo(it->second, field_value);
+ if (!phone || !phone->SetInfo(it->second, field_value)) {
+ has_non_empty_fields = true;
+ form_group->SetRawInfo(it->second, field_value);
+ }
}
}
+
+ return has_non_empty_fields;
+}
+
+// Imports address data from the given registry |key| into the given |profile|,
+// with the help of |reg_to_field|. Returns true if any fields were set, false
+// otherwise.
+bool ImportSingleProfile(const RegKey& key,
+ const RegToFieldMap& reg_to_field,
+ AutofillProfile* profile) {
+ PhoneNumber::PhoneCombineHelper phone;
+ bool has_non_empty_fields =
+ ImportSingleFormGroup(key, reg_to_field, profile, &phone);
+
// Now re-construct the phones if needed.
string16 constructed_number;
- if (!phone.IsEmpty() &&
- phone.ParseNumber(std::string("US"), &constructed_number)) {
+ const std::string app_locale = AutofillCountry::ApplicationLocale();
+ if (phone.ParseNumber(*profile, app_locale, &constructed_number)) {
+ has_non_empty_fields = true;
profile->SetRawInfo(PHONE_HOME_WHOLE_NUMBER, constructed_number);
}
@@ -232,7 +252,7 @@
key_name.append(iterator_profiles.Name());
RegKey key(HKEY_CURRENT_USER, key_name.c_str(), KEY_READ);
AutofillProfile profile;
- if (ImportSingleProfile(&profile, &key, reg_to_field)) {
+ if (ImportSingleProfile(key, reg_to_field, &profile)) {
// Combine phones into whole phone #.
profiles->push_back(profile);
}
@@ -241,8 +261,8 @@
string16 salt;
RegKey cc_key(HKEY_CURRENT_USER, kCreditCardKey, KEY_READ);
if (cc_key.Valid()) {
- password_hash = ReadAndDecryptValue(&cc_key, kPasswordHashValue);
- salt = ReadAndDecryptValue(&cc_key, kSaltValue);
+ password_hash = ReadAndDecryptValue(cc_key, kPasswordHashValue);
+ salt = ReadAndDecryptValue(cc_key, kSaltValue);
}
// We import CC profiles only if they are not password protected.
@@ -255,7 +275,7 @@
key_name.append(iterator_cc.Name());
RegKey key(HKEY_CURRENT_USER, key_name.c_str(), KEY_READ);
CreditCard credit_card;
- if (ImportSingleProfile(&credit_card, &key, reg_to_field)) {
+ if (ImportSingleFormGroup(key, reg_to_field, &credit_card, NULL)) {
// TODO(isherman): Call into GetInfo() below, rather than GetRawInfo().
string16 cc_number = credit_card.GetRawInfo(CREDIT_CARD_NUMBER);
if (!cc_number.empty())
diff --git a/chrome/browser/autofill/personal_data_manager.cc b/chrome/browser/autofill/personal_data_manager.cc
index 14b6b17..7d6f259 100644
--- a/chrome/browser/autofill/personal_data_manager.cc
+++ b/chrome/browser/autofill/personal_data_manager.cc
@@ -294,8 +294,7 @@
// Construct the phone number. Reject the profile if the number is invalid.
if (imported_profile.get() && !home.IsEmpty()) {
string16 constructed_number;
- if (!home.ParseNumber(imported_profile->CountryCode(),
- &constructed_number) ||
+ if (!home.ParseNumber(*imported_profile, app_locale, &constructed_number) ||
!imported_profile->SetInfo(PHONE_HOME_WHOLE_NUMBER, constructed_number,
app_locale)) {
imported_profile.reset();
diff --git a/chrome/browser/autofill/phone_number.cc b/chrome/browser/autofill/phone_number.cc
index e5027fa..8d3f859 100644
--- a/chrome/browser/autofill/phone_number.cc
+++ b/chrome/browser/autofill/phone_number.cc
@@ -28,6 +28,19 @@
RemoveChars(*number, kPhoneNumberSeparators, number);
}
+// Returns the region code for this phone number, which is an ISO 3166 2-letter
+// country code. The returned value is based on the |profile|; if the |profile|
+// does not have a country code associated with it, falls back to the country
+// code corresponding to the |app_locale|.
+std::string GetRegion(const AutofillProfile& profile,
+ const std::string& app_locale) {
+ std::string country_code = profile.CountryCode();
+ if (!country_code.empty())
+ return country_code;
+
+ return AutofillCountry::CountryCodeForLocale(app_locale);
+}
+
} // namespace
PhoneNumber::PhoneNumber(AutofillProfile* profile)
@@ -97,7 +110,8 @@
// TODO(isherman): Can/should this use the cached_parsed_phone_?
string16 normalized_phone =
- autofill_i18n::NormalizePhoneNumber(phone, GetRegion(app_locale));
+ autofill_i18n::NormalizePhoneNumber(phone,
+ GetRegion(*profile_, app_locale));
return !normalized_phone.empty() ? normalized_phone : phone;
}
@@ -156,7 +170,7 @@
// For US numbers, also compare to the three-digit prefix and the four-digit
// suffix, since web sites often split numbers into these two fields.
string16 number = GetInfo(PHONE_HOME_NUMBER, app_locale);
- if (GetRegion(app_locale) == "US" &&
+ if (GetRegion(*profile_, app_locale) == "US" &&
number.size() == (kPrefixLength + kSuffixLength)) {
string16 prefix = number.substr(kPrefixOffset, kPrefixLength);
string16 suffix = number.substr(kSuffixOffset, kSuffixLength);
@@ -167,22 +181,15 @@
string16 whole_number = GetInfo(PHONE_HOME_WHOLE_NUMBER, app_locale);
if (!whole_number.empty()) {
string16 normalized_number =
- autofill_i18n::NormalizePhoneNumber(text, GetRegion(app_locale));
+ autofill_i18n::NormalizePhoneNumber(text,
+ GetRegion(*profile_, app_locale));
if (normalized_number == whole_number)
matching_types->insert(PHONE_HOME_WHOLE_NUMBER);
}
}
-std::string PhoneNumber::GetRegion(const std::string& app_locale) const {
- const std::string country_code = profile_->CountryCode();
- if (country_code.empty())
- return AutofillCountry::CountryCodeForLocale(app_locale);
-
- return country_code;
-}
-
void PhoneNumber::UpdateCacheIfNeeded(const std::string& app_locale) const {
- std::string region = GetRegion(app_locale);
+ std::string region = GetRegion(*profile_, app_locale);
if (!number_.empty() && cached_parsed_phone_.GetRegion() != region)
cached_parsed_phone_ = autofill_i18n::PhoneObject(number_, region);
}
@@ -223,15 +230,20 @@
return false;
}
-bool PhoneNumber::PhoneCombineHelper::ParseNumber(const std::string& region,
- string16* value) {
+bool PhoneNumber::PhoneCombineHelper::ParseNumber(
+ const AutofillProfile& profile,
+ const std::string& app_locale,
+ string16* value) {
+ if (IsEmpty())
+ return false;
+
if (!whole_number_.empty()) {
*value = whole_number_;
return true;
}
return autofill_i18n::ConstructPhoneNumber(
- country_, city_, phone_, region,
+ country_, city_, phone_, GetRegion(profile, app_locale),
(country_.empty() ?
autofill_i18n::NATIONAL : autofill_i18n::INTERNATIONAL),
value);
diff --git a/chrome/browser/autofill/phone_number.h b/chrome/browser/autofill/phone_number.h
index e2f203b2..ac02ed768 100644
--- a/chrome/browser/autofill/phone_number.h
+++ b/chrome/browser/autofill/phone_number.h
@@ -56,8 +56,13 @@
// returns true. For all other field types returs false.
bool SetInfo(AutofillFieldType type, const string16& value);
- // Returns true if parsing was successful, false otherwise.
- bool ParseNumber(const std::string& region, string16* value);
+ // Parses the number built up from pieces stored via SetInfo() according to
+ // the specified |profile|'s country code, falling back to the given
+ // |app_locale| if the |profile| has no associated country code. Returns
+ // true if parsing was successful, false otherwise.
+ bool ParseNumber(const AutofillProfile& profile,
+ const std::string& app_locale,
+ string16* value);
// Returns true if both |phone_| and |whole_number_| are empty.
bool IsEmpty() const;
@@ -73,13 +78,6 @@
// FormGroup:
virtual void GetSupportedTypes(FieldTypeSet* supported_types) const OVERRIDE;
- // Returns the region code for this phone number, which is an ISO 3166
- // 2-letter country code. The name "region" is chosen since "country code"
- // already refers to part of a phone number. The returned value is based on
- // the |profile_|; if the |profile_| does not have a country code associated
- // with it, falls back to the country code corresponding to the |app_locale|.
- std::string GetRegion(const std::string& app_locale) const;
-
// Updates the cached parsed number if the profile's region has changed
// since the last time the cache was updated.
void UpdateCacheIfNeeded(const std::string& app_locale) const;
diff --git a/chrome/browser/autofill/phone_number_unittest.cc b/chrome/browser/autofill/phone_number_unittest.cc
index c2c77e1c..0aa83ed 100644
--- a/chrome/browser/autofill/phone_number_unittest.cc
+++ b/chrome/browser/autofill/phone_number_unittest.cc
@@ -81,6 +81,9 @@
}
TEST(PhoneNumberTest, PhoneCombineHelper) {
+ AutofillProfile profile;
+ profile.SetCountryCode("US");
+
PhoneNumber::PhoneCombineHelper number1;
EXPECT_FALSE(number1.SetInfo(ADDRESS_BILLING_CITY,
ASCIIToUTF16("1")));
@@ -91,7 +94,7 @@
EXPECT_TRUE(number1.SetInfo(PHONE_HOME_NUMBER,
ASCIIToUTF16("2345678")));
string16 parsed_phone;
- EXPECT_TRUE(number1.ParseNumber("US", &parsed_phone));
+ EXPECT_TRUE(number1.ParseNumber(profile, "en-US", &parsed_phone));
// International format as it has a country code.
EXPECT_EQ(ASCIIToUTF16("+1 650-234-5678"), parsed_phone);
@@ -100,7 +103,7 @@
ASCIIToUTF16("650")));
EXPECT_TRUE(number3.SetInfo(PHONE_HOME_NUMBER,
ASCIIToUTF16("2345680")));
- EXPECT_TRUE(number3.ParseNumber("US", &parsed_phone));
+ EXPECT_TRUE(number3.ParseNumber(profile, "en-US", &parsed_phone));
// National format as it does not have a country code.
EXPECT_EQ(ASCIIToUTF16("(650) 234-5680"), parsed_phone);
@@ -109,13 +112,13 @@
ASCIIToUTF16("123"))); // Incorrect city code.
EXPECT_TRUE(number4.SetInfo(PHONE_HOME_NUMBER,
ASCIIToUTF16("2345680")));
- EXPECT_FALSE(number4.ParseNumber("US", &parsed_phone));
+ EXPECT_FALSE(number4.ParseNumber(profile, "en-US", &parsed_phone));
EXPECT_EQ(string16(), parsed_phone);
PhoneNumber::PhoneCombineHelper number5;
EXPECT_TRUE(number5.SetInfo(PHONE_HOME_CITY_AND_NUMBER,
ASCIIToUTF16("6502345681")));
- EXPECT_TRUE(number5.ParseNumber("US", &parsed_phone));
+ EXPECT_TRUE(number5.ParseNumber(profile, "en-US", &parsed_phone));
EXPECT_EQ(ASCIIToUTF16("(650) 234-5681"), parsed_phone);
PhoneNumber::PhoneCombineHelper number6;
@@ -125,6 +128,18 @@
ASCIIToUTF16("234")));
EXPECT_TRUE(number6.SetInfo(PHONE_HOME_NUMBER,
ASCIIToUTF16("5682")));
- EXPECT_TRUE(number6.ParseNumber("US", &parsed_phone));
+ EXPECT_TRUE(number6.ParseNumber(profile, "en-US", &parsed_phone));
+ EXPECT_EQ(ASCIIToUTF16("(650) 234-5682"), parsed_phone);
+
+ // Ensure parsing is possible when falling back to detecting the country code
+ // based on the app locale.
+ PhoneNumber::PhoneCombineHelper number7;
+ EXPECT_TRUE(number7.SetInfo(PHONE_HOME_CITY_CODE,
+ ASCIIToUTF16("650")));
+ EXPECT_TRUE(number7.SetInfo(PHONE_HOME_NUMBER,
+ ASCIIToUTF16("234")));
+ EXPECT_TRUE(number7.SetInfo(PHONE_HOME_NUMBER,
+ ASCIIToUTF16("5682")));
+ EXPECT_TRUE(number7.ParseNumber(AutofillProfile(), "en-US", &parsed_phone));
EXPECT_EQ(ASCIIToUTF16("(650) 234-5682"), parsed_phone);
}
