Commit d6fe2aca authored by Roger McFarlane's avatar Roger McFarlane Committed by Commit Bot

[autofill] Support consolidated uploads for a field.

This CL changes the uploaded autofilltype (i.e. vote) field to be repeating instead of a single required field per uploaded field.

Background: The autofill upload proto has a repeated "Field" attribute that has a single required autofilltype field. Autofill can send multiple votes for a given field (if a field matches the users full address as well as address line 1, for example). Prior to this CL, clients would send multiple "Field" entries (repeating all attached metadata) in the event of multiple votes. Clients also send multiple "Field" entries if there are fields having the same signature (address line 1,2, sharing the same field name/id, for example, is not uncommon). The recipient of these votes is not able to tell these 2 cases apart (multiple votes for a single field vs votes for multiple fields with the same signature).

With this CL, clients will upload exactly one "Field" entry per logical field in the form. In this way all type votes for a field will be consolsidated to a single Field entry. It will also consolidate (instead of repeating) any additional metadata send about the field into that single entry for the field.

Bug: 850606
Change-Id: I73acea17f755b6ebe37f11c9e72e02f9bf1c5484
Reviewed-on: https://chromium-review.googlesource.com/1142485
Commit-Queue: Roger McFarlane <rogerm@chromium.org>
Reviewed-by: default avatarSebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582158}
parent e0fdc66e
...@@ -258,8 +258,11 @@ std::ostream& operator<<(std::ostream& out, ...@@ -258,8 +258,11 @@ std::ostream& operator<<(std::ostream& out,
for (const auto& field : upload.field()) { for (const auto& field : upload.field()) {
out << "\n Field" out << "\n Field"
<< "\n signature: " << field.signature() << "\n signature: " << field.signature() << "\n autofill_type: ["
<< "\n autofill_type: " << field.autofill_type(); << field.autofill_type(0);
for (int i = 1; i < field.autofill_type_size(); ++i)
out << ", " << field.autofill_type(i);
out << "]";
if (!field.name().empty()) if (!field.name().empty())
out << "\n name: " << field.name(); out << "\n name: " << field.name();
if (!field.autocomplete().empty()) if (!field.autocomplete().empty())
......
...@@ -576,7 +576,7 @@ void FillUploadField(AutofillUploadContents::Field* field, ...@@ -576,7 +576,7 @@ void FillUploadField(AutofillUploadContents::Field* field,
field->set_type(control_type); field->set_type(control_type);
if (autocomplete) if (autocomplete)
field->set_autocomplete(autocomplete); field->set_autocomplete(autocomplete);
field->set_autofill_type(autofill_type); field->add_autofill_type(autofill_type);
} }
void FillQueryField(AutofillQueryContents::Form::Field* field, void FillQueryField(AutofillQueryContents::Form::Field* field,
......
...@@ -1573,44 +1573,45 @@ void FormStructure::EncodeFormForUpload(AutofillUploadContents* upload) const { ...@@ -1573,44 +1573,45 @@ void FormStructure::EncodeFormForUpload(AutofillUploadContents* upload) const {
if (IsCheckable(field->check_status)) if (IsCheckable(field->check_status))
continue; continue;
const ServerFieldTypeSet& types = field->possible_types(); // Add the same field elements as the query and a few more below.
for (const auto& field_type : types) { if (ShouldSkipField(*field))
// Add the same field elements as the query and a few more below. continue;
if (ShouldSkipField(*field))
continue;
AutofillUploadContents::Field* added_field = upload->add_field(); auto* added_field = upload->add_field();
added_field->set_autofill_type(field_type);
if (field->generation_type()) {
added_field->set_generation_type(field->generation_type());
added_field->set_generated_password_changed(
field->generated_password_changed());
}
if (field->vote_type()) { for (const auto& field_type : field->possible_types()) {
added_field->set_vote_type(field->vote_type()); added_field->add_autofill_type(field_type);
} }
if (field->generation_type()) {
added_field->set_generation_type(field->generation_type());
added_field->set_generated_password_changed(
field->generated_password_changed());
}
added_field->set_signature(field->GetFieldSignature()); if (field->vote_type()) {
added_field->set_vote_type(field->vote_type());
}
added_field->set_signature(field->GetFieldSignature());
if (field->properties_mask) if (field->properties_mask)
added_field->set_properties_mask(field->properties_mask); added_field->set_properties_mask(field->properties_mask);
if (IsAutofillFieldMetadataEnabled()) { if (IsAutofillFieldMetadataEnabled()) {
added_field->set_type(field->form_control_type); added_field->set_type(field->form_control_type);
if (!field->name.empty()) if (!field->name.empty())
added_field->set_name(base::UTF16ToUTF8(field->name)); added_field->set_name(base::UTF16ToUTF8(field->name));
if (!field->id.empty()) if (!field->id.empty())
added_field->set_id(base::UTF16ToUTF8(field->id)); added_field->set_id(base::UTF16ToUTF8(field->id));
if (!field->autocomplete_attribute.empty()) if (!field->autocomplete_attribute.empty())
added_field->set_autocomplete(field->autocomplete_attribute); added_field->set_autocomplete(field->autocomplete_attribute);
if (!field->css_classes.empty()) if (!field->css_classes.empty())
added_field->set_css_classes(base::UTF16ToUTF8(field->css_classes)); added_field->set_css_classes(base::UTF16ToUTF8(field->css_classes));
}
} }
} }
} }
......
...@@ -2471,18 +2471,18 @@ TEST_F(FormStructureTest, EncodeUploadRequest) { ...@@ -2471,18 +2471,18 @@ TEST_F(FormStructureTest, EncodeUploadRequest) {
// Adjust the expected proto string. // Adjust the expected proto string.
upload.set_form_signature(7816485729218079147U); upload.set_form_signature(7816485729218079147U);
upload.set_autofill_used(false); upload.set_autofill_used(false);
// Create an additonal 8 fields (total of 13). // Create an additional 2 fields (total of 7).
for (int i = 0; i < 8; ++i) { for (int i = 0; i < 2; ++i) {
test::FillUploadField(upload.add_field(), 509334676U, "address", "text", test::FillUploadField(upload.add_field(), 509334676U, "address", "text",
nullptr, 30U); nullptr, 30U);
} }
// Put the appropriate autofill type on the different address fields. // Put the appropriate autofill type on the different address fields.
upload.mutable_field(6)->set_autofill_type(31U); upload.mutable_field(5)->add_autofill_type(31U);
upload.mutable_field(7)->set_autofill_type(37U); upload.mutable_field(5)->add_autofill_type(37U);
upload.mutable_field(8)->set_autofill_type(38U); upload.mutable_field(5)->add_autofill_type(38U);
upload.mutable_field(10)->set_autofill_type(31U); upload.mutable_field(6)->add_autofill_type(31U);
upload.mutable_field(11)->set_autofill_type(37U); upload.mutable_field(6)->add_autofill_type(37U);
upload.mutable_field(12)->set_autofill_type(38U); upload.mutable_field(6)->add_autofill_type(38U);
ASSERT_TRUE(upload.SerializeToString(&expected_upload_string)); ASSERT_TRUE(upload.SerializeToString(&expected_upload_string));
AutofillUploadContents encoded_upload3; AutofillUploadContents encoded_upload3;
...@@ -3553,14 +3553,9 @@ TEST_F(FormStructureTest, CheckMultipleTypes) { ...@@ -3553,14 +3553,9 @@ TEST_F(FormStructureTest, CheckMultipleTypes) {
form_structure->field(2)->set_possible_types(possible_field_types[2]); form_structure->field(2)->set_possible_types(possible_field_types[2]);
// Modify the expected upload. // Modify the expected upload.
// Put the NAME_FIRST prediction on the third field. // Add the NAME_FIRST prediction to the third field.
upload.mutable_field(2)->set_autofill_type(3); upload.mutable_field(2)->add_autofill_type(3);
// Replace the fourth field by the old third field. upload.mutable_field(2)->mutable_autofill_type()->SwapElements(0, 1);
test::FillUploadField(upload.mutable_field(3), 2404144663U, "last", "text",
nullptr, 5U);
// Re-add the old fourth field.
test::FillUploadField(upload.add_field(), 509334676U, "address", "text",
nullptr, 30U);
ASSERT_TRUE(upload.SerializeToString(&expected_upload_string)); ASSERT_TRUE(upload.SerializeToString(&expected_upload_string));
...@@ -3578,8 +3573,7 @@ TEST_F(FormStructureTest, CheckMultipleTypes) { ...@@ -3578,8 +3573,7 @@ TEST_F(FormStructureTest, CheckMultipleTypes) {
possible_field_types[form_structure->field_count() - 1]); possible_field_types[form_structure->field_count() - 1]);
// Adjust the expected upload proto. // Adjust the expected upload proto.
test::FillUploadField(upload.add_field(), 509334676U, "address", "text", upload.mutable_field(3)->add_autofill_type(31U);
nullptr, 31U);
ASSERT_TRUE(upload.SerializeToString(&expected_upload_string)); ASSERT_TRUE(upload.SerializeToString(&expected_upload_string));
AutofillUploadContents encoded_upload3; AutofillUploadContents encoded_upload3;
...@@ -3598,7 +3592,7 @@ TEST_F(FormStructureTest, CheckMultipleTypes) { ...@@ -3598,7 +3592,7 @@ TEST_F(FormStructureTest, CheckMultipleTypes) {
possible_field_types[form_structure->field_count() - 1]); possible_field_types[form_structure->field_count() - 1]);
// Adjust the expected upload proto. // Adjust the expected upload proto.
upload.mutable_field(5)->set_autofill_type(60); upload.mutable_field(3)->set_autofill_type(1, 60);
ASSERT_TRUE(upload.SerializeToString(&expected_upload_string)); ASSERT_TRUE(upload.SerializeToString(&expected_upload_string));
AutofillUploadContents encoded_upload4; AutofillUploadContents encoded_upload4;
......
...@@ -76,7 +76,7 @@ message AutofillUploadContents { ...@@ -76,7 +76,7 @@ message AutofillUploadContents {
// enum of types located at // enum of types located at
// components/autofill/core/browser/field_types.h // components/autofill/core/browser/field_types.h
// AutoFillFieldType // AutoFillFieldType
required fixed32 autofill_type = 7; repeated fixed32 autofill_type = 7;
// The value of the name attribute on the field, if present. Otherwise, the // The value of the name attribute on the field, if present. Otherwise, the
// value of the id attribute. See HTMLFormControlElement::nameForAutofill. // value of the id attribute. See HTMLFormControlElement::nameForAutofill.
......
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