[Autofill] Fix phone upload bug.

components/autofill/core/browser/phone_number.cc

Index: components/autofill/core/browser/phone_number.cc
diff --git a/components/autofill/core/browser/phone_number.cc b/components/autofill/core/browser/phone_number.cc
index 1952e58568f87dfce82ee1614c7089059b538e79..c986d9bb8c6f308b81f404776ddbdd16a8b2dca9 100644
--- a/components/autofill/core/browser/phone_number.cc
+++ b/components/autofill/core/browser/phone_number.cc
@@ -149,6 +149,9 @@ 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".

[Mathieu, 2016/01/27 18:48:35]

Mention that FormGroup::GetMatchingTypes will only do simple canonicalization in
order to match against the stored data, hence why we need more domain-specific
knowledge here and cover special cases below.

[sebsg, 2016/01/27 20:00:43]

Done.

   base::string16 stripped_text = text;
   base::RemoveChars(stripped_text, base::ASCIIToUTF16(" .()-"), &stripped_text);
   FormGroup::GetMatchingTypes(stripped_text, app_locale, matching_types);
@@ -164,6 +167,12 @@ void PhoneNumber::GetMatchingTypes(const base::string16& text,
       matching_types->insert(PHONE_HOME_NUMBER);
   }
 
+  // TODO(crbug.com/581391): Investigate the use of phonenumberutil when

[Mathieu, 2016/01/27 18:48:35]

*PhoneNumberUtil

[sebsg, 2016/01/27 20:00:43]

Done.

+  // matching phone numbers for upload.
+  // Normalizes the |text| based on the app_locale before comparing it to the
+  // whole number. For example, the number from France "33 2 49 19 70 70" would

[Mathieu, 2016/01/27 18:48:35]

*the France number and *the US number

[sebsg, 2016/01/27 20:00:43]

Done.

+  // be normalized to "+33249197070" whereas the number from the US
+  // "+1 (234) 567-8901" would be normalized to "12345678901".
   base::string16 whole_number =
       GetInfo(AutofillType(PHONE_HOME_WHOLE_NUMBER), app_locale);
   if (!whole_number.empty()) {
@@ -172,6 +181,18 @@ void PhoneNumber::GetMatchingTypes(const base::string16& text,
     if (normalized_number == whole_number)
       matching_types->insert(PHONE_HOME_WHOLE_NUMBER);

[Mathieu, 2016/01/27 18:48:35]

I think there's a case where we could be doubly adding PHOME_HOME_WHOLE_NUMBER
here (say it was matched in FormGroup::GetMatchingTypes). I guess it doesn't
matter because |matching_types| is an std::set, but perhaps let's add a check on
line 178 for  matching_types->find(PHONE_HOME_WHOLE_NUMBER) ==
matching_types->end()

[sebsg, 2016/01/27 20:00:43]

Done.

   }
+
+  // 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

[Mathieu, 2016/01/27 18:48:35]

nit: space after .

[sebsg, 2016/01/27 20:00:43]

Done.

+  // 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);
+  }
 }
 
 void PhoneNumber::UpdateCacheIfNeeded(const std::string& app_locale) const {