[autofill]Always override autocomplete=tel/tel-* with server prediction for phones.

Many web sites set autocomplete to wrong tel types. For example, they set it to
tel but then expects a national number(Sephora.com). This change overrides all fields
with autocomplete attribute set to tel/tel-* with server predictions, as long as
server thinks it's a phone number field as well.

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

components/autofill/core/browser/autofill_field.cc

@@ -91,18 +91,19 @@ void AutofillField::SetTypeTo(ServerFieldType type) {
 }
 
 AutofillType AutofillField::Type() const {
+  // If autocomplete=tel/tel-* and server confirms it really is a phone field,
+  // we always user the server prediction as html types are not very reliable.
+  if ((GroupTypeOfHtmlFieldType(html_type_, html_mode_) == PHONE_BILLING ||
+       GroupTypeOfHtmlFieldType(html_type_, html_mode_) == PHONE_HOME) &&
+      (GroupTypeOfServerFieldType(overall_server_type_) == PHONE_BILLING ||
+       GroupTypeOfServerFieldType(overall_server_type_) == PHONE_HOME)) {
+    return AutofillType(overall_server_type_);
+  }
+
   // Use the html type specified by the website unless it is unrecognized and
   // autofill predicts a credit card type.
   if (html_type_ != HTML_TYPE_UNSPECIFIED &&
       !(html_type_ == HTML_TYPE_UNRECOGNIZED && IsCreditCardPrediction())) {
-    // If autocomplete=tel and server says it's a phone field without country
-    // code, we override html prediction(autocomplete=tel leads to whole number
-    // prediction).
-    if (html_type_ == HTML_TYPE_TEL &&
-        overall_server_type_ == PHONE_HOME_CITY_AND_NUMBER) {
-      return AutofillType(PHONE_HOME_CITY_AND_NUMBER);
-    }
-
     return AutofillType(html_type_, html_mode_);
   }
 

components/autofill/core/browser/autofill_type.cc

@@ -8,40 +8,8 @@
 
 namespace autofill {
 
-AutofillType::AutofillType(ServerFieldType field_type)
-    : html_type_(HTML_TYPE_UNSPECIFIED), html_mode_(HTML_MODE_NONE) {
-  if ((field_type < NO_SERVER_DATA || field_type >= MAX_VALID_FIELD_TYPE) ||
-      (field_type >= 15 && field_type <= 19) ||
-      (field_type >= 25 && field_type <= 29) ||
-      (field_type >= 44 && field_type <= 50)) {
-    server_type_ = UNKNOWN_TYPE;
-  } else {
-    server_type_ = field_type;
-  }
-}
-
-AutofillType::AutofillType(HtmlFieldType field_type, HtmlFieldMode mode)
-    : server_type_(UNKNOWN_TYPE),
-      html_type_(field_type),
-      html_mode_(mode) {}
-
-
-AutofillType::AutofillType(const AutofillType& autofill_type) {
-  *this = autofill_type;
-}
-
-AutofillType& AutofillType::operator=(const AutofillType& autofill_type) {
-  if (this != &autofill_type) {
-    this->server_type_ = autofill_type.server_type_;
-    this->html_type_ = autofill_type.html_type_;
-    this->html_mode_ = autofill_type.html_mode_;
-  }
-
-  return *this;
-}
-
-FieldTypeGroup AutofillType::group() const {
-  switch (server_type_) {
+FieldTypeGroup GroupTypeOfServerFieldType(ServerFieldType field_type) {
+  switch (field_type) {
     case NAME_FIRST:
     case NAME_MIDDLE:
     case NAME_LAST:
@@ -150,16 +118,21 @@ FieldTypeGroup AutofillType::group() const {
       return USERNAME_FIELD;
 
     case UNKNOWN_TYPE:
-      break;
+      return NO_GROUP;
+    default:
+      return NO_GROUP;
   }
+}
 
-  switch (html_type_) {
+FieldTypeGroup GroupTypeOfHtmlFieldType(HtmlFieldType field_type,
+                                        HtmlFieldMode field_mode) {
+  switch (field_type) {
     case HTML_TYPE_NAME:
     case HTML_TYPE_GIVEN_NAME:
     case HTML_TYPE_ADDITIONAL_NAME:
     case HTML_TYPE_ADDITIONAL_NAME_INITIAL:
     case HTML_TYPE_FAMILY_NAME:
-      return html_mode_ == HTML_MODE_BILLING ? NAME_BILLING : NAME;
+      return field_mode == HTML_MODE_BILLING ? NAME_BILLING : NAME;
 
     case HTML_TYPE_ORGANIZATION:
       return COMPANY;
@@ -175,7 +148,7 @@ FieldTypeGroup AutofillType::group() const {
     case HTML_TYPE_COUNTRY_NAME:
     case HTML_TYPE_POSTAL_CODE:
     case HTML_TYPE_FULL_ADDRESS:
-      return html_mode_ == HTML_MODE_BILLING ? ADDRESS_BILLING : ADDRESS_HOME;
+      return field_mode == HTML_MODE_BILLING ? ADDRESS_BILLING : ADDRESS_HOME;
 
     case HTML_TYPE_CREDIT_CARD_NAME_FULL:
     case HTML_TYPE_CREDIT_CARD_NAME_FIRST:
@@ -204,21 +177,46 @@ FieldTypeGroup AutofillType::group() const {
     case HTML_TYPE_TEL_LOCAL_PREFIX:
     case HTML_TYPE_TEL_LOCAL_SUFFIX:
     case HTML_TYPE_TEL_EXTENSION:
-      return html_mode_ == HTML_MODE_BILLING ? PHONE_BILLING : PHONE_HOME;
+      return field_mode == HTML_MODE_BILLING ? PHONE_BILLING : PHONE_HOME;
 
     case HTML_TYPE_EMAIL:
       return EMAIL;
 
     case HTML_TYPE_UPI_VPA:
       // TODO(crbug/702223): Add support for UPI-VPA.
-      break;
+      return NO_GROUP;
 
     case HTML_TYPE_UNSPECIFIED:
     case HTML_TYPE_UNRECOGNIZED:
-      break;
+      return NO_GROUP;
+    default:
+      return NO_GROUP;
   }
+}
 
-  return NO_GROUP;
+AutofillType::AutofillType(ServerFieldType field_type)
+    : html_type_(HTML_TYPE_UNSPECIFIED), html_mode_(HTML_MODE_NONE) {
+  if ((field_type < NO_SERVER_DATA || field_type >= MAX_VALID_FIELD_TYPE) ||
+      (field_type >= 15 && field_type <= 19) ||
+      (field_type >= 25 && field_type <= 29) ||
+      (field_type >= 44 && field_type <= 50)) {
+    server_type_ = UNKNOWN_TYPE;
+  } else {
+    server_type_ = field_type;
+  }
+}
+
+AutofillType::AutofillType(HtmlFieldType field_type, HtmlFieldMode mode)
+    : server_type_(UNKNOWN_TYPE), html_type_(field_type), html_mode_(mode) {}
+
+FieldTypeGroup AutofillType::group() const {
+  FieldTypeGroup result = NO_GROUP;
+  if (server_type_ != UNKNOWN_TYPE) {
+    result = GroupTypeOfServerFieldType(server_type_);
+  } else {
+    result = GroupTypeOfHtmlFieldType(html_type_, html_mode_);
+  }
+  return result;
 }
 
 bool AutofillType::IsUnknown() const {

components/autofill/core/browser/autofill_type.h

@@ -11,14 +11,23 @@
 
 namespace autofill {
 
+// Help method that takes a |ServerFieldType| and returns it's corresponding
+// |FieldTypeGroup| value.
+FieldTypeGroup GroupTypeOfServerFieldType(ServerFieldType field_type);
+
+// Help method that takes a |HtmlFieldType| and |HtmlFieldMode|, then returns
+// their corresponding |FieldTypeGroup| value.
+FieldTypeGroup GroupTypeOfHtmlFieldType(HtmlFieldType field_type,
+                                        HtmlFieldMode field_mode);
+
 // The high-level description of Autofill types, used to categorize form fields
 // and for associating form fields with form values in the Web Database.
 class AutofillType {
  public:
   explicit AutofillType(ServerFieldType field_type);
   AutofillType(HtmlFieldType field_type, HtmlFieldMode mode);
-  AutofillType(const AutofillType& autofill_type);
-  AutofillType& operator=(const AutofillType& autofill_type);
+  AutofillType(const AutofillType& autofill_type) = default;
+  AutofillType& operator=(const AutofillType& autofill_type) = default;
 
   HtmlFieldType html_type() const { return html_type_; }
 

components/autofill/core/browser/field_filler_unittest.cc

@@ -211,8 +211,9 @@ TEST_F(AutofillFieldFillerTest, Type_CreditCardOverrideHtml_ServerPredicitons) {
   EXPECT_EQ(NAME_FULL, field.Type().GetStorableType());
 }
 
-// Tests that a server prediction of *_CITY_AND_NUMBER override html
-// "autocomplete=tel" prediction, which is always *_WHOLE_NUMBER
+// Tests that if both autocomplete attibutes and server agree it's a phone
+// field, always use server predicted type. If they disagree with autocomplete
+// says it's a phone field, always use autocomplete attribute.
 TEST_F(AutofillFieldFillerTest,
        Type_ServerPredictionOfCityAndNumber_OverrideHtml) {
   AutofillField field;
@@ -222,11 +223,22 @@ TEST_F(AutofillFieldFillerTest,
   field.set_overall_server_type(PHONE_HOME_CITY_AND_NUMBER);
   EXPECT_EQ(PHONE_HOME_CITY_AND_NUMBER, field.Type().GetStorableType());
 
-  // Other phone number prediction does not override.
+  // Overrides to another number format.
   field.set_overall_server_type(PHONE_HOME_NUMBER);
+  EXPECT_EQ(PHONE_HOME_NUMBER, field.Type().GetStorableType());
+
+  // Overrides autocomplete=tel-national too.
+  field.SetHtmlType(HTML_TYPE_TEL_NATIONAL, HTML_MODE_NONE);
+  field.set_overall_server_type(PHONE_HOME_WHOLE_NUMBER);
   EXPECT_EQ(PHONE_HOME_WHOLE_NUMBER, field.Type().GetStorableType());
 
-  // If html type not specified, we use server prediction.
+  // If autocomplete=tel-national but server says it's not a phone field,
+  // do not override.
+  field.SetHtmlType(HTML_TYPE_TEL_NATIONAL, HTML_MODE_NONE);
+  field.set_overall_server_type(CREDIT_CARD_EXP_DATE_2_DIGIT_YEAR);
+  EXPECT_EQ(PHONE_HOME_CITY_AND_NUMBER, field.Type().GetStorableType());
+
+  // If html type not specified, we still use server prediction.
   field.SetHtmlType(HTML_TYPE_UNSPECIFIED, HTML_MODE_NONE);
   field.set_overall_server_type(PHONE_HOME_CITY_AND_NUMBER);
   EXPECT_EQ(PHONE_HOME_CITY_AND_NUMBER, field.Type().GetStorableType());