diff options
author | isherman@chromium.org <isherman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-01-15 22:33:22 +0000 |
---|---|---|
committer | isherman@chromium.org <isherman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-01-15 22:33:22 +0000 |
commit | 7df904dfce777e1714bed6a0c549cd5725dd1d0e (patch) | |
tree | 73919fd7a6d7551508738574351cff9d9a4f78fa | |
parent | dc65b35c0805d3fd4359f4ed81dae84e08a71253 (diff) | |
download | chromium_src-7df904dfce777e1714bed6a0c549cd5725dd1d0e.zip chromium_src-7df904dfce777e1714bed6a0c549cd5725dd1d0e.tar.gz chromium_src-7df904dfce777e1714bed6a0c549cd5725dd1d0e.tar.bz2 |
[Autofill] Phone number cleanup.
Lots of little things:
* In PhoneNumber, use the cached_parsed_phone_ for requests for the whole number as well as for other requests.
* Updated the comment within PhoneNumber::SetInfo() to reflect what the method actually does.
* Add a test to verify that cached phone numbers are correctly invalidated and updated.
* Remove ParsePhoneNumberInternal() in favor of using ParsePhoneNumber() throughout.
* Remove a redundant parameter to ConstructPhoneNumber()
* Use hacker_case for simple accessors in PhoneObject
* Factor out shared code in phone_number_i18n.cc
* Make sure to clear the incoming PhoneNumber argument to ParsePhoneNumber(), as libphonenumber doesn't do this for us.
* Dramatically simplify the implementations for NormalizePhoneNumber(), ConstructPhoneNumber(), and PhoneObject::GetWholeNumber() by relying on factored out shared code.
* Fix PhoneObject::operator=() to copy the whole_number_ as well as the other fields. (FWIW, since whole_number_ is usually computed from i18n_number_, this only rarely caused problems.)
* Remove the explicit UI thread and MessageLoop from PhoneNumberI18NTest now that they're no longer needed.
BUG=none
Review URL: https://chromiumcodereview.appspot.com/11879013
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@176993 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/autofill/phone_number.cc | 52 | ||||
-rw-r--r-- | chrome/browser/autofill/phone_number_i18n.cc | 243 | ||||
-rw-r--r-- | chrome/browser/autofill/phone_number_i18n.h | 57 | ||||
-rw-r--r-- | chrome/browser/autofill/phone_number_i18n_unittest.cc | 128 | ||||
-rw-r--r-- | chrome/browser/autofill/phone_number_unittest.cc | 58 | ||||
-rw-r--r-- | chrome/chrome_tests_unit.gypi | 1 |
6 files changed, 282 insertions, 257 deletions
diff --git a/chrome/browser/autofill/phone_number.cc b/chrome/browser/autofill/phone_number.cc index 8d3f859..5fe7665 100644 --- a/chrome/browser/autofill/phone_number.cc +++ b/chrome/browser/autofill/phone_number.cc @@ -48,8 +48,7 @@ PhoneNumber::PhoneNumber(AutofillProfile* profile) } PhoneNumber::PhoneNumber(const PhoneNumber& number) - : FormGroup(), - profile_(NULL) { + : profile_(NULL) { *this = number; } @@ -103,39 +102,30 @@ void PhoneNumber::SetRawInfo(AutofillFieldType type, const string16& value) { // If the phone cannot be normalized, returns the stored value verbatim. string16 PhoneNumber::GetInfo(AutofillFieldType type, const std::string& app_locale) const { - if (type == PHONE_HOME_WHOLE_NUMBER) { - // Whole numbers require special handling: If normalization for the number - // fails, return the non-normalized number instead. - string16 phone = GetRawInfo(type); - - // TODO(isherman): Can/should this use the cached_parsed_phone_? - string16 normalized_phone = - autofill_i18n::NormalizePhoneNumber(phone, - GetRegion(*profile_, app_locale)); - return !normalized_phone.empty() ? normalized_phone : phone; - } - UpdateCacheIfNeeded(app_locale); - if (!cached_parsed_phone_.IsValidNumber()) + + // Queries for whole numbers will return the non-normalized number if + // normalization for the number fails. All other field types require + // normalization. + if (type != PHONE_HOME_WHOLE_NUMBER && !cached_parsed_phone_.IsValidNumber()) return string16(); switch (type) { + case PHONE_HOME_WHOLE_NUMBER: + return cached_parsed_phone_.GetWholeNumber(); + case PHONE_HOME_NUMBER: - return cached_parsed_phone_.GetNumber(); + return cached_parsed_phone_.number(); case PHONE_HOME_CITY_CODE: - return cached_parsed_phone_.GetCityCode(); + return cached_parsed_phone_.city_code(); case PHONE_HOME_COUNTRY_CODE: - return cached_parsed_phone_.GetCountryCode(); + return cached_parsed_phone_.country_code(); case PHONE_HOME_CITY_AND_NUMBER: return - cached_parsed_phone_.GetCityCode() + cached_parsed_phone_.GetNumber(); - - case PHONE_HOME_WHOLE_NUMBER: - NOTREACHED(); // Should have been handled above. - return string16(); + cached_parsed_phone_.city_code() + cached_parsed_phone_.number(); default: NOTREACHED(); @@ -146,17 +136,14 @@ string16 PhoneNumber::GetInfo(AutofillFieldType type, bool PhoneNumber::SetInfo(AutofillFieldType type, const string16& value, const std::string& app_locale) { - string16 number = value; - StripPunctuation(&number); - SetRawInfo(type, number); + SetRawInfo(type, value); if (number_.empty()) return true; - // Normalize the phone number by validating and translating it into a - // digits-only format. + // Store a formatted (i.e., pretty printed) version of the number. UpdateCacheIfNeeded(app_locale); - number_ = cached_parsed_phone_.GetWholeNumber(); + number_ = cached_parsed_phone_.GetFormattedNumber(); return !number_.empty(); } @@ -190,7 +177,7 @@ void PhoneNumber::GetMatchingTypes(const string16& text, void PhoneNumber::UpdateCacheIfNeeded(const std::string& app_locale) const { std::string region = GetRegion(*profile_, app_locale); - if (!number_.empty() && cached_parsed_phone_.GetRegion() != region) + if (!number_.empty() && cached_parsed_phone_.region() != region) cached_parsed_phone_ = autofill_i18n::PhoneObject(number_, region); } @@ -243,10 +230,7 @@ bool PhoneNumber::PhoneCombineHelper::ParseNumber( } return autofill_i18n::ConstructPhoneNumber( - country_, city_, phone_, GetRegion(profile, app_locale), - (country_.empty() ? - autofill_i18n::NATIONAL : autofill_i18n::INTERNATIONAL), - value); + country_, city_, phone_, GetRegion(profile, app_locale), value); } bool PhoneNumber::PhoneCombineHelper::IsEmpty() const { diff --git a/chrome/browser/autofill/phone_number_i18n.cc b/chrome/browser/autofill/phone_number_i18n.cc index d3af5ad..31ad713 100644 --- a/chrome/browser/autofill/phone_number_i18n.cc +++ b/chrome/browser/autofill/phone_number_i18n.cc @@ -8,6 +8,7 @@ #include "base/logging.h" #include "base/stringprintf.h" #include "base/string_number_conversions.h" +#include "base/string_util.h" #include "base/utf_string_conversions.h" #include "chrome/browser/autofill/autofill_country.h" #include "third_party/libphonenumber/src/phonenumber_api.h" @@ -25,40 +26,69 @@ std::string SanitizeRegion(const std::string& region) { AutofillCountry::ApplicationLocale()); } -PhoneNumberUtil::PhoneNumberFormat UtilsTypeToPhoneLibType( - autofill_i18n::FullPhoneFormat phone_format) { - switch (phone_format) { - case autofill_i18n::E164: - return PhoneNumberUtil::E164; - case autofill_i18n::INTERNATIONAL: - return PhoneNumberUtil::INTERNATIONAL; - case autofill_i18n::NATIONAL: - return PhoneNumberUtil::NATIONAL; - case autofill_i18n::RFC3966: - return PhoneNumberUtil::RFC3966; - default: - NOTREACHED(); - return PhoneNumberUtil::NATIONAL; +// Returns true if |phone_number| is valid. +bool IsValidPhoneNumber(const PhoneNumber& phone_number) { + PhoneNumberUtil* phone_util = PhoneNumberUtil::GetInstance(); + if (!phone_util->IsPossibleNumber(phone_number)) + return false; + + // Verify that the number has a valid area code (that in some cases could be + // empty) for the parsed country code. Also verify that this is a valid + // number (for example, in the US 1234567 is not valid, because numbers do not + // start with 1). + if (!phone_util->IsValidNumber(phone_number)) + return false; + + return true; +} + +// Formats the given |number| as a human-readable string, and writes the result +// into |formatted_number|. Also, normalizes the formatted number, and writes +// that result into |normalized_number|. This function should only be called +// with numbers already known to be valid, i.e. validation should be done prior +// to calling this function. Note that the |country_code|, which determines +// whether to format in the national or in the international format, is passed +// in explicitly, as |number| might have an implicit country code set, even +// though the original input lacked a country code. +void FormatValidatedNumber(const PhoneNumber& number, + const string16& country_code, + string16* formatted_number, + string16* normalized_number) { + PhoneNumberUtil::PhoneNumberFormat format = + country_code.empty() ? + PhoneNumberUtil::NATIONAL : + PhoneNumberUtil::INTERNATIONAL; + + PhoneNumberUtil* phone_util = PhoneNumberUtil::GetInstance(); + std::string processed_number; + phone_util->Format(number, format, &processed_number); + + if (formatted_number) + *formatted_number = UTF8ToUTF16(processed_number); + + if (normalized_number) { + phone_util->NormalizeDigitsOnly(&processed_number); + *normalized_number = UTF8ToUTF16(processed_number); } } +} // namespace + +namespace autofill_i18n { + // Parses the number stored in |value| as it should be interpreted in the given // |region|, and stores the results into the remaining arguments. The |region| // should be sanitized prior to calling this function. -bool ParsePhoneNumberInternal(const string16& value, - const std::string& region, - string16* country_code, - string16* city_code, - string16* number, - PhoneNumber* i18n_number) { - DCHECK(country_code); - DCHECK(city_code); - DCHECK(number); - DCHECK(i18n_number); - +bool ParsePhoneNumber(const string16& value, + const std::string& region, + string16* country_code, + string16* city_code, + string16* number, + PhoneNumber* i18n_number) { country_code->clear(); city_code->clear(); number->clear(); + *i18n_number = PhoneNumber(); std::string number_text(UTF16ToUTF8(value)); @@ -72,24 +102,13 @@ bool ParsePhoneNumberInternal(const string16& value, return false; } - PhoneNumberUtil::ValidationResult validation = - phone_util->IsPossibleNumberWithReason(*i18n_number); - if (validation != PhoneNumberUtil::IS_POSSIBLE) - return false; - - // This verifies that number has a valid area code (that in some cases could - // be empty) for parsed country code. Also verifies that this is a valid - // number (in US 1234567 is not valid, because numbers do not start with 1). - if (!phone_util->IsValidNumber(*i18n_number)) + if (!IsValidPhoneNumber(*i18n_number)) return false; std::string national_significant_number; phone_util->GetNationalSignificantNumber(*i18n_number, &national_significant_number); - std::string area_code; - std::string subscriber_number; - int area_length = phone_util->GetLengthOfGeographicalAreaCode(*i18n_number); int destination_length = phone_util->GetLengthOfNationalDestinationCode(*i18n_number); @@ -98,6 +117,9 @@ bool ParsePhoneNumberInternal(const string16& value, // these two types of codes are the same. if (destination_length > area_length) area_length = destination_length; + + std::string area_code; + std::string subscriber_number; if (area_length > 0) { area_code = national_significant_number.substr(0, area_length); subscriber_number = national_significant_number.substr(area_length); @@ -110,14 +132,15 @@ bool ParsePhoneNumberInternal(const string16& value, phone_util->NormalizeDigitsOnly(&number_text); string16 normalized_number(UTF8ToUTF16(number_text)); - // Check if parsed number has country code and it was not inferred from the + + // Check if parsed number has a country code that was not inferred from the // region. if (i18n_number->has_country_code()) { *country_code = UTF8ToUTF16( - base::StringPrintf("%d", i18n_number->country_code())); + base::StringPrintf("%d", i18n_number->country_code())); if (normalized_number.length() <= national_significant_number.length() && - (normalized_number.length() < country_code->length() || - normalized_number.compare(0, country_code->length(), *country_code))) { + !StartsWith(normalized_number, *country_code, + true /* case_sensitive */)) { country_code->clear(); } } @@ -125,98 +148,48 @@ bool ParsePhoneNumberInternal(const string16& value, return true; } -} // namespace - -namespace autofill_i18n { - string16 NormalizePhoneNumber(const string16& value, std::string const& region) { - string16 number; - string16 city_code; string16 country_code; - string16 result; - // Full number - parse it, split it and re-combine into canonical form. - if (!ParsePhoneNumber(value, region, &country_code, &city_code, &number)) + string16 unused_city_code; + string16 unused_number; + PhoneNumber phone_number; + if (!ParsePhoneNumber(value, SanitizeRegion(region), &country_code, + &unused_city_code, &unused_number, &phone_number)) { return string16(); // Parsing failed - do not store phone. - if (!autofill_i18n::ConstructPhoneNumber( - country_code, city_code, number, - region, - (country_code.empty() ? - autofill_i18n::NATIONAL : autofill_i18n::INTERNATIONAL), - &result)) { - // Reconstruction failed - do not store phone. - return string16(); } - std::string result_utf8(UTF16ToUTF8(result)); - PhoneNumberUtil::GetInstance()->NormalizeDigitsOnly(&result_utf8); - return UTF8ToUTF16(result_utf8); -} -bool ParsePhoneNumber(const string16& value, - const std::string& region, - string16* country_code, - string16* city_code, - string16* number) { - PhoneNumber unused_i18n_number; - return ParsePhoneNumberInternal(value, SanitizeRegion(region), - country_code, city_code, number, - &unused_i18n_number); + string16 normalized_number; + FormatValidatedNumber(phone_number, country_code, NULL, &normalized_number); + return normalized_number; } bool ConstructPhoneNumber(const string16& country_code, const string16& city_code, const string16& number, const std::string& region, - FullPhoneFormat phone_format, string16* whole_number) { - DCHECK(whole_number); - whole_number->clear(); - std::string normalized_number(UTF16ToUTF8(city_code)); - normalized_number.append(UTF16ToUTF8(number)); - - PhoneNumberUtil* phone_util = PhoneNumberUtil::GetInstance(); - - phone_util->NormalizeDigitsOnly(&normalized_number); - - int64 number_int = 0; - if (!base::StringToInt64(normalized_number, &number_int) || !number_int) + string16 unused_country_code; + string16 unused_city_code; + string16 unused_number; + PhoneNumber phone_number; + if (!ParsePhoneNumber(country_code + city_code + number, + SanitizeRegion(region), + &unused_country_code, &unused_city_code, &unused_number, + &phone_number)) { return false; + } - PhoneNumber i18n_number; - i18n_number.set_national_number(static_cast<uint64>(number_int)); - - int country_int = phone_util->GetCountryCodeForRegion( - SanitizeRegion(region)); - if (!country_code.empty() && !base::StringToInt(country_code, &country_int)) - return false; - if (country_int) - i18n_number.set_country_code(country_int); - - PhoneNumberUtil::ValidationResult validation = - phone_util->IsPossibleNumberWithReason(i18n_number); - if (validation != PhoneNumberUtil::IS_POSSIBLE) - return false; - - // This verifies that number has a valid area code (that in some cases could - // be empty) for parsed country code. Also verifies that this is a valid - // number (in US 1234567 is not valid, because numbers do not start with 1). - if (!phone_util->IsValidNumber(i18n_number)) - return false; - - std::string formatted_number; - - phone_util->Format(i18n_number, UtilsTypeToPhoneLibType(phone_format), - &formatted_number); - *whole_number = UTF8ToUTF16(formatted_number); + FormatValidatedNumber(phone_number, country_code, whole_number, NULL); return true; } bool PhoneNumbersMatch(const string16& number_a, const string16& number_b, const std::string& raw_region) { - // Sanitize the provided |country_code| before trying to use it for parsing. + // Sanitize the provided |raw_region| before trying to use it for parsing. const std::string region = SanitizeRegion(raw_region); PhoneNumberUtil* phone_util = PhoneNumberUtil::GetInstance(); @@ -243,10 +216,10 @@ bool PhoneNumbersMatch(const string16& number_a, case PhoneNumberUtil::NSN_MATCH: case PhoneNumberUtil::EXACT_MATCH: return true; - default: - NOTREACHED(); - return false; } + + NOTREACHED(); + return false; } PhoneObject::PhoneObject(const string16& number, const std::string& region) @@ -259,10 +232,11 @@ PhoneObject::PhoneObject(const string16& number, const std::string& region) // verify. scoped_ptr<PhoneNumber> i18n_number(new PhoneNumber); - if (ParsePhoneNumberInternal(number, region_, &country_code_, &city_code_, - &number_, i18n_number.get())) { - // Phone successfully parsed - set |i18n_number_| object, |whole_number_| - // will be set on the first call to GetWholeNumber(). + if (ParsePhoneNumber(number, region_, &country_code_, &city_code_, &number_, + i18n_number.get())) { + // The phone number was successfully parsed, so store the parsed version. + // The formatted and normalized versions will be set on the first call to + // the coresponding methods. i18n_number_.reset(i18n_number.release()); } else { // Parsing failed. Store passed phone "as is" into |whole_number_|. @@ -280,17 +254,21 @@ PhoneObject::PhoneObject() : i18n_number_(NULL) { PhoneObject::~PhoneObject() { } +string16 PhoneObject::GetFormattedNumber() const { + if (i18n_number_ && formatted_number_.empty()) { + FormatValidatedNumber(*i18n_number_, country_code_, &formatted_number_, + &whole_number_); + } + + return formatted_number_; +} + string16 PhoneObject::GetWholeNumber() const { - if (i18n_number_.get() && whole_number_.empty()) { - PhoneNumberUtil::PhoneNumberFormat format = PhoneNumberUtil::INTERNATIONAL; - if (country_code_.empty()) - format = PhoneNumberUtil::NATIONAL; - - std::string formatted_number; - PhoneNumberUtil* phone_util = PhoneNumberUtil::GetInstance(); - phone_util->Format(*i18n_number_, format, &formatted_number); - whole_number_ = UTF8ToUTF16(formatted_number); + if (i18n_number_ && whole_number_.empty()) { + FormatValidatedNumber(*i18n_number_, country_code_, &formatted_number_, + &whole_number_); } + return whole_number_; } @@ -298,12 +276,19 @@ PhoneObject& PhoneObject::operator=(const PhoneObject& other) { if (this == &other) return *this; - country_code_ = other.country_code_; - city_code_ = other.city_code_; - number_ = other.number_; region_ = other.region_; + if (other.i18n_number_.get()) i18n_number_.reset(new PhoneNumber(*other.i18n_number_)); + else + i18n_number_.reset(); + + country_code_ = other.country_code_; + city_code_ = other.city_code_; + number_ = other.number_; + + formatted_number_ = other.formatted_number_; + whole_number_ = other.whole_number_; return *this; } diff --git a/chrome/browser/autofill/phone_number_i18n.h b/chrome/browser/autofill/phone_number_i18n.h index 5e40b11..9010593 100644 --- a/chrome/browser/autofill/phone_number_i18n.h +++ b/chrome/browser/autofill/phone_number_i18n.h @@ -24,34 +24,28 @@ namespace autofill_i18n { // Most of the following functions require |region| to operate. The |region| is // a ISO 3166 standard code ("US" for USA, "CZ" for Czech Republic, etc.). -// Normalizes phone number, by changing digits in the extended fonts -// (such as \xFF1x) into '0'-'9'. Also strips out non-digit characters. -string16 NormalizePhoneNumber(const string16& value, - const std::string& region); - // Parses the number stored in |value| as a phone number interpreted in the // given |region|, and stores the results into the remaining arguments. The // |region| should be a 2-letter country code. This is an internal function, // exposed in the header file so that it can be tested. -bool ParsePhoneNumber(const string16& value, - const std::string& region, - string16* country_code, - string16* city_code, - string16* number) WARN_UNUSED_RESULT; - -enum FullPhoneFormat { - E164, // Example: +16502345678 - INTERNATIONAL, // Example: +1 650-234-5678 - NATIONAL, // Example: (650) 234-5678 - RFC3966 // Example: +1-650-234-5678 -}; +bool ParsePhoneNumber( + const string16& value, + const std::string& region, + string16* country_code, + string16* city_code, + string16* number, + i18n::phonenumbers::PhoneNumber* i18n_number) WARN_UNUSED_RESULT; + +// Normalizes phone number, by changing digits in the extended fonts +// (such as \xFF1x) into '0'-'9'. Also strips out non-digit characters. +string16 NormalizePhoneNumber(const string16& value, + const std::string& region); // Constructs whole phone number from parts. // |city_code| - area code, could be empty. // |country_code| - country code, could be empty. // |number| - local number, should not be empty. // |region| - current region, the parsing is based on. -// |phone_format| - whole number constructed in that format, // |whole_number| - constructed whole number. // Separator characters are stripped before parsing the digits. // Returns true if parsing was successful, false otherwise. @@ -59,7 +53,6 @@ bool ConstructPhoneNumber(const string16& country_code, const string16& city_code, const string16& number, const std::string& region, - FullPhoneFormat phone_format, string16* whole_number) WARN_UNUSED_RESULT; // Returns true if |number_a| and |number_b| parse to the same phone number in @@ -76,11 +69,13 @@ class PhoneObject { PhoneObject(); ~PhoneObject(); - string16 GetCountryCode() const { return country_code_; } - string16 GetCityCode() const { return city_code_; } - string16 GetNumber() const { return number_; } - std::string GetRegion() const { return region_; } + std::string region() const { return region_; } + + string16 country_code() const { return country_code_; } + string16 city_code() const { return city_code_; } + string16 number() const { return number_; } + string16 GetFormattedNumber() const; string16 GetWholeNumber() const; PhoneObject& operator=(const PhoneObject& other); @@ -88,12 +83,22 @@ class PhoneObject { bool IsValidNumber() const { return i18n_number_ != NULL; } private: + // The region code used to parse this number. + std::string region_; + + // The parsed number and its components. + scoped_ptr<i18n::phonenumbers::PhoneNumber> i18n_number_; string16 city_code_; string16 country_code_; string16 number_; - mutable string16 whole_number_; // Set on first request. - std::string region_; - scoped_ptr<i18n::phonenumbers::PhoneNumber> i18n_number_; + + // Pretty printed version of the whole number, or empty if parsing failed. + // Set on first request. + mutable string16 formatted_number_; + + // The whole number, normalized to contain only digits if possible. + // Set on first request. + mutable string16 whole_number_; }; } // namespace autofill_i18n diff --git a/chrome/browser/autofill/phone_number_i18n_unittest.cc b/chrome/browser/autofill/phone_number_i18n_unittest.cc index f869d9c..fb03759 100644 --- a/chrome/browser/autofill/phone_number_i18n_unittest.cc +++ b/chrome/browser/autofill/phone_number_i18n_unittest.cc @@ -8,6 +8,7 @@ #include "chrome/browser/autofill/phone_number_i18n.h" #include "content/public/test/test_browser_thread.h" #include "testing/gtest/include/gtest/gtest.h" +#include "third_party/libphonenumber/src/phonenumber_api.h" using autofill_i18n::NormalizePhoneNumber; using autofill_i18n::ParsePhoneNumber; @@ -15,20 +16,7 @@ using autofill_i18n::ConstructPhoneNumber; using autofill_i18n::PhoneNumbersMatch; using content::BrowserThread; -class PhoneNumberI18NTest : public testing::Test { - public: - // In order to access the application locale -- which the tested functions do - // internally -- this test must run on the UI thread. - PhoneNumberI18NTest() : ui_thread_(BrowserThread::UI, &message_loop_) {} - - private: - MessageLoopForUI message_loop_; - content::TestBrowserThread ui_thread_; - - DISALLOW_COPY_AND_ASSIGN(PhoneNumberI18NTest); -}; - -TEST_F(PhoneNumberI18NTest, NormalizePhoneNumber) { +TEST(PhoneNumberI18NTest, NormalizePhoneNumber) { // "Large" digits. string16 phone1(UTF8ToUTF16("\xEF\xBC\x91\xEF\xBC\x96\xEF\xBC\x95\xEF\xBC\x90" "\xEF\xBC\x97\xEF\xBC\x94\xEF\xBC\x99\xEF\xBC\x98" @@ -50,17 +38,19 @@ TEST_F(PhoneNumberI18NTest, NormalizePhoneNumber) { EXPECT_EQ(NormalizePhoneNumber(phone5, "US"), ASCIIToUTF16("6502346789")); } -TEST_F(PhoneNumberI18NTest, ParsePhoneNumber) { +TEST(PhoneNumberI18NTest, ParsePhoneNumber) { string16 number; string16 city_code; string16 country_code; + i18n::phonenumbers::PhoneNumber unused_i18n_number; // Test for empty string. Should give back empty strings. string16 phone0; EXPECT_FALSE(ParsePhoneNumber(phone0, "US", &country_code, &city_code, - &number)); + &number, + &unused_i18n_number)); EXPECT_EQ(string16(), number); EXPECT_EQ(string16(), city_code); EXPECT_EQ(string16(), country_code); @@ -70,7 +60,8 @@ TEST_F(PhoneNumberI18NTest, ParsePhoneNumber) { EXPECT_FALSE(ParsePhoneNumber(phone1, "US", &country_code, &city_code, - &number)); + &number, + &unused_i18n_number)); EXPECT_EQ(string16(), number); EXPECT_EQ(string16(), city_code); EXPECT_EQ(string16(), country_code); @@ -81,7 +72,8 @@ TEST_F(PhoneNumberI18NTest, ParsePhoneNumber) { EXPECT_FALSE(ParsePhoneNumber(phone2, "US", &country_code, &city_code, - &number)); + &number, + &unused_i18n_number)); EXPECT_EQ(string16(), number); EXPECT_EQ(string16(), city_code); EXPECT_EQ(string16(), country_code); @@ -91,7 +83,8 @@ TEST_F(PhoneNumberI18NTest, ParsePhoneNumber) { EXPECT_FALSE(ParsePhoneNumber(phone3, "US", &country_code, &city_code, - &number)); + &number, + &unused_i18n_number)); EXPECT_EQ(string16(), number); EXPECT_EQ(string16(), city_code); EXPECT_EQ(string16(), country_code); @@ -102,7 +95,8 @@ TEST_F(PhoneNumberI18NTest, ParsePhoneNumber) { EXPECT_FALSE(ParsePhoneNumber(phone4, "US", &country_code, &city_code, - &number)); + &number, + &unused_i18n_number)); EXPECT_EQ(string16(), number); EXPECT_EQ(string16(), city_code); EXPECT_EQ(string16(), country_code); @@ -114,7 +108,8 @@ TEST_F(PhoneNumberI18NTest, ParsePhoneNumber) { EXPECT_FALSE(ParsePhoneNumber(phone_separator4, "US", &country_code, &city_code, - &number)); + &number, + &unused_i18n_number)); EXPECT_EQ(string16(), number); EXPECT_EQ(string16(), city_code); EXPECT_EQ(string16(), country_code); @@ -126,7 +121,8 @@ TEST_F(PhoneNumberI18NTest, ParsePhoneNumber) { EXPECT_FALSE(ParsePhoneNumber(phone5, "US", &country_code, &city_code, - &number)); + &number, + &unused_i18n_number)); EXPECT_EQ(string16(), number); EXPECT_EQ(string16(), city_code); EXPECT_EQ(string16(), country_code); @@ -136,7 +132,8 @@ TEST_F(PhoneNumberI18NTest, ParsePhoneNumber) { EXPECT_FALSE(ParsePhoneNumber(phone6, "US", &country_code, &city_code, - &number)); + &number, + &unused_i18n_number)); EXPECT_EQ(string16(), number); EXPECT_EQ(string16(), city_code); EXPECT_EQ(string16(), country_code); @@ -145,7 +142,8 @@ TEST_F(PhoneNumberI18NTest, ParsePhoneNumber) { EXPECT_TRUE(ParsePhoneNumber(phone7, "US", &country_code, &city_code, - &number)); + &number, + &unused_i18n_number)); EXPECT_EQ(ASCIIToUTF16("4567890"), number); EXPECT_EQ(ASCIIToUTF16("650"), city_code); EXPECT_EQ(string16(), country_code); @@ -156,7 +154,8 @@ TEST_F(PhoneNumberI18NTest, ParsePhoneNumber) { EXPECT_TRUE(ParsePhoneNumber(phone_separator7, "US", &country_code, &city_code, - &number)); + &number, + &unused_i18n_number)); EXPECT_EQ(ASCIIToUTF16("4567890"), number); EXPECT_EQ(ASCIIToUTF16("650"), city_code); EXPECT_EQ(string16(), country_code); @@ -168,7 +167,8 @@ TEST_F(PhoneNumberI18NTest, ParsePhoneNumber) { EXPECT_FALSE(ParsePhoneNumber(phone8, "US", &country_code, &city_code, - &number)); + &number, + &unused_i18n_number)); EXPECT_EQ(string16(), number); EXPECT_EQ(string16(), city_code); EXPECT_EQ(string16(), country_code); @@ -178,7 +178,8 @@ TEST_F(PhoneNumberI18NTest, ParsePhoneNumber) { EXPECT_TRUE(ParsePhoneNumber(phone9, "US", &country_code, &city_code, - &number)); + &number, + &unused_i18n_number)); EXPECT_EQ(ASCIIToUTF16("4567890"), number); EXPECT_EQ(ASCIIToUTF16("650"), city_code); EXPECT_EQ(ASCIIToUTF16("1"), country_code); @@ -188,7 +189,8 @@ TEST_F(PhoneNumberI18NTest, ParsePhoneNumber) { EXPECT_TRUE(ParsePhoneNumber(phone10, "US", &country_code, &city_code, - &number)); + &number, + &unused_i18n_number)); EXPECT_EQ(ASCIIToUTF16("4567890"), number); EXPECT_EQ(ASCIIToUTF16("812"), city_code); EXPECT_EQ(ASCIIToUTF16("7"), country_code); @@ -200,7 +202,8 @@ TEST_F(PhoneNumberI18NTest, ParsePhoneNumber) { EXPECT_TRUE(ParsePhoneNumber(phone11, "US", &country_code, &city_code, - &number)); + &number, + &unused_i18n_number)); EXPECT_EQ(ASCIIToUTF16("4567890"), number); EXPECT_EQ(ASCIIToUTF16("650"), city_code); EXPECT_EQ(ASCIIToUTF16("1"), country_code); @@ -211,7 +214,8 @@ TEST_F(PhoneNumberI18NTest, ParsePhoneNumber) { EXPECT_TRUE(ParsePhoneNumber(phone12, "US", &country_code, &city_code, - &number)); + &number, + &unused_i18n_number)); EXPECT_EQ(ASCIIToUTF16("910112"), number); EXPECT_EQ(ASCIIToUTF16("278"), city_code); EXPECT_EQ(ASCIIToUTF16("420"), country_code); @@ -219,7 +223,8 @@ TEST_F(PhoneNumberI18NTest, ParsePhoneNumber) { EXPECT_TRUE(ParsePhoneNumber(phone12, "CZ", &country_code, &city_code, - &number)); + &number, + &unused_i18n_number)); EXPECT_EQ(ASCIIToUTF16("910112"), number); EXPECT_EQ(ASCIIToUTF16("278"), city_code); EXPECT_EQ(ASCIIToUTF16("420"), country_code); @@ -228,11 +233,13 @@ TEST_F(PhoneNumberI18NTest, ParsePhoneNumber) { EXPECT_FALSE(ParsePhoneNumber(phone13, "US", &country_code, &city_code, - &number)); + &number, + &unused_i18n_number)); EXPECT_TRUE(ParsePhoneNumber(phone13, "CZ", &country_code, &city_code, - &number)); + &number, + &unused_i18n_number)); EXPECT_EQ(ASCIIToUTF16("910112"), number); EXPECT_EQ(ASCIIToUTF16("578"), city_code); EXPECT_EQ(ASCIIToUTF16("420"), country_code); @@ -241,7 +248,8 @@ TEST_F(PhoneNumberI18NTest, ParsePhoneNumber) { EXPECT_TRUE(ParsePhoneNumber(phone14, "US", &country_code, &city_code, - &number)); + &number, + &unused_i18n_number)); EXPECT_EQ(ASCIIToUTF16("3569377"), number); EXPECT_EQ(ASCIIToUTF16("650"), city_code); EXPECT_EQ(ASCIIToUTF16("1"), country_code); @@ -252,91 +260,75 @@ TEST_F(PhoneNumberI18NTest, ParsePhoneNumber) { EXPECT_TRUE(ParsePhoneNumber(phone15, "US", &country_code, &city_code, - &number)); + &number, + &unused_i18n_number)); EXPECT_EQ(ASCIIToUTF16("3569377"), number); EXPECT_EQ(ASCIIToUTF16("800"), city_code); EXPECT_EQ(ASCIIToUTF16("1"), country_code); } -TEST_F(PhoneNumberI18NTest, ConstructPhoneNumber) { +TEST(PhoneNumberI18NTest, ConstructPhoneNumber) { string16 number; EXPECT_TRUE(ConstructPhoneNumber(ASCIIToUTF16("1"), ASCIIToUTF16("650"), ASCIIToUTF16("2345678"), "US", - autofill_i18n::E164, - &number)); - EXPECT_EQ(number, ASCIIToUTF16("+16502345678")); - EXPECT_TRUE(ConstructPhoneNumber(ASCIIToUTF16("1"), - ASCIIToUTF16("650"), - ASCIIToUTF16("2345678"), - "US", - autofill_i18n::INTERNATIONAL, &number)); EXPECT_EQ(number, ASCIIToUTF16("+1 650-234-5678")); - EXPECT_TRUE(ConstructPhoneNumber(ASCIIToUTF16("1"), + EXPECT_TRUE(ConstructPhoneNumber(string16(), ASCIIToUTF16("650"), ASCIIToUTF16("2345678"), "US", - autofill_i18n::NATIONAL, &number)); EXPECT_EQ(number, ASCIIToUTF16("(650) 234-5678")); EXPECT_TRUE(ConstructPhoneNumber(ASCIIToUTF16("1"), - ASCIIToUTF16("650"), - ASCIIToUTF16("2345678"), - "US", - autofill_i18n::RFC3966, - &number)); - EXPECT_EQ(number, ASCIIToUTF16("tel:+1-650-234-5678")); - EXPECT_TRUE(ConstructPhoneNumber(string16(), - ASCIIToUTF16("650"), - ASCIIToUTF16("2345678"), + string16(), + ASCIIToUTF16("6502345678"), "US", - autofill_i18n::INTERNATIONAL, &number)); EXPECT_EQ(number, ASCIIToUTF16("+1 650-234-5678")); EXPECT_TRUE(ConstructPhoneNumber(string16(), string16(), ASCIIToUTF16("6502345678"), "US", - autofill_i18n::INTERNATIONAL, &number)); - EXPECT_EQ(number, ASCIIToUTF16("+1 650-234-5678")); + EXPECT_EQ(number, ASCIIToUTF16("(650) 234-5678")); EXPECT_FALSE(ConstructPhoneNumber(string16(), ASCIIToUTF16("650"), ASCIIToUTF16("234567890"), "US", - autofill_i18n::INTERNATIONAL, &number)); EXPECT_EQ(number, string16()); // Italian number - EXPECT_TRUE(ConstructPhoneNumber(string16(), + EXPECT_TRUE(ConstructPhoneNumber(ASCIIToUTF16("39"), ASCIIToUTF16("347"), ASCIIToUTF16("2345678"), "IT", - autofill_i18n::INTERNATIONAL, &number)); EXPECT_EQ(number, ASCIIToUTF16("+39 347 234 5678")); + EXPECT_TRUE(ConstructPhoneNumber(string16(), + ASCIIToUTF16("347"), + ASCIIToUTF16("2345678"), + "IT", + &number)); + EXPECT_EQ(number, ASCIIToUTF16("347 234 5678")); // German number. EXPECT_TRUE(ConstructPhoneNumber(ASCIIToUTF16("49"), ASCIIToUTF16("024"), ASCIIToUTF16("2345678901"), "DE", - autofill_i18n::NATIONAL, &number)); - EXPECT_EQ(number, ASCIIToUTF16("02423/45678901")); - - EXPECT_TRUE(ConstructPhoneNumber(ASCIIToUTF16("49"), + EXPECT_EQ(number, ASCIIToUTF16("+49 2423/45678901")); + EXPECT_TRUE(ConstructPhoneNumber(string16(), ASCIIToUTF16("024"), ASCIIToUTF16("2345678901"), "DE", - autofill_i18n::INTERNATIONAL, &number)); - EXPECT_EQ(number, ASCIIToUTF16("+49 2423/45678901")); + EXPECT_EQ(number, ASCIIToUTF16("02423/45678901")); } -TEST_F(PhoneNumberI18NTest, PhoneNumbersMatch) { +TEST(PhoneNumberI18NTest, PhoneNumbersMatch) { // Same numbers, defined country code. EXPECT_TRUE(PhoneNumbersMatch(ASCIIToUTF16("4158889999"), ASCIIToUTF16("4158889999"), diff --git a/chrome/browser/autofill/phone_number_unittest.cc b/chrome/browser/autofill/phone_number_unittest.cc index 0aa83ed..5c96aab 100644 --- a/chrome/browser/autofill/phone_number_unittest.cc +++ b/chrome/browser/autofill/phone_number_unittest.cc @@ -80,6 +80,64 @@ TEST(PhoneNumberTest, Matcher) { matching_types.end()); } +// Verify that PhoneNumber::SetInfo() correctly formats the incoming number. +TEST(PhoneNumberTest, SetInfo) { + AutofillProfile profile; + profile.SetCountryCode("US"); + + PhoneNumber phone(&profile); + EXPECT_EQ(string16(), phone.GetRawInfo(PHONE_HOME_WHOLE_NUMBER)); + + // Set the formatted info directly. + EXPECT_TRUE(phone.SetInfo(PHONE_HOME_WHOLE_NUMBER, + ASCIIToUTF16("(650) 234-5678"), "US")); + EXPECT_EQ(ASCIIToUTF16("(650) 234-5678"), + phone.GetRawInfo(PHONE_HOME_WHOLE_NUMBER)); + + // Unformatted numbers should be formatted. + EXPECT_TRUE(phone.SetInfo(PHONE_HOME_WHOLE_NUMBER, + ASCIIToUTF16("8887776666"), "US")); + EXPECT_EQ(ASCIIToUTF16("(888) 777-6666"), + phone.GetRawInfo(PHONE_HOME_WHOLE_NUMBER)); + + // Differently formatted numbers should be re-formatted. + EXPECT_TRUE(phone.SetInfo(PHONE_HOME_WHOLE_NUMBER, + ASCIIToUTF16("800-432-8765"), "US")); + EXPECT_EQ(ASCIIToUTF16("(800) 432-8765"), + phone.GetRawInfo(PHONE_HOME_WHOLE_NUMBER)); + + // Invalid numbers should not be stored. In the US, phone numbers cannot + // start with the digit '1'. + EXPECT_FALSE(phone.SetInfo(PHONE_HOME_WHOLE_NUMBER, + ASCIIToUTF16("650111111"), "US")); + EXPECT_EQ(string16(), phone.GetRawInfo(PHONE_HOME_WHOLE_NUMBER)); +} + +// Test that cached phone numbers are correctly invalidated and updated. +TEST(PhoneNumberTest, UpdateCachedPhoneNumber) { + AutofillProfile profile; + profile.SetCountryCode("US"); + + PhoneNumber phone(&profile); + phone.SetRawInfo(PHONE_HOME_WHOLE_NUMBER, ASCIIToUTF16("6502345678")); + EXPECT_EQ(ASCIIToUTF16("650"), phone.GetInfo(PHONE_HOME_CITY_CODE, "US")); + + // Update the area code. + phone.SetRawInfo(PHONE_HOME_WHOLE_NUMBER, ASCIIToUTF16("8322345678")); + EXPECT_EQ(ASCIIToUTF16("832"), phone.GetInfo(PHONE_HOME_CITY_CODE, "US")); + + // Change the phone number to have a UK format, but try to parse with the + // wrong locale. + phone.SetRawInfo(PHONE_HOME_WHOLE_NUMBER, ASCIIToUTF16("07023456789")); + EXPECT_EQ(string16(), phone.GetInfo(PHONE_HOME_CITY_CODE, "US")); + + // Now try parsing using the correct locale. Note that the profile's country + // code should override the app locale, which is still set to "US". + profile.SetCountryCode("GB"); + phone.SetRawInfo(PHONE_HOME_WHOLE_NUMBER, ASCIIToUTF16("07023456789")); + EXPECT_EQ(ASCIIToUTF16("70"), phone.GetInfo(PHONE_HOME_CITY_CODE, "US")); +} + TEST(PhoneNumberTest, PhoneCombineHelper) { AutofillProfile profile; profile.SetCountryCode("US"); diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index af7d366..6ca7ade 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -1666,6 +1666,7 @@ '../third_party/cld/cld.gyp:cld', '../third_party/leveldatabase/leveldatabase.gyp:leveldatabase', '../third_party/libjingle/libjingle.gyp:libjingle', + '../third_party/libphonenumber/libphonenumber.gyp:libphonenumber', '../tools/json_schema_compiler/test/json_schema_compiler_tests.gyp:json_schema_compiler_tests', '../ui/gl/gl.gyp:gl', '../v8/tools/gyp/v8.gyp:v8', |