Commit 154ae328 authored by Vadym Doroshenko's avatar Vadym Doroshenko Committed by Commit Bot

Implement password form parsing with SINGLE_USERNAME prediction.

This CL is a part of username first flow implementation. The server
sends prediction SINGLE_USERNAME for a field which is a single username
in a form (i.e. the first part of sign-in process).

This CL implements that in case of receiving SINGLE_USERNAME prediction
FormParser chooses this field as a username field, and chooses no other
fields as passwords.

Bug: 959776
Change-Id: I2d62b488c041ace1d525f5627bab8c97aa9ccd20
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1726030Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682331}
parent 10269458
...@@ -147,6 +147,7 @@ bool IsNotUsernameField(const ProcessedField& field) { ...@@ -147,6 +147,7 @@ bool IsNotUsernameField(const ProcessedField& field) {
bool IsPasswordPrediction(const CredentialFieldType field_type) { bool IsPasswordPrediction(const CredentialFieldType field_type) {
switch (field_type) { switch (field_type) {
case CredentialFieldType::kUsername: case CredentialFieldType::kUsername:
case CredentialFieldType::kSingleUsername:
case CredentialFieldType::kNone: case CredentialFieldType::kNone:
return false; return false;
case CredentialFieldType::kCurrentPassword: case CredentialFieldType::kCurrentPassword:
...@@ -213,6 +214,9 @@ struct SignificantFields { ...@@ -213,6 +214,9 @@ struct SignificantFields {
// attributes. // attributes.
bool is_new_password_reliable = false; bool is_new_password_reliable = false;
// True if the current form has only username, but no passwords.
bool is_single_username = false;
// Returns true if some password field is present. This is the minimal // Returns true if some password field is present. This is the minimal
// requirement for a successful creation of a PasswordForm is present. // requirement for a successful creation of a PasswordForm is present.
bool HasPasswords() const { bool HasPasswords() const {
...@@ -220,6 +224,12 @@ struct SignificantFields { ...@@ -220,6 +224,12 @@ struct SignificantFields {
<< "There is no password to confirm if there is no new password field."; << "There is no password to confirm if there is no new password field.";
return password || new_password; return password || new_password;
} }
void ClearAllPasswordFields() {
password = nullptr;
new_password = nullptr;
confirmation_password = nullptr;
}
}; };
// Returns true if |field| is in |significant_fields|. // Returns true if |field| is in |significant_fields|.
...@@ -284,18 +294,25 @@ void ParseUsingPredictions(std::vector<ProcessedField>* processed_fields, ...@@ -284,18 +294,25 @@ void ParseUsingPredictions(std::vector<ProcessedField>* processed_fields,
case CredentialFieldType::kUsername: case CredentialFieldType::kUsername:
if (!result->username) { if (!result->username) {
processed_field = FindField(processed_fields, prediction); processed_field = FindField(processed_fields, prediction);
if (processed_field) { if (processed_field)
result->username = processed_field->field; result->username = processed_field->field;
}
} else if (!second_username) { } else if (!second_username) {
processed_field = FindField(processed_fields, prediction); processed_field = FindField(processed_fields, prediction);
if (processed_field) { if (processed_field)
second_username = processed_field->field; second_username = processed_field->field;
}
} else { } else {
prevent_handling_two_usernames = true; prevent_handling_two_usernames = true;
} }
break; break;
case CredentialFieldType::kSingleUsername:
processed_field = FindField(processed_fields, prediction);
if (processed_field) {
result->username = processed_field->field;
result->is_single_username = true;
result->ClearAllPasswordFields();
return;
}
break;
case CredentialFieldType::kCurrentPassword: case CredentialFieldType::kCurrentPassword:
if (result->password) { if (result->password) {
prevent_handling_two_usernames = true; prevent_handling_two_usernames = true;
...@@ -478,7 +495,8 @@ std::vector<const FormFieldData*> GetRelevantPasswords( ...@@ -478,7 +495,8 @@ std::vector<const FormFieldData*> GetRelevantPasswords(
if (processed_field.is_password) if (processed_field.is_password)
passwords.push_back(&processed_field); passwords.push_back(&processed_field);
} }
DCHECK(!passwords.empty()); if (passwords.empty())
return std::vector<const FormFieldData*>();
// These two counters are used to determine the ReadonlyPasswordFields value // These two counters are used to determine the ReadonlyPasswordFields value
// corresponding to this form. // corresponding to this form.
...@@ -803,8 +821,6 @@ std::vector<ProcessedField> ProcessFields( ...@@ -803,8 +821,6 @@ std::vector<ProcessedField> ProcessFields(
DCHECK(all_possible_passwords->empty()); DCHECK(all_possible_passwords->empty());
std::vector<ProcessedField> result; std::vector<ProcessedField> result;
bool password_field_found = false;
result.reserve(fields.size()); result.reserve(fields.size());
// |all_possible_passwords| should only contain each value once. // |all_possible_passwords| should only contain each value once.
...@@ -840,8 +856,6 @@ std::vector<ProcessedField> ProcessFields( ...@@ -840,8 +856,6 @@ std::vector<ProcessedField> ProcessFields(
ProcessedField processed_field = { ProcessedField processed_field = {
.field = &field, .autocomplete_flag = flag, .is_password = is_password}; .field = &field, .autocomplete_flag = flag, .is_password = is_password};
password_field_found |= is_password;
if (field.properties_mask & FieldPropertiesFlags::USER_TYPED) if (field.properties_mask & FieldPropertiesFlags::USER_TYPED)
processed_field.interactability = Interactability::kCertain; processed_field.interactability = Interactability::kCertain;
else if (field.is_focusable) else if (field.is_focusable)
...@@ -850,9 +864,6 @@ std::vector<ProcessedField> ProcessFields( ...@@ -850,9 +864,6 @@ std::vector<ProcessedField> ProcessFields(
result.push_back(processed_field); result.push_back(processed_field);
} }
if (!password_field_found)
result.clear();
return result; return result;
} }
...@@ -908,8 +919,10 @@ std::unique_ptr<PasswordForm> AssemblePasswordForm( ...@@ -908,8 +919,10 @@ std::unique_ptr<PasswordForm> AssemblePasswordForm(
autofill::ValueElementVector all_possible_passwords, autofill::ValueElementVector all_possible_passwords,
autofill::ValueElementVector all_possible_usernames, autofill::ValueElementVector all_possible_usernames,
const base::Optional<FormPredictions>& form_predictions) { const base::Optional<FormPredictions>& form_predictions) {
if (!significant_fields.HasPasswords()) if (!significant_fields.HasPasswords() &&
!significant_fields.is_single_username) {
return nullptr; return nullptr;
}
// Create the PasswordForm and set data not related to specific fields. // Create the PasswordForm and set data not related to specific fields.
auto result = std::make_unique<PasswordForm>(); auto result = std::make_unique<PasswordForm>();
...@@ -970,11 +983,14 @@ std::unique_ptr<PasswordForm> FormDataParser::Parse(const FormData& form_data, ...@@ -970,11 +983,14 @@ std::unique_ptr<PasswordForm> FormDataParser::Parse(const FormData& form_data,
} }
// (2) If that failed, try to parse with autocomplete attributes. // (2) If that failed, try to parse with autocomplete attributes.
if (!significant_fields.is_single_username) {
ParseUsingAutocomplete(processed_fields, &significant_fields); ParseUsingAutocomplete(processed_fields, &significant_fields);
if (username_detection_method == if (username_detection_method ==
UsernameDetectionMethod::kNoUsernameDetected && UsernameDetectionMethod::kNoUsernameDetected &&
significant_fields.username) { significant_fields.username) {
username_detection_method = UsernameDetectionMethod::kAutocompleteAttribute; username_detection_method =
UsernameDetectionMethod::kAutocompleteAttribute;
}
} }
// Pass the "reliability" information to mark the new-password fields as // Pass the "reliability" information to mark the new-password fields as
...@@ -989,6 +1005,7 @@ std::unique_ptr<PasswordForm> FormDataParser::Parse(const FormData& form_data, ...@@ -989,6 +1005,7 @@ std::unique_ptr<PasswordForm> FormDataParser::Parse(const FormData& form_data,
const bool username_found_before_heuristic = significant_fields.username; const bool username_found_before_heuristic = significant_fields.username;
// Try to parse with base heuristic. // Try to parse with base heuristic.
if (!significant_fields.is_single_username) {
Interactability username_max = Interactability::kUnlikely; Interactability username_max = Interactability::kUnlikely;
ParseUsingBaseHeuristics(processed_fields, mode, &significant_fields, ParseUsingBaseHeuristics(processed_fields, mode, &significant_fields,
&username_max, &readonly_status_); &username_max, &readonly_status_);
...@@ -1003,8 +1020,9 @@ std::unique_ptr<PasswordForm> FormDataParser::Parse(const FormData& form_data, ...@@ -1003,8 +1020,9 @@ std::unique_ptr<PasswordForm> FormDataParser::Parse(const FormData& form_data,
// fields, unless the username already came from more reliable types of // fields, unless the username already came from more reliable types of
// analysis. // analysis.
if (!username_found_before_heuristic) { if (!username_found_before_heuristic) {
const FormFieldData* username_field_by_context = FindUsernameInPredictions( const FormFieldData* username_field_by_context =
form_data.username_predictions, processed_fields, username_max); FindUsernameInPredictions(form_data.username_predictions,
processed_fields, username_max);
if (username_field_by_context && if (username_field_by_context &&
!(mode == FormDataParser::Mode::kSaving && !(mode == FormDataParser::Mode::kSaving &&
username_field_by_context->value.empty())) { username_field_by_context->value.empty())) {
...@@ -1018,6 +1036,7 @@ std::unique_ptr<PasswordForm> FormDataParser::Parse(const FormData& form_data, ...@@ -1018,6 +1036,7 @@ std::unique_ptr<PasswordForm> FormDataParser::Parse(const FormData& form_data,
} }
} }
} }
}
UMA_HISTOGRAM_ENUMERATION("PasswordManager.UsernameDetectionMethod", UMA_HISTOGRAM_ENUMERATION("PasswordManager.UsernameDetectionMethod",
username_detection_method, username_detection_method,
......
...@@ -2096,6 +2096,21 @@ TEST(FormParserTest, ContradictingPasswordPredictionAndAutocomplete) { ...@@ -2096,6 +2096,21 @@ TEST(FormParserTest, ContradictingPasswordPredictionAndAutocomplete) {
.autocomplete_attribute = "new-password"}}}}); .autocomplete_attribute = "new-password"}}}});
} }
TEST(FormParserTest, SingleUsernamePrediction) {
CheckTestData({
{"1 field",
{{.role = ElementRole::USERNAME,
.form_control_type = "text",
.prediction = {.type = autofill::SINGLE_USERNAME}}}},
{"Password field is ignored",
{{.role = ElementRole::USERNAME,
.form_control_type = "text",
.prediction = {.type = autofill::SINGLE_USERNAME}},
{.form_control_type = "password",
.prediction = {.type = autofill::PASSWORD}}}},
});
}
} // namespace } // namespace
} // namespace password_manager } // namespace password_manager
...@@ -30,6 +30,8 @@ CredentialFieldType DeriveFromServerFieldType(ServerFieldType type) { ...@@ -30,6 +30,8 @@ CredentialFieldType DeriveFromServerFieldType(ServerFieldType type) {
case autofill::USERNAME: case autofill::USERNAME:
case autofill::USERNAME_AND_EMAIL_ADDRESS: case autofill::USERNAME_AND_EMAIL_ADDRESS:
return CredentialFieldType::kUsername; return CredentialFieldType::kUsername;
case autofill::SINGLE_USERNAME:
return CredentialFieldType::kSingleUsername;
case autofill::PASSWORD: case autofill::PASSWORD:
return CredentialFieldType::kCurrentPassword; return CredentialFieldType::kCurrentPassword;
case autofill::ACCOUNT_CREATION_PASSWORD: case autofill::ACCOUNT_CREATION_PASSWORD:
......
...@@ -20,6 +20,7 @@ namespace password_manager { ...@@ -20,6 +20,7 @@ namespace password_manager {
enum class CredentialFieldType { enum class CredentialFieldType {
kNone, kNone,
kUsername, kUsername,
kSingleUsername,
kCurrentPassword, kCurrentPassword,
kNewPassword, kNewPassword,
kConfirmationPassword kConfirmationPassword
......
...@@ -13,8 +13,8 @@ ...@@ -13,8 +13,8 @@
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
using autofill::AutofillField;
using autofill::ACCOUNT_CREATION_PASSWORD; using autofill::ACCOUNT_CREATION_PASSWORD;
using autofill::AutofillField;
using autofill::CONFIRMATION_PASSWORD; using autofill::CONFIRMATION_PASSWORD;
using autofill::EMAIL_ADDRESS; using autofill::EMAIL_ADDRESS;
using autofill::FormData; using autofill::FormData;
...@@ -24,6 +24,7 @@ using autofill::NEW_PASSWORD; ...@@ -24,6 +24,7 @@ using autofill::NEW_PASSWORD;
using autofill::NO_SERVER_DATA; using autofill::NO_SERVER_DATA;
using autofill::PASSWORD; using autofill::PASSWORD;
using autofill::ServerFieldType; using autofill::ServerFieldType;
using autofill::SINGLE_USERNAME;
using autofill::UNKNOWN_TYPE; using autofill::UNKNOWN_TYPE;
using autofill::USERNAME; using autofill::USERNAME;
using autofill::USERNAME_AND_EMAIL_ADDRESS; using autofill::USERNAME_AND_EMAIL_ADDRESS;
...@@ -186,6 +187,8 @@ TEST(FormPredictionsTest, DeriveFromServerFieldType) { ...@@ -186,6 +187,8 @@ TEST(FormPredictionsTest, DeriveFromServerFieldType) {
{"Username", USERNAME, CredentialFieldType::kUsername}, {"Username", USERNAME, CredentialFieldType::kUsername},
{"Username/Email", USERNAME_AND_EMAIL_ADDRESS, {"Username/Email", USERNAME_AND_EMAIL_ADDRESS,
CredentialFieldType::kUsername}, CredentialFieldType::kUsername},
{"Single Username", SINGLE_USERNAME,
CredentialFieldType::kSingleUsername},
{"Password", PASSWORD, CredentialFieldType::kCurrentPassword}, {"Password", PASSWORD, CredentialFieldType::kCurrentPassword},
{"New password", NEW_PASSWORD, CredentialFieldType::kNewPassword}, {"New password", NEW_PASSWORD, CredentialFieldType::kNewPassword},
{"Account creation password", ACCOUNT_CREATION_PASSWORD, {"Account creation password", ACCOUNT_CREATION_PASSWORD,
......
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