Commit 60e4597e authored by Matthias Körber's avatar Matthias Körber Committed by Commit Bot

[Autofill] Import addresses from section union as a fallback.

This CL relaxes the requirements to import a profile. If no address \
profile could be imported from a form section, the import is performed
on the union of all sections.

Change-Id: Id366c8acc5ab98deb130c592dad507ef7f4cf5c5
Bug: 1097692
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2254603
Commit-Queue: Matthias Körber <koerber@google.com>
Reviewed-by: default avatarDominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#780729}
parent df39977d
......@@ -466,6 +466,17 @@ bool FormDataImporter::ImportAddressProfiles(const FormStructure& form) {
// And close the div of the section import log.
import_log_buffer << CTag{"div"};
}
// TODO(crbug.com/1097125): Remove feature test.
// Run the import on the union of the section if the import was not
// successful and if there is more than one section.
if (num_saved_profiles == 0 &&
base::FeatureList::IsEnabled(
features::kAutofillProfileImportFromUnifiedSection) &&
sections.size() > 1) {
// Try to import by combining all sections.
if (ImportAddressProfileForSection(form, "", &import_log_buffer))
num_saved_profiles++;
}
}
import_log_buffer << LogMessage::kImportAddressProfileFromFormNumberOfImports
<< num_saved_profiles << CTag{};
......@@ -509,7 +520,8 @@ bool FormDataImporter::ImportAddressProfileForSection(
// 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|.
if (field->section != section)
// If section is empty, use all fields.
if (field->section != section && !section.empty())
continue;
base::string16 value;
......
......@@ -114,7 +114,8 @@ class FormDataImporter {
bool ImportAddressProfiles(const FormStructure& form);
// Helper method for ImportAddressProfiles which only considers the fields for
// a specified |section|.
// a specified |section|. If |section| is the empty string, the import is
// performed on the union of all sections.
bool ImportAddressProfileForSection(const FormStructure& form,
const std::string& section,
LogBuffer* import_log_buffer);
......
......@@ -319,6 +319,60 @@ TEST_F(FormDataImporterTest, ImportAddressProfiles) {
EXPECT_EQ(0, expected.Compare(*results[0]));
}
TEST_F(FormDataImporterTest, ImportAddressProfileFromUnifiedSection) {
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("Email:", "email", "theprez@gmail.com", "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);
FormStructure form_structure(form);
form_structure.DetermineHeuristicTypes();
// Assign the address field another section than the other fields.
form_structure.field(3)->section = "another_section";
base::test::ScopedFeatureList scoped_feature;
scoped_feature.InitAndDisableFeature(
features::kAutofillProfileImportFromUnifiedSection);
// Without the feature, the import is expected to fail.
ImportAddressProfiles(/*extraction_success=*/false, form_structure);
// After enabled the feature, the import is expected to succeed.
scoped_feature.Reset();
scoped_feature.InitAndEnableFeature(
features::kAutofillProfileImportFromUnifiedSection);
ImportAddressProfiles(/*extraction_success=*/true, form_structure);
AutofillProfile expected(base::GenerateGUID(), test::kEmptyOrigin);
test::SetProfileInfo(&expected, "George", nullptr, "Washington",
"theprez@gmail.com", nullptr, "21 Laussat St", nullptr,
"San Francisco", "California", "94102", nullptr,
nullptr);
const std::vector<AutofillProfile*>& results2 =
personal_data_manager_->GetProfiles();
ASSERT_EQ(1U, results2.size());
EXPECT_EQ(0, expected.Compare(*results2[0]));
}
TEST_F(FormDataImporterTest, ImportAddressProfiles_BadEmail) {
FormData form;
form.url = GURL("https://wwww.foo.com");
......
......@@ -131,6 +131,10 @@ const base::Feature kAutofillPreferServerNamePredictions{
const base::Feature kAutofillProfileClientValidation{
"AutofillProfileClientValidation", base::FEATURE_DISABLED_BY_DEFAULT};
const base::Feature kAutofillProfileImportFromUnifiedSection{
"AutofillProfileImportFromUnifiedSection",
base::FEATURE_DISABLED_BY_DEFAULT};
// Controls whether Autofill uses server-side validation to ensure that fields
// with invalid data are not suggested.
const base::Feature kAutofillProfileServerValidation{
......
......@@ -43,6 +43,7 @@ extern const base::Feature kAutofillOffNoServerData;
extern const base::Feature kAutofillOverrideWithRaterConsensus;
extern const base::Feature kAutofillPreferServerNamePredictions;
extern const base::Feature kAutofillProfileClientValidation;
extern const base::Feature kAutofillProfileImportFromUnifiedSection;
extern const base::Feature kAutofillProfileServerValidation;
extern const base::Feature kAutofillRejectCompanyBirthyear;
extern const base::Feature kAutofillRejectCompanySocialTitle;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment