Commit 2df34855 authored by Luka Dojcilovic's avatar Luka Dojcilovic Committed by Commit Bot

[Password Generation] Crowdsource |SubmissionIndicatorEvent| to improve...

[Password Generation] Crowdsource |SubmissionIndicatorEvent| to improve successful form submission detection

Bug: 552420
Change-Id: I68fc0b0d4b25003ebbc4de7d25df757958add4bb
Reviewed-on: https://chromium-review.googlesource.com/1196386Reviewed-by: default avatarMaxim Kolosovskiy <kolos@chromium.org>
Reviewed-by: default avatarVadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Luka Dojcilovic <lukad@google.com>
Cr-Commit-Position: refs/heads/master@{#588831}
parent d35d73c4
......@@ -329,6 +329,7 @@ void EncodePasswordAttributesVote(
FormStructure::FormStructure(const FormData& form)
: form_name_(form.name),
submission_event_(PasswordForm::SubmissionIndicatorEvent::NONE),
source_url_(form.origin),
target_url_(form.action),
main_frame_origin_(form.main_frame_origin),
......@@ -439,6 +440,13 @@ bool FormStructure::EncodeUploadRequest(
upload->set_autofill_used(form_was_autofilled);
upload->set_data_present(EncodeFieldTypes(available_field_types));
upload->set_passwords_revealed(passwords_were_revealed_);
if (submission_event_ != PasswordForm::SubmissionIndicatorEvent::NONE) {
DCHECK(submission_event_ != PasswordForm::SubmissionIndicatorEvent::
SUBMISSION_INDICATOR_EVENT_COUNT);
upload->set_submission_event(
static_cast<AutofillUploadContents_SubmissionIndicatorEvent>(
submission_event_));
}
if (password_attributes_vote_) {
EncodePasswordAttributesVote(*password_attributes_vote_,
password_length_vote_, upload);
......
......@@ -23,6 +23,7 @@
#include "components/autofill/core/browser/field_types.h"
#include "components/autofill/core/browser/form_types.h"
#include "components/autofill/core/browser/proto/server.pb.h"
#include "components/autofill/core/common/password_form.h"
#include "url/gurl.h"
#include "url/origin.h"
......@@ -231,6 +232,11 @@ class FormStructure {
return has_author_specified_upi_vpa_hint_;
}
void set_submission_event(
PasswordForm::SubmissionIndicatorEvent submission_event) {
submission_event_ = submission_event;
}
void set_upload_required(UploadRequired required) {
upload_required_ = required;
}
......@@ -284,6 +290,11 @@ class FormStructure {
"|password_attributes_vote_| has no value.";
return password_length_vote_;
}
PasswordForm::SubmissionIndicatorEvent get_submission_event_for_testing()
const {
return submission_event_;
}
#endif
bool operator==(const FormData& form) const;
......@@ -444,6 +455,10 @@ class FormStructure {
// The name of the form.
base::string16 form_name_;
// The type of the event that was taken as an indication that the form has
// been successfully submitted.
PasswordForm::SubmissionIndicatorEvent submission_event_;
// The source URL.
GURL source_url_;
......
......@@ -20,6 +20,7 @@
#include "components/autofill/core/common/autofill_features.h"
#include "components/autofill/core/common/form_data.h"
#include "components/autofill/core/common/form_field_data.h"
#include "components/autofill/core/common/password_form.h"
#include "components/autofill/core/common/signatures_util.h"
#include "components/variations/entropy_provider.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -2831,6 +2832,8 @@ TEST_F(FormStructureTest, EncodeUploadRequest) {
form_structure->set_password_attributes_vote(
std::make_pair(PasswordAttribute::kHasNumeric, true));
form_structure->set_password_length_vote(10u);
form_structure->set_submission_event(
PasswordForm::SubmissionIndicatorEvent::HTML_FORM_SUBMISSION);
ASSERT_EQ(form_structure->field_count(), possible_field_types.size());
ASSERT_EQ(form_structure->field_count(),
......@@ -2855,6 +2858,7 @@ TEST_F(FormStructureTest, EncodeUploadRequest) {
// Prepare the expected proto string.
AutofillUploadContents upload;
upload.set_submission(true);
upload.set_submission_event(AutofillUploadContents::HTML_FORM_SUBMISSION);
upload.set_client_version("6.1.1715.1442/en (GGLL)");
upload.set_form_signature(8736493185895608956U);
upload.set_autofill_used(false);
......@@ -2913,7 +2917,8 @@ TEST_F(FormStructureTest, EncodeUploadRequest) {
form_structure->set_password_attributes_vote(
std::make_pair(PasswordAttribute::kHasNumeric, true));
form_structure->set_password_length_vote(10u);
form_structure->set_submission_event(
PasswordForm::SubmissionIndicatorEvent::HTML_FORM_SUBMISSION);
ASSERT_EQ(form_structure->field_count(), possible_field_types.size());
ASSERT_EQ(form_structure->field_count(),
possible_field_types_validities.size());
......
......@@ -52,7 +52,7 @@ message AutofillQueryResponseContents {
// This message contains information about the field types in a single form.
// It is sent by the toolbar to contribute to the field type statistics.
// Next available id: 30
// Next available id: 31
message AutofillUploadContents {
required string client_version = 1;
required fixed64 form_signature = 2;
......@@ -187,6 +187,26 @@ message AutofillUploadContents {
// Noisifed password length.
optional uint32 password_length = 29;
// The end of the section of password attributes.
// Event observed by the password manager which indicated that the form was
// successfully submitted. Corresponds to
// |PasswordForm::SubmissionIndicatorEvent|.
enum SubmissionIndicatorEvent {
NONE = 0;
HTML_FORM_SUBMISSION = 1;
SAME_DOCUMENT_NAVIGATION = 2;
XHR_SUCCEEDED = 3;
FRAME_DETACHED = 4;
DEPRECATED_MANUAL_SAVE = 5; // obsolete
DOM_MUTATION_AFTER_XHR = 6;
PROVISIONALLY_SAVED_FORM_ON_START_PROVISIONAL_LOAD = 7;
DEPRECATED_FILLED_FORM_ON_START_PROVISIONAL_LOAD = 8; // unused
DEPRECATED_FILLED_INPUT_ELEMENTS_ON_START_PROVISIONAL_LOAD = 9; // unused
}
// The type of the event that was taken as an indication that the form has
// been successfully submitted.
optional SubmissionIndicatorEvent submission_event = 30;
}
// This proto contains information about the validity of each field in an
......
......@@ -25,6 +25,19 @@ MATCHER_P(SignatureIsSameAs,
return false;
}
MATCHER_P(SubmissionEventIsSameAs,
expected_submission_event,
std::string(negation ? "submission event isn't "
: "submission event is ") +
std::to_string(static_cast<int>(expected_submission_event))) {
if (expected_submission_event == arg.get_submission_event_for_testing())
return true;
*result_listener << "submission event is "
<< arg.get_submission_event_for_testing() << " instead";
return false;
}
MATCHER_P(UploadedAutofillTypesAre, expected_types, "") {
size_t fields_matched_type_count = 0;
bool conflict_found = false;
......
......@@ -137,7 +137,7 @@ void VotesUploader::SendVotesOnSave(
// values. Fill username field value with username to allow AutofillManager
// to detect username autofill type.
form_data.fields[0].value = pending_credentials->username_value;
SendSignInVote(form_data);
SendSignInVote(form_data, submitted_form.submission_event);
}
if (pending_credentials->times_used == 1 ||
......@@ -227,6 +227,7 @@ bool VotesUploader::UploadPasswordVote(
// re-uses credentials, a vote about the saved form is sent. If the user saves
// credentials, the observed and pending forms are the same.
FormStructure form_structure(form_to_upload.form_data);
form_structure.set_submission_event(submitted_form.submission_event);
if (!autofill_manager->ShouldUploadForm(form_structure)) {
UMA_HISTOGRAM_BOOLEAN("PasswordGeneration.UploadStarted", false);
return false;
......@@ -318,6 +319,7 @@ void VotesUploader::UploadFirstLoginVotes(
return;
FormStructure form_structure(form_to_upload.form_data);
form_structure.set_submission_event(form_to_upload.submission_event);
if (!autofill_manager->ShouldUploadForm(form_structure))
return;
......@@ -349,11 +351,14 @@ void VotesUploader::UploadFirstLoginVotes(
std::string(), true /* observed_submission */);
}
void VotesUploader::SendSignInVote(const FormData& form_data) {
void VotesUploader::SendSignInVote(
const FormData& form_data,
const PasswordForm::SubmissionIndicatorEvent& submission_event) {
AutofillManager* autofill_manager = client_->GetAutofillManagerForMainFrame();
if (!autofill_manager)
return;
std::unique_ptr<FormStructure> form_structure(new FormStructure(form_data));
form_structure->set_submission_event(submission_event);
form_structure->set_is_signin_upload(true);
DCHECK(form_structure->ShouldBeUploaded());
DCHECK_EQ(2u, form_structure->field_count());
......
......@@ -137,7 +137,9 @@ class VotesUploader {
// Send a vote for sign-in forms with autofill types for a username field.
// TODO(https://crbug.com/831123): Remove this method.
void SendSignInVote(const autofill::FormData& form_data);
void SendSignInVote(
const autofill::FormData& form_data,
const autofill::PasswordForm::SubmissionIndicatorEvent& submission_event);
// Adds a vote on password generation usage to |form_structure|.
void AddGeneratedVote(autofill::FormStructure* form_structure);
......
......@@ -147,16 +147,21 @@ TEST_F(VotesUploaderTest, UploadPasswordVoteUpdate) {
submitted_form_.new_password_element = new_password_element;
form_to_upload_.confirmation_password_element = confirmation_element;
submitted_form_.confirmation_password_element = confirmation_element;
ServerFieldTypeSet expexted_field_types = {NEW_PASSWORD,
submitted_form_.submission_event =
PasswordForm::SubmissionIndicatorEvent::HTML_FORM_SUBMISSION;
ServerFieldTypeSet expected_field_types = {NEW_PASSWORD,
CONFIRMATION_PASSWORD};
FieldTypeMap expected_types = {{new_password_element, NEW_PASSWORD},
{confirmation_element, CONFIRMATION_PASSWORD}};
PasswordForm::SubmissionIndicatorEvent expected_submission_event =
PasswordForm::SubmissionIndicatorEvent::HTML_FORM_SUBMISSION;
EXPECT_CALL(*mock_autofill_download_manager_,
StartUploadRequest(
AllOf(SignatureIsSameAs(form_to_upload_),
UploadedAutofillTypesAre(expected_types)),
false, expexted_field_types, login_form_signature_, true));
UploadedAutofillTypesAre(expected_types),
SubmissionEventIsSameAs(expected_submission_event)),
false, expected_field_types, login_form_signature_, true));
EXPECT_TRUE(votes_uploader.UploadPasswordVote(
form_to_upload_, submitted_form_, NEW_PASSWORD, login_form_signature_));
......@@ -170,11 +175,16 @@ TEST_F(VotesUploaderTest, UploadPasswordVoteSave) {
submitted_form_.password_element = password_element;
form_to_upload_.confirmation_password_element = confirmation_element;
submitted_form_.confirmation_password_element = confirmation_element;
ServerFieldTypeSet expexted_field_types = {PASSWORD, CONFIRMATION_PASSWORD};
submitted_form_.submission_event =
PasswordForm::SubmissionIndicatorEvent::HTML_FORM_SUBMISSION;
ServerFieldTypeSet expected_field_types = {PASSWORD, CONFIRMATION_PASSWORD};
PasswordForm::SubmissionIndicatorEvent expected_submission_event =
PasswordForm::SubmissionIndicatorEvent::HTML_FORM_SUBMISSION;
EXPECT_CALL(*mock_autofill_download_manager_,
StartUploadRequest(_, false, expexted_field_types,
login_form_signature_, true));
StartUploadRequest(
SubmissionEventIsSameAs(expected_submission_event), false,
expected_field_types, login_form_signature_, true));
EXPECT_TRUE(votes_uploader.UploadPasswordVote(
form_to_upload_, submitted_form_, PASSWORD, login_form_signature_));
......
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