[Autofill] Fix phone upload bug.

components/autofill/core/browser/phone_number.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/autofill/core/browser/phone_number.h"
 
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/autofill/core/browser/autofill_country.h"
@@ skipping 128 matching lines @@
   if (base::ContainsOnlyChars(number_, base::ASCIIToUTF16("+0123456789"))) {
     number_ = cached_parsed_phone_.GetFormattedNumber();
   } else if (i18n::NormalizePhoneNumber(
                  number_, GetRegion(*profile_, app_locale)).empty()) {
     // The number doesn't make sense for this region; clear it.
     number_.clear();
   }
   return !number_.empty();
 }
 
 void PhoneNumber::GetMatchingTypes(const base::string16& text,

[Mathieu, 2016/01/25 20:32:16]

Since you have the context in your head, please enhance comments in this method
for each code block (I see that you have started and it's a good idea). I would
like to see clearly described examples of phone numbers
(WHOLE/HOME_NUMBER/CITY_AND_NUMBER), what are the values at each step of the way
(maybe before/after i18n::NormalizePhoneNumber), etc. Think of a reader who has
little context on this and make it clear to them.

[sebsg, 2016/01/26 21:31:07]

Done.

                                    const std::string& app_locale,
                                    ServerFieldTypeSet* matching_types) const {
   base::string16 stripped_text = text;
   base::RemoveChars(stripped_text, base::ASCIIToUTF16(" .()-"), &stripped_text);
   FormGroup::GetMatchingTypes(stripped_text, app_locale, matching_types);
 
   // 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.
   base::string16 number = GetInfo(AutofillType(PHONE_HOME_NUMBER), app_locale);
   if (GetRegion(*profile_, app_locale) == "US" &&
       number.size() == (kPrefixLength + kSuffixLength)) {
     base::string16 prefix = number.substr(kPrefixOffset, kPrefixLength);
     base::string16 suffix = number.substr(kSuffixOffset, kSuffixLength);
     if (text == prefix || text == suffix)
       matching_types->insert(PHONE_HOME_NUMBER);
   }
 
+  // To match whole phone numbers with the "+" in front of the country code.

[Mathieu, 2016/01/25 20:32:16]

is the + always present?

[sebsg, 2016/01/26 21:31:07]

I put a better comment.

   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)
+    if (normalized_number == whole_number) {

[Mathieu, 2016/01/25 20:32:16]

revert change to add {}

[sebsg, 2016/01/26 21:31:07]

Done.

       matching_types->insert(PHONE_HOME_WHOLE_NUMBER);
+    }
+  }
+
+  // If both PHONE_HOME_CITY_AND_NUMBER and PHONE_HOME_WHOLE_NUMBER, it means

[Mathieu, 2016/01/25 20:32:16]

nit: sentence is missing a few words

[sebsg, 2016/01/26 21:31:07]

Done.

+  // there is no country code and we should only return
+  // PHONE_HOME_CITY_AND_NUMBER.
+  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 {
   std::string region = GetRegion(*profile_, app_locale);
   if (!number_.empty() && cached_parsed_phone_.region() != region)
     cached_parsed_phone_ = i18n::PhoneObject(number_, region);
 }
 
 PhoneNumber::PhoneCombineHelper::PhoneCombineHelper() {
@@ skipping 47 matching lines @@
 
   return i18n::ConstructPhoneNumber(
       country_, city_, phone_, GetRegion(profile, app_locale), value);
 }
 
 bool PhoneNumber::PhoneCombineHelper::IsEmpty() const {
   return phone_.empty() && whole_number_.empty();
 }
 
 }  // namespace autofill