Another performance improvement for phone library - at least +25% to previous cl (982ms for 100 items DB test comared to 1251ms)

But feels like much more as the penalty shifted to DB thread.
TEST=The caching object is tested by multiple AutofillManager, PersonalDataManager, etc. unittests + new unit-test to test performance.
BUG=85152
Review URL: http://codereview.chromium.org/7044102

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@90434 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 201 insertions, 90 deletions

diff --git a/chrome/browser/autofill/phone_number.cc b/chrome/browser/autofill/phone_number.cc
index d426396..57d44a1 100644
--- a/chrome/browser/autofill/phone_number.cc
+++ b/chrome/browser/autofill/phone_number.cc
@@ -54,7 +54,7 @@ PhoneNumber& PhoneNumber::operator=(const PhoneNumber& number) {
     return *this;
   number_ = number.number_;
   phone_group_ = number.phone_group_;
-  ClearCachedNumbers();
+  cached_parsed_phone_ = number.cached_parsed_phone_;
   return *this;
 }
 
@@ -63,20 +63,20 @@ void PhoneNumber::GetMatchingTypes(const string16& text,
   string16 stripped_text(text);
   StripPunctuation(&stripped_text);
 
-  if (!UpdateCacheIfNeeded())
+  if (!cached_parsed_phone_.IsValidNumber())
     return;
 
-  if (IsNumber(stripped_text, cached_local_number_))
+  if (IsNumber(stripped_text, cached_parsed_phone_.GetNumber()))
     matching_types->insert(GetNumberType());
 
-  if (stripped_text == cached_city_code_)
+  if (stripped_text == cached_parsed_phone_.GetCityCode())
     matching_types->insert(GetCityCodeType());
 
-  if (stripped_text == cached_country_code_)
+  if (stripped_text == cached_parsed_phone_.GetCountryCode())
     matching_types->insert(GetCountryCodeType());
 
-  string16 city_and_local(cached_city_code_);
-  city_and_local.append(cached_local_number_);
+  string16 city_and_local(cached_parsed_phone_.GetCityCode());
+  city_and_local.append(cached_parsed_phone_.GetNumber());
   if (stripped_text == city_and_local)
     matching_types->insert(GetCityAndNumberType());
 
@@ -94,37 +94,38 @@ void PhoneNumber::GetNonEmptyTypes(FieldTypeSet* non_empty_types) const {
 
   non_empty_types->insert(GetWholeNumberType());
 
-  if (!UpdateCacheIfNeeded())
+  if (!cached_parsed_phone_.IsValidNumber())
     return;
 
   non_empty_types->insert(GetNumberType());
 
-  if (!cached_city_code_.empty()) {
+  if (!cached_parsed_phone_.GetCityCode().empty()) {
     non_empty_types->insert(GetCityCodeType());
     non_empty_types->insert(GetCityAndNumberType());
   }
 
-  if (!cached_country_code_.empty())
+  if (!cached_parsed_phone_.GetCountryCode().empty())
     non_empty_types->insert(GetCountryCodeType());
 }
 
 string16 PhoneNumber::GetInfo(AutofillFieldType type) const {
   if (type == GetWholeNumberType())
     return number_;
-  if (!UpdateCacheIfNeeded())
+
+  if (!cached_parsed_phone_.IsValidNumber())
     return string16();
 
   if (type == GetNumberType())
-    return cached_local_number_;
+    return cached_parsed_phone_.GetNumber();
 
   if (type == GetCityCodeType())
-    return cached_city_code_;
+    return cached_parsed_phone_.GetCityCode();
 
   if (type == GetCountryCodeType())
-    return cached_country_code_;
+    return cached_parsed_phone_.GetCountryCode();
 
-  string16 city_and_local(cached_city_code_);
-  city_and_local.append(cached_local_number_);
+  string16 city_and_local(cached_parsed_phone_.GetCityCode());
+  city_and_local.append(cached_parsed_phone_.GetNumber());
   if (type == GetCityAndNumberType())
     return city_and_local;
 
@@ -136,7 +137,6 @@ void PhoneNumber::SetInfo(AutofillFieldType type, const string16& value) {
   FieldTypeGroup group = AutofillType(type).group();
   if (phone_group_ == AutofillType::NO_GROUP)
     phone_group_ = group;  // First call on empty phone - set the group.
-  ClearCachedNumbers();
   if (subgroup == AutofillType::PHONE_NUMBER) {
     // Should not be set directly.
     NOTREACHED();
@@ -149,6 +149,7 @@ void PhoneNumber::SetInfo(AutofillFieldType type, const string16& value) {
   } else if (subgroup == AutofillType::PHONE_CITY_AND_NUMBER ||
              subgroup == AutofillType::PHONE_WHOLE_NUMBER) {
     number_ = value;
+    cached_parsed_phone_ = autofill_i18n::PhoneObject(number_, locale_);
     StripPunctuation(&number_);
   } else {
     NOTREACHED();
@@ -160,14 +161,14 @@ bool PhoneNumber::NormalizePhone() {
   if (number_.empty())
     return true;
 
-  ClearCachedNumbers();
-  number_ = autofill_i18n::NormalizePhoneNumber(number_, locale_);
+  number_ = cached_parsed_phone_.GetWholeNumber();
   return !number_.empty();
 }
 
 void PhoneNumber::set_locale(const std::string& locale) {
   locale_ = locale;
-  ClearCachedNumbers();
+  if (!number_.empty() && cached_parsed_phone_.GetLocale() != locale_)
+    cached_parsed_phone_ = autofill_i18n::PhoneObject(number_, locale_);
 }
 
 AutofillFieldType PhoneNumber::GetNumberType() const {
@@ -269,27 +270,11 @@ bool PhoneNumber::IsNumber(const string16& text, const string16& number) const {
 }
 
 bool PhoneNumber::IsWholeNumber(const string16& text) const {
-  return autofill_i18n::ComparePhones(text, number_, locale_) ==
-         autofill_i18n::PHONES_EQUAL;
+  return cached_parsed_phone_.ComparePhones(text) ==
+      autofill_i18n::PHONES_EQUAL;
 }
 
 // Static.
 void PhoneNumber::StripPunctuation(string16* number) {
   RemoveChars(*number, kPhoneNumberSeparators, number);
 }
-
-void PhoneNumber::ClearCachedNumbers() const {
-  cached_country_code_.clear();
-  cached_city_code_.clear();
-  cached_local_number_.clear();
-}
-
-bool PhoneNumber::UpdateCacheIfNeeded() const {
-  if (cached_local_number_.empty() && !number_.empty()) {
-    return autofill_i18n::ParsePhoneNumber(
-        number_, locale_, &cached_country_code_, &cached_city_code_,
-        &cached_local_number_);
-  } else {
-    return true;
-  }
-}
diff --git a/chrome/browser/autofill/phone_number.h b/chrome/browser/autofill/phone_number.h
index fb47f18..cd168c9 100644
--- a/chrome/browser/autofill/phone_number.h
+++ b/chrome/browser/autofill/phone_number.h
@@ -13,6 +13,7 @@
 #include "base/string16.h"
 #include "chrome/browser/autofill/autofill_type.h"
 #include "chrome/browser/autofill/form_group.h"
+#include "chrome/browser/autofill/phone_number_i18n.h"
 
 // A form group that stores phone number information.
 class PhoneNumber : public FormGroup {
@@ -84,11 +85,6 @@ class PhoneNumber : public FormGroup {
 
   static void StripPunctuation(string16* number);
 
-  void ClearCachedNumbers() const;
-  // Updates cached parsed parts of the number. Returns false only if there was
-  // parsing involved and it failed.
-  bool UpdateCacheIfNeeded() const;
-
   // Phone group -  currently it is PHONE_HOME and PHONE_FAX.
   AutofillType::FieldTypeGroup phone_group_;
   // Locale for phone normalizing.
@@ -96,10 +92,8 @@ class PhoneNumber : public FormGroup {
   // The phone number.
   string16 number_;
 
-  // Cached parsed parts of the number.
-  mutable string16 cached_country_code_;
-  mutable string16 cached_city_code_;
-  mutable string16 cached_local_number_;
+  // Cached number.
+  autofill_i18n::PhoneObject cached_parsed_phone_;
 };
 
 #endif  // CHROME_BROWSER_AUTOFILL_PHONE_NUMBER_H_
diff --git a/chrome/browser/autofill/phone_number_i18n.cc b/chrome/browser/autofill/phone_number_i18n.cc
index 961058c..c26f7c5 100644
--- a/chrome/browser/autofill/phone_number_i18n.cc
+++ b/chrome/browser/autofill/phone_number_i18n.cc
@@ -38,41 +38,16 @@ i18n::phonenumbers::PhoneNumberUtil::PhoneNumberFormat UtilsTypeToPhoneLibType(
   return i18n::phonenumbers::PhoneNumberUtil::NATIONAL;
 }
 
-}  // namespace
-
-namespace autofill_i18n {
-
-string16 NormalizePhoneNumber(const string16& value,
-                              std::string const& locale) {
-  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, locale, &country_code, &city_code, &number))
-    return string16();  // Parsing failed - do not store phone.
-  if (!autofill_i18n::ConstructPhoneNumber(
-          country_code, city_code, number,
-          locale,
-          (country_code.empty() ?
-              autofill_i18n::NATIONAL : autofill_i18n::INTERNATIONAL),
-          &result)) {
-    // Reconstruction failed - do not store phone.
-    return string16();
-  }
-  std::string result_utf8(UTF16ToUTF8(result));
-  i18n::phonenumbers::PhoneNumberUtil::NormalizeDigitsOnly(&result_utf8);
-  return UTF8ToUTF16(result_utf8);
-}
-
-bool ParsePhoneNumber(const string16& value,
-                      const std::string& locale,
-                      string16* country_code,
-                      string16* city_code,
-                      string16* number) {
+bool ParsePhoneNumberInternal(const string16& value,
+                              const std::string& locale,
+                              string16* country_code,
+                              string16* city_code,
+                              string16* number,
+                              i18n::phonenumbers::PhoneNumber* i18n_number) {
   DCHECK(number);
   DCHECK(city_code);
   DCHECK(country_code);
+  DCHECK(i18n_number);
 
   number->clear();
   city_code->clear();
@@ -81,38 +56,37 @@ bool ParsePhoneNumber(const string16& value,
   std::string number_text(UTF16ToUTF8(value));
 
   // Parse phone number based on the locale.
-  i18n::phonenumbers::PhoneNumber i18n_number;
   i18n::phonenumbers::PhoneNumberUtil* phone_util =
       i18n::phonenumbers::PhoneNumberUtil::GetInstance();
   DCHECK(phone_util);
 
   if (phone_util->Parse(number_text, SanitizeLocaleCode(locale).c_str(),
-                        &i18n_number) !=
+                        i18n_number) !=
       i18n::phonenumbers::PhoneNumberUtil::NO_PARSING_ERROR) {
     return false;
   }
 
   i18n::phonenumbers::PhoneNumberUtil::ValidationResult validation =
-      phone_util->IsPossibleNumberWithReason(i18n_number);
+      phone_util->IsPossibleNumberWithReason(*i18n_number);
   if (validation != i18n::phonenumbers::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 (!phone_util->IsValidNumber(*i18n_number))
     return false;
 
   std::string national_significant_number;
-  phone_util->GetNationalSignificantNumber(i18n_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 area_length = phone_util->GetLengthOfGeographicalAreaCode(*i18n_number);
   int destination_length =
-      phone_util->GetLengthOfNationalDestinationCode(i18n_number);
+      phone_util->GetLengthOfNationalDestinationCode(*i18n_number);
   // Some phones have a destination code in lieu of area code: mobile operators
   // in Europe, toll and toll-free numbers in USA, etc. From our point of view
   // these two types of codes are the same.
@@ -132,9 +106,9 @@ bool ParsePhoneNumber(const string16& value,
   string16 normalized_number(UTF8ToUTF16(number_text));
   // Check if parsed number has country code and it was not inferred from the
   // locale.
-  if (i18n_number.has_country_code()) {
-    *country_code = UTF8ToUTF16(base::StringPrintf("%d",
-                                                   i18n_number.country_code()));
+  if (i18n_number->has_country_code()) {
+    *country_code = UTF8ToUTF16(
+         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))) {
@@ -145,6 +119,43 @@ bool ParsePhoneNumber(const string16& value,
   return true;
 }
 
+}  // namespace
+
+namespace autofill_i18n {
+
+string16 NormalizePhoneNumber(const string16& value,
+                              std::string const& locale) {
+  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, locale, &country_code, &city_code, &number))
+    return string16();  // Parsing failed - do not store phone.
+  if (!autofill_i18n::ConstructPhoneNumber(
+          country_code, city_code, number,
+          locale,
+          (country_code.empty() ?
+              autofill_i18n::NATIONAL : autofill_i18n::INTERNATIONAL),
+          &result)) {
+    // Reconstruction failed - do not store phone.
+    return string16();
+  }
+  std::string result_utf8(UTF16ToUTF8(result));
+  i18n::phonenumbers::PhoneNumberUtil::NormalizeDigitsOnly(&result_utf8);
+  return UTF8ToUTF16(result_utf8);
+}
+
+bool ParsePhoneNumber(const string16& value,
+                      const std::string& locale,
+                      string16* country_code,
+                      string16* city_code,
+                      string16* number) {
+  i18n::phonenumbers::PhoneNumber i18n_number;
+  return ParsePhoneNumberInternal(value, locale, country_code, city_code,
+                                  number, &i18n_number);
+}
+
 bool ConstructPhoneNumber(const string16& country_code,
                           const string16& city_code,
                           const string16& number,
@@ -265,5 +276,90 @@ bool PhoneNumbersMatch(const string16& number_a,
   return ComparePhones(number_a, number_b, country_code) == PHONES_EQUAL;
 }
 
+PhoneObject::PhoneObject(const string16& number, const std::string& locale)
+    : locale_(SanitizeLocaleCode(locale)),
+      i18n_number_(NULL) {
+  scoped_ptr<i18n::phonenumbers::PhoneNumber>
+      i18n_number(new i18n::phonenumbers::PhoneNumber);
+  if (ParsePhoneNumberInternal(number, locale_, &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().
+    i18n_number_.reset(i18n_number.release());
+  } else {
+    // Parsing failed. Store passed phone "as is" into |whole_number_|.
+    whole_number_ = number;
+  }
+}
+
+PhoneObject::PhoneObject(const PhoneObject& other)
+    : i18n_number_(NULL) {
+  *this = other;
+}
+
+PhoneObject::PhoneObject()
+    : i18n_number_(NULL) {
+}
+
+PhoneObject::~PhoneObject() {
+}
+
+string16 PhoneObject::GetWholeNumber() const {
+  if (i18n_number_.get() && whole_number_.empty()) {
+    i18n::phonenumbers::PhoneNumberUtil::PhoneNumberFormat format =
+        i18n::phonenumbers::PhoneNumberUtil::INTERNATIONAL;
+    if (country_code_.empty())
+      format = i18n::phonenumbers::PhoneNumberUtil::NATIONAL;
+
+    std::string formatted_number;
+    i18n::phonenumbers::PhoneNumberUtil* phone_util =
+        i18n::phonenumbers::PhoneNumberUtil::GetInstance();
+    phone_util->Format(*i18n_number_, format, &formatted_number);
+    i18n::phonenumbers::PhoneNumberUtil::NormalizeDigitsOnly(&formatted_number);
+    whole_number_ = UTF8ToUTF16(formatted_number);
+  }
+  return whole_number_;
+}
+
+PhoneMatch PhoneObject::ComparePhones(const string16& phone_number) const {
+  PhoneObject phone(phone_number, locale_);
+  if (!i18n_number_.get() || !phone.i18n_number_.get()) {
+    if (GetWholeNumber().empty())
+      return PHONES_NOT_EQUAL;
+    return (GetWholeNumber() == phone.GetWholeNumber()) ? PHONES_EQUAL :
+                                                          PHONES_NOT_EQUAL;
+  }
+
+  i18n::phonenumbers::PhoneNumberUtil* phone_util =
+      i18n::phonenumbers::PhoneNumberUtil::GetInstance();
+  switch (phone_util->IsNumberMatch(*i18n_number_, *(phone.i18n_number_))) {
+    case i18n::phonenumbers::PhoneNumberUtil::INVALID_NUMBER:
+    case i18n::phonenumbers::PhoneNumberUtil::NO_MATCH:
+      return PHONES_NOT_EQUAL;
+    case i18n::phonenumbers::PhoneNumberUtil::SHORT_NSN_MATCH:
+      return PHONES_SUBMATCH;
+    case i18n::phonenumbers::PhoneNumberUtil::NSN_MATCH:
+    case i18n::phonenumbers::PhoneNumberUtil::EXACT_MATCH:
+      return PHONES_EQUAL;
+    default:
+      NOTREACHED();
+  }
+  return PHONES_NOT_EQUAL;
+}
+
+PhoneObject& PhoneObject::operator=(const PhoneObject& other) {
+  if (this == &other)
+    return *this;
+  country_code_ = other.country_code_;
+  city_code_ = other.city_code_;
+  number_ = other.number_;
+  locale_ = other.locale_;
+  if (other.i18n_number_.get()) {
+    i18n_number_.reset(new i18n::phonenumbers::PhoneNumber(
+        *other.i18n_number_));
+  }
+  return *this;
+}
+
 }  // namespace autofill_i18n
 
diff --git a/chrome/browser/autofill/phone_number_i18n.h b/chrome/browser/autofill/phone_number_i18n.h
index 9d75cab..a8ecef7 100644
--- a/chrome/browser/autofill/phone_number_i18n.h
+++ b/chrome/browser/autofill/phone_number_i18n.h
@@ -10,10 +10,16 @@
 #include <vector>
 
 #include "base/compiler_specific.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/string16.h"
 
-// Utilities to process, normalize and compare international phone numbers.
+namespace i18n {
+namespace phonenumbers {
+class PhoneNumber;
+}
+}
 
+// Utilities to process, normalize and compare international phone numbers.
 namespace autofill_i18n {
 
 // Most of the following functions require |locale| to operate. The |locale| is
@@ -88,6 +94,36 @@ bool PhoneNumbersMatch(const string16& number_a,
                        const string16& number_b,
                        const std::string& country_code);
 
+// The cached phone number, does parsing only once, improves performance.
+class PhoneObject {
+ public:
+  PhoneObject(const string16& number, const std::string& locale);
+  PhoneObject(const PhoneObject&);
+  PhoneObject();
+  ~PhoneObject();
+
+  string16 GetCountryCode() const { return country_code_; }
+  string16 GetCityCode() const { return city_code_; }
+  string16 GetNumber() const { return number_; }
+  std::string GetLocale() const { return locale_; }
+
+  string16 GetWholeNumber() const;
+
+  PhoneMatch ComparePhones(const string16& phone) const;
+
+  PhoneObject& operator=(const PhoneObject& other);
+
+  bool IsValidNumber() const { return i18n_number_ != NULL; }
+
+ private:
+  string16 city_code_;
+  string16 country_code_;
+  string16 number_;
+  mutable string16 whole_number_;  // Set on first request.
+  std::string locale_;
+  scoped_ptr<i18n::phonenumbers::PhoneNumber> i18n_number_;
+};
+
 }  // namespace autofill_i18n
 
 #endif  // CHROME_BROWSER_AUTOFILL_PHONE_NUMBER_I18N_H_