[Autofill] Import first number from form with multiple phone numbers

Prior to this change, when the form contained multiple phone numbers,
the import was aborted since Autofill currently supports storing only
one phone number per profile.

Now with AutofillEnableImportWhenMultiplePhoneNumbers enabled we try to
import the first phone number and ignore all other phone components in
the form.

Bug: 1154727
Change-Id: I47dff6abcf862819ad23809c0fc684dfa3ada06f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2632675Reviewed-by: Matthias Körber <koerber@google.com>
Reviewed-by: Christoph Schwering <schwering@google.com>
Commit-Queue: Elizabeth Popova <lizapopova@google.com>
Cr-Commit-Position: refs/heads/master@{#844525}

components/autofill/core/browser/form_data_importer.cc

@@ -58,11 +58,21 @@ bool IsValidFieldTypeAndValue(const std::set<ServerFieldType>& types_seen,
                               LogBuffer* import_log_buffer) {
   // Abandon the import if two fields of the same type are encountered.
   // This indicates ambiguous data or miscategorization of types.
-  // Make an exception for PHONE_HOME_NUMBER however as both prefix and
-  // suffix are stored against this type, and for EMAIL_ADDRESS because it is
-  // common to see second 'confirm email address' fields on forms.
-  if (types_seen.count(field_type) && field_type != PHONE_HOME_NUMBER &&
-      field_type != EMAIL_ADDRESS) {
+  // Make an exception for:
+  // - EMAIL_ADDRESS because it is common to see second 'confirm email address'
+  // field;
+  // - PHONE_HOME_NUMBER because it is used to store both prefix and suffix of a
+  // single number;
+  // - phone number components because a form might request several phone
+  // numbers.
+  // TODO(crbug.com/1156315) Remove feature & PHONE_HOME_NUMBER checks when
+  // launched.
+  auto field_type_group = AutofillType(field_type).group();
+  if (types_seen.count(field_type) && field_type != EMAIL_ADDRESS &&
+      (base::FeatureList::IsEnabled(
+           features::kAutofillEnableImportWhenMultiplePhoneNumbers)
+           ? field_type_group != PHONE_BILLING && field_type_group != PHONE_HOME
+           : field_type != PHONE_HOME_NUMBER)) {
     if (import_log_buffer) {
       *import_log_buffer << LogMessage::kImportAddressProfileFromFormFailed
                          << "Multiple fields of type "
@@ -529,6 +539,10 @@ bool FormDataImporter::ImportAddressProfileForSection(
   // Tracks if the form section contains an invalid country.
   bool has_invalid_country = false;
 
+  // Tracks if subsequent phone number fields should be ignored,
+  // since they do not belong to the first phone number in the form.
+  bool ignore_phone_number_fields = false;
+
   // Go through each |form| field and attempt to constitute a valid profile.
   for (const auto& field : form) {
     // Reject fields that are not within the specified |section|.
@@ -576,6 +590,27 @@ bool FormDataImporter::ImportAddressProfileForSection(
     if (!IsValidFieldTypeAndValue(types_seen, server_field_type, value,
                                   import_log_buffer))
       has_invalid_field_types = true;
+
+    // Found phone number component field.
+    // TODO(crbug.com/1156315) Remove feature check when launched.
+    if ((field_type.group() == PHONE_BILLING ||
+         field_type.group() == PHONE_HOME) &&
+        base::FeatureList::IsEnabled(
+            features::kAutofillEnableImportWhenMultiplePhoneNumbers)) {
+      if (ignore_phone_number_fields)
+        continue;
+      // PHONE_HOME_NUMBER is used for both prefix and suffix, so it might occur
+      // multiple times for a single number. Duplication of any other phone
+      // component means it belongs to a new number. Since Autofill currently
+      // supports storing only one phone number per profile, ignore this and all
+      // subsequent phone number fields.
+      if (server_field_type != PHONE_HOME_NUMBER &&
+          types_seen.count(server_field_type)) {
+        ignore_phone_number_fields = true;
+        continue;
+      }
+    }
+
     types_seen.insert(server_field_type);
 
     // We need to store phone data in the variables, before building the whole

components/autofill/core/browser/form_data_importer_unittest.cc

@@ -762,6 +762,121 @@ TEST_P(FormDataImporterTest, ImportAddressProfiles_TwoDifferentEmails) {
   ASSERT_EQ(0U, personal_data_manager_->GetProfiles().size());
 }
 
+// Tests that multiple phone numbers do not block profile import and the first
+// one is saved.
+TEST_P(FormDataImporterTest, ImportAddressProfiles_MultiplePhoneNumbers) {
+  base::test::ScopedFeatureList enable_import_when_multiple_phones_feature;
+  enable_import_when_multiple_phones_feature.InitAndEnableFeature(
+      features::kAutofillEnableImportWhenMultiplePhoneNumbers);
+
+  FormData form;
+  form.url = GURL("https://wwww.foo.com");
+
+  FormFieldData field;
+  test::CreateTestFormField("First name:", "first_name", "George", "text",
+                            &field);
+  form.fields.push_back(field);
+  test::CreateTestFormField("Last name:", "last_name", "Washington", "text",
+                            &field);
+  form.fields.push_back(field);
+  test::CreateTestFormField("Address:", "address1", "21 Laussat St", "text",
+                            &field);
+  form.fields.push_back(field);
+  test::CreateTestFormField("City:", "city", "San Francisco", "text", &field);
+  form.fields.push_back(field);
+  test::CreateTestFormField("State:", "state", "California", "text", &field);
+  form.fields.push_back(field);
+  test::CreateTestFormField("Zip:", "zip", "94102", "text", &field);
+  form.fields.push_back(field);
+  test::CreateTestFormField("Phone 1:", "phone1", "+16505550000", "text",
+                            &field);
+  form.fields.push_back(field);
+  test::CreateTestFormField("Phone 2:", "phone2", "+14155551234", "text",
+                            &field);
+  form.fields.push_back(field);
+
+  FormStructure form_structure(form);
+  form_structure.DetermineHeuristicTypes();
+  ImportAddressProfiles(/*extraction_successful=*/true, form_structure);
+
+  AutofillProfile expected(base::GenerateGUID(), test::kEmptyOrigin);
+  test::SetProfileInfo(&expected, "George", nullptr, "Washington", nullptr,
+                       nullptr, "21 Laussat St", nullptr, "San Francisco",
+                       "California", "94102", nullptr, "1 650-555-0000");
+
+  const std::vector<AutofillProfile*>& results =
+      personal_data_manager_->GetProfiles();
+  ASSERT_EQ(1U, results.size());
+  EXPECT_EQ(0, expected.Compare(*results[0]));
+}
+
+// Tests that multiple phone numbers do not block profile import and the first
+// one is saved.
+TEST_P(FormDataImporterTest,
+       ImportAddressProfiles_MultiplePhoneNumbersSplitAcrossMultipleFields) {
+  base::test::ScopedFeatureList enable_import_when_multiple_phones_feature;
+  enable_import_when_multiple_phones_feature.InitAndEnableFeature(
+      features::kAutofillEnableImportWhenMultiplePhoneNumbers);
+
+  FormData form;
+  form.url = GURL("https://wwww.foo.com");
+
+  FormFieldData field;
+  test::CreateTestFormField("First name:", "first_name", "George", "text",
+                            &field);
+  form.fields.push_back(field);
+  test::CreateTestFormField("Last name:", "last_name", "Washington", "text",
+                            &field);
+  form.fields.push_back(field);
+  test::CreateTestFormField("Address:", "address1", "21 Laussat St", "text",
+                            &field);
+  form.fields.push_back(field);
+  test::CreateTestFormField("City:", "city", "San Francisco", "text", &field);
+  form.fields.push_back(field);
+  test::CreateTestFormField("State:", "state", "California", "text", &field);
+  form.fields.push_back(field);
+  test::CreateTestFormField("Zip:", "zip", "94102", "text", &field);
+  form.fields.push_back(field);
+  test::CreateTestFormField("Phone 1:", "home_phone_area_code1", "650", "text",
+                            &field);
+  field.max_length = 3;
+  form.fields.push_back(field);
+  test::CreateTestFormField("Phone 1:", "home_phone_prefix1", "555", "text",
+                            &field);
+  field.max_length = 3;
+  form.fields.push_back(field);
+  test::CreateTestFormField("Phone 1:", "home_phone_suffix1", "0000", "text",
+                            &field);
+  field.max_length = 4;
+  form.fields.push_back(field);
+  test::CreateTestFormField("Phone 2:", "home_phone_area_code2", "202", "text",
+                            &field);
+  field.max_length = 3;
+  form.fields.push_back(field);
+  test::CreateTestFormField("Phone 2:", "home_phone_prefix2", "555", "text",
+                            &field);
+  field.max_length = 3;
+  form.fields.push_back(field);
+  test::CreateTestFormField("Phone 2:", "home_phone_suffix2", "1234", "text",
+                            &field);
+  field.max_length = 4;
+  form.fields.push_back(field);
+
+  FormStructure form_structure(form);
+  form_structure.DetermineHeuristicTypes();
+  ImportAddressProfiles(/*extraction_successful=*/true, form_structure);
+
+  AutofillProfile expected(base::GenerateGUID(), test::kEmptyOrigin);
+  test::SetProfileInfo(&expected, "George", nullptr, "Washington", nullptr,
+                       nullptr, "21 Laussat St", nullptr, "San Francisco",
+                       "California", "94102", nullptr, "(650) 555-0000");
+
+  const std::vector<AutofillProfile*>& results =
+      personal_data_manager_->GetProfiles();
+  ASSERT_EQ(1U, results.size());
+  EXPECT_EQ(0, expected.Compare(*results[0]));
+}
+
 // Tests that not enough filled fields will result in not importing an address.
 TEST_P(FormDataImporterTest, ImportAddressProfiles_NotEnoughFilledFields) {
   FormData form;

components/autofill/core/common/autofill_features.cc

@@ -98,6 +98,13 @@ const base::Feature kAutofillEnableDependentLocalityParsing{
 const base::Feature kAutofillEnableHideSuggestionsUI{
     "AutofillEnableHideSuggestionsUI", base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Controls whether to save the first number in a form with multiple phone
+// numbers instead of aborting the import.
+// TODO(crbug.com/1167484) Remove once launched
+const base::Feature kAutofillEnableImportWhenMultiplePhoneNumbers{
+    "AutofillEnableImportWhenMultiplePhoneNumbers",
+    base::FEATURE_DISABLED_BY_DEFAULT};
+
 // When enabled and user has single account, a footer indicating user's e-mail
 // address and profile picture will appear at the bottom of InfoBars which has
 // corresponding account indication footer flags on.

components/autofill/core/common/autofill_features.h

@@ -33,6 +33,7 @@ extern const base::Feature kAutofillEnableAccountWalletStorage;
 extern const base::Feature kAutofillEnableAugmentedPhoneCountryCode;
 extern const base::Feature kAutofillEnableDependentLocalityParsing;
 extern const base::Feature kAutofillEnableHideSuggestionsUI;
+extern const base::Feature kAutofillEnableImportWhenMultiplePhoneNumbers;
 extern const base::Feature
     kAutofillEnableInfoBarAccountIndicationFooterForSingleAccountUsers;
 extern const base::Feature