[Autofill] Best effort filling for invalid phone numbers.

At the moment autofill refuses to fill anything if the web site is asking
for a complete phone number but what user has saved is considered as
invalid by libphonenumber.

We should relex this restriction and fill the phone field with exactly what
the user has saved, as long as it's a field expecting complete number.

Phone number components field will still be left empty.

Bug: 793400
Change-Id: Ief7689185dfdf8c29526f620f80f95fed9a6d69c
Reviewed-on: https://chromium-review.googlesource.com/818015
Commit-Queue: Hui Wu <wuandy@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: Roger McFarlane <rogerm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523140}

components/autofill/core/browser/phone_number.cc

@@ -31,9 +31,7 @@ std::string GetRegion(const AutofillProfile& profile,
 
 }  // namespace
 
-PhoneNumber::PhoneNumber(AutofillProfile* profile)
-    : profile_(profile) {
-}
+PhoneNumber::PhoneNumber(AutofillProfile* profile) : profile_(profile) {}
 
 PhoneNumber::PhoneNumber(const PhoneNumber& number) : profile_(nullptr) {
   *this = number;
@@ -143,12 +141,16 @@ base::string16 PhoneNumber::GetInfoImpl(const AutofillType& type,
   ServerFieldType storable_type = type.GetStorableType();
   UpdateCacheIfNeeded(app_locale);
 
-  // Queries for whole numbers will return the non-normalized number if
-  // normalization for the number fails.  All other field types require
-  // normalization.
-  if (storable_type != PHONE_HOME_WHOLE_NUMBER &&
-      !cached_parsed_phone_.IsValidNumber())
+  // When the phone number autofill has stored cannot be normalized, it
+  // responds to queries for complete numbers with whatever the raw stored value
+  // is, and simply return empty string for any queries for phone components.
+  if (!cached_parsed_phone_.IsValidNumber()) {
+    if (storable_type == PHONE_HOME_WHOLE_NUMBER ||
+        storable_type == PHONE_HOME_CITY_AND_NUMBER) {
+      return cached_parsed_phone_.GetWholeNumber();
+    }
     return base::string16();
+  }
 
   switch (storable_type) {
     case PHONE_HOME_WHOLE_NUMBER:
@@ -164,8 +166,7 @@ base::string16 PhoneNumber::GetInfoImpl(const AutofillType& type,
       return cached_parsed_phone_.country_code();
 
     case PHONE_HOME_CITY_AND_NUMBER:
-      return
-          cached_parsed_phone_.city_code() + cached_parsed_phone_.number();
+      return cached_parsed_phone_.city_code() + cached_parsed_phone_.number();
 
     case PHONE_HOME_EXTENSION:
       return base::string16();
@@ -189,26 +190,24 @@ bool PhoneNumber::SetInfoImpl(const AutofillType& type,
   UpdateCacheIfNeeded(app_locale);
   if (base::ContainsOnlyChars(number_, base::ASCIIToUTF16("+0123456789"))) {
     number_ = cached_parsed_phone_.GetFormattedNumber();
-  } else if (i18n::NormalizePhoneNumber(
-                 number_, GetRegion(*profile_, app_locale)).empty()) {
+  } 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::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() {
-}
+PhoneNumber::PhoneCombineHelper::PhoneCombineHelper() {}
 
-PhoneNumber::PhoneCombineHelper::~PhoneCombineHelper() {
-}
+PhoneNumber::PhoneCombineHelper::~PhoneCombineHelper() {}
 
 bool PhoneNumber::PhoneCombineHelper::SetInfo(const AutofillType& type,
                                               const base::string16& value) {
@@ -253,8 +252,8 @@ bool PhoneNumber::PhoneCombineHelper::ParseNumber(
     return true;
   }
 
-  return i18n::ConstructPhoneNumber(
-      country_, city_, phone_, GetRegion(profile, app_locale), value);
+  return i18n::ConstructPhoneNumber(country_, city_, phone_,
+                                    GetRegion(profile, app_locale), value);
 }
 
 bool PhoneNumber::PhoneCombineHelper::IsEmpty() const {

components/autofill/core/browser/phone_number_unittest.cc

@@ -2,12 +2,13 @@
 // 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/string16.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/autofill/core/browser/autofill_profile.h"
 #include "components/autofill/core/browser/autofill_type.h"
 #include "components/autofill/core/browser/field_types.h"
-#include "components/autofill/core/browser/phone_number.h"
 #include "components/autofill/core/browser/phone_number_i18n.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -105,8 +106,7 @@ TEST(PhoneNumberTest, SetInfo) {
   EXPECT_EQ(ASCIIToUTF16("(888) 777-6666"),
             phone.GetRawInfo(PHONE_HOME_WHOLE_NUMBER));
   EXPECT_TRUE(phone.SetInfo(AutofillType(PHONE_HOME_WHOLE_NUMBER),
-                            ASCIIToUTF16("+18887776666"),
-                            "US"));
+                            ASCIIToUTF16("+18887776666"), "US"));
   EXPECT_EQ(ASCIIToUTF16("1 888-777-6666"),
             phone.GetRawInfo(PHONE_HOME_WHOLE_NUMBER));
 
@@ -126,6 +126,16 @@ TEST(PhoneNumberTest, SetInfo) {
   EXPECT_FALSE(phone.SetInfo(AutofillType(PHONE_HOME_WHOLE_NUMBER),
                              ASCIIToUTF16("650111111"), "US"));
   EXPECT_EQ(base::string16(), phone.GetRawInfo(PHONE_HOME_WHOLE_NUMBER));
+
+  // If the stored number is invalid, we still try to respond to queries
+  // for a complete number with what user has entered.
+  EXPECT_FALSE(phone.SetInfo(AutofillType(PHONE_HOME_WHOLE_NUMBER),
+                             ASCIIToUTF16("5141231234"), "US"));
+  EXPECT_EQ(ASCIIToUTF16("5141231234"),
+            phone.GetInfo(PHONE_HOME_CITY_AND_NUMBER, "CA"));
+  EXPECT_EQ(ASCIIToUTF16("5141231234"),
+            phone.GetInfo(PHONE_HOME_WHOLE_NUMBER, "CA"));
+  EXPECT_EQ(base::string16(), phone.GetInfo(PHONE_HOME_CITY_CODE, "CA"));
 }
 
 // Test that cached phone numbers are correctly invalidated and updated.
@@ -162,12 +172,12 @@ TEST(PhoneNumberTest, PhoneCombineHelper) {
   profile.SetRawInfo(ADDRESS_HOME_COUNTRY, ASCIIToUTF16("US"));
 
   PhoneNumber::PhoneCombineHelper number1;
-  EXPECT_FALSE(number1.SetInfo(AutofillType(ADDRESS_BILLING_CITY),
-                               ASCIIToUTF16("1")));
+  EXPECT_FALSE(
+      number1.SetInfo(AutofillType(ADDRESS_BILLING_CITY), ASCIIToUTF16("1")));
   EXPECT_TRUE(number1.SetInfo(AutofillType(PHONE_HOME_COUNTRY_CODE),
                               ASCIIToUTF16("1")));
-  EXPECT_TRUE(number1.SetInfo(AutofillType(PHONE_HOME_CITY_CODE),
-                              ASCIIToUTF16("650")));
+  EXPECT_TRUE(
+      number1.SetInfo(AutofillType(PHONE_HOME_CITY_CODE), ASCIIToUTF16("650")));
   EXPECT_TRUE(number1.SetInfo(AutofillType(PHONE_HOME_NUMBER),
                               ASCIIToUTF16("2345678")));
   base::string16 parsed_phone;
@@ -176,8 +186,8 @@ TEST(PhoneNumberTest, PhoneCombineHelper) {
   EXPECT_EQ(ASCIIToUTF16("1 650-234-5678"), parsed_phone);
 
   PhoneNumber::PhoneCombineHelper number3;
-  EXPECT_TRUE(number3.SetInfo(AutofillType(PHONE_HOME_CITY_CODE),
-                              ASCIIToUTF16("650")));
+  EXPECT_TRUE(
+      number3.SetInfo(AutofillType(PHONE_HOME_CITY_CODE), ASCIIToUTF16("650")));
   EXPECT_TRUE(number3.SetInfo(AutofillType(PHONE_HOME_NUMBER),
                               ASCIIToUTF16("2345680")));
   EXPECT_TRUE(number3.ParseNumber(profile, "en-US", &parsed_phone));
@@ -199,24 +209,24 @@ TEST(PhoneNumberTest, PhoneCombineHelper) {
   EXPECT_EQ(ASCIIToUTF16("(650) 234-5681"), parsed_phone);
 
   PhoneNumber::PhoneCombineHelper number6;
-  EXPECT_TRUE(number6.SetInfo(AutofillType(PHONE_HOME_CITY_CODE),
-                              ASCIIToUTF16("650")));
-  EXPECT_TRUE(number6.SetInfo(AutofillType(PHONE_HOME_NUMBER),
-                              ASCIIToUTF16("234")));
-  EXPECT_TRUE(number6.SetInfo(AutofillType(PHONE_HOME_NUMBER),
-                              ASCIIToUTF16("5682")));
+  EXPECT_TRUE(
+      number6.SetInfo(AutofillType(PHONE_HOME_CITY_CODE), ASCIIToUTF16("650")));
+  EXPECT_TRUE(
+      number6.SetInfo(AutofillType(PHONE_HOME_NUMBER), ASCIIToUTF16("234")));
+  EXPECT_TRUE(
+      number6.SetInfo(AutofillType(PHONE_HOME_NUMBER), ASCIIToUTF16("5682")));
   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(AutofillType(PHONE_HOME_CITY_CODE),
-                              ASCIIToUTF16("650")));
-  EXPECT_TRUE(number7.SetInfo(AutofillType(PHONE_HOME_NUMBER),
-                              ASCIIToUTF16("234")));
-  EXPECT_TRUE(number7.SetInfo(AutofillType(PHONE_HOME_NUMBER),
-                              ASCIIToUTF16("5682")));
+  EXPECT_TRUE(
+      number7.SetInfo(AutofillType(PHONE_HOME_CITY_CODE), ASCIIToUTF16("650")));
+  EXPECT_TRUE(
+      number7.SetInfo(AutofillType(PHONE_HOME_NUMBER), ASCIIToUTF16("234")));
+  EXPECT_TRUE(
+      number7.SetInfo(AutofillType(PHONE_HOME_NUMBER), ASCIIToUTF16("5682")));
   EXPECT_TRUE(number7.ParseNumber(AutofillProfile(), "en-US", &parsed_phone));
   EXPECT_EQ(ASCIIToUTF16("(650) 234-5682"), parsed_phone);
 }