Commit 7f2b8219 authored by Matthias Körber's avatar Matthias Körber Committed by Commit Bot

[PasswordManager] Put clear-text field password generation behind Finch flag

To allow for a partial roll-out and better metrics, the feature to offer password generation for clear-text fields is put behind a Finch flag.

Bug: 1021897
Change-Id: I4e799b4db354d1285063bb25bd0dc902bfe35b70
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1899380
Commit-Queue: Matthias Körber <koerber@google.com>
Reviewed-by: default avatarVadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713003}
parent d921069a
...@@ -35,6 +35,7 @@ source_set("unit_tests") { ...@@ -35,6 +35,7 @@ source_set("unit_tests") {
"//components/autofill/core/browser", "//components/autofill/core/browser",
"//components/autofill/core/browser/proto", "//components/autofill/core/browser/proto",
"//components/autofill/core/common", "//components/autofill/core/common",
"//components/password_manager/core/common",
"//testing/gmock", "//testing/gmock",
"//testing/gtest", "//testing/gtest",
"//url", "//url",
......
...@@ -146,6 +146,13 @@ bool IsNotUsernameField(const ProcessedField& field) { ...@@ -146,6 +146,13 @@ bool IsNotUsernameField(const ProcessedField& field) {
return field.server_hints_not_username; return field.server_hints_not_username;
} }
// Checks if the Finch experiment for offering password generation for
// server-predicted clear-text fields is enabled.
bool IsPasswordGenerationForClearTextFieldsEnabled() {
return base::FeatureList::IsEnabled(
password_manager::features::KEnablePasswordGenerationForClearTextFields);
}
// Returns true iff |field_type| is one of password types. // Returns true iff |field_type| is one of password types.
bool IsPasswordPrediction(const CredentialFieldType field_type) { bool IsPasswordPrediction(const CredentialFieldType field_type) {
switch (field_type) { switch (field_type) {
...@@ -343,6 +350,10 @@ void ParseUsingPredictions(std::vector<ProcessedField>* processed_fields, ...@@ -343,6 +350,10 @@ void ParseUsingPredictions(std::vector<ProcessedField>* processed_fields,
if (!result->new_password) { if (!result->new_password) {
processed_field = FindField(processed_fields, prediction); processed_field = FindField(processed_fields, prediction);
if (processed_field) { if (processed_field) {
if (!IsPasswordGenerationForClearTextFieldsEnabled() &&
!processed_field->is_password) {
continue;
}
result->new_password = processed_field->field; result->new_password = processed_field->field;
processed_field->is_predicted_as_password = true; processed_field->is_predicted_as_password = true;
} }
...@@ -351,6 +362,10 @@ void ParseUsingPredictions(std::vector<ProcessedField>* processed_fields, ...@@ -351,6 +362,10 @@ void ParseUsingPredictions(std::vector<ProcessedField>* processed_fields,
case CredentialFieldType::kConfirmationPassword: case CredentialFieldType::kConfirmationPassword:
processed_field = FindField(processed_fields, prediction); processed_field = FindField(processed_fields, prediction);
if (processed_field) { if (processed_field) {
if (!IsPasswordGenerationForClearTextFieldsEnabled() &&
!processed_field->is_password) {
continue;
}
result->confirmation_password = processed_field->field; result->confirmation_password = processed_field->field;
processed_field->is_predicted_as_password = true; processed_field->is_predicted_as_password = true;
} }
......
...@@ -15,11 +15,13 @@ ...@@ -15,11 +15,13 @@
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/test/metrics/histogram_tester.h" #include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "components/autofill/core/browser/field_types.h" #include "components/autofill/core/browser/field_types.h"
#include "components/autofill/core/common/form_data.h" #include "components/autofill/core/common/form_data.h"
#include "components/autofill/core/common/form_field_data.h" #include "components/autofill/core/common/form_field_data.h"
#include "components/autofill/core/common/password_form.h" #include "components/autofill/core/common/password_form.h"
#include "components/password_manager/core/common/password_manager_features.h"
#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"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -1114,6 +1116,9 @@ TEST(FormParserTest, ReadonlyFields) { ...@@ -1114,6 +1116,9 @@ TEST(FormParserTest, ReadonlyFields) {
} }
TEST(FormParserTest, ServerPredictionsForClearTextPasswordFields) { TEST(FormParserTest, ServerPredictionsForClearTextPasswordFields) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(
password_manager::features::KEnablePasswordGenerationForClearTextFields);
CheckTestData({ CheckTestData({
{ {
.description_for_logging = "Server prediction for account change " .description_for_logging = "Server prediction for account change "
......
...@@ -133,8 +133,11 @@ bool HasSingleUsernameVote(const FormStructure& form) { ...@@ -133,8 +133,11 @@ bool HasSingleUsernameVote(const FormStructure& form) {
// Returns true if at least one of the fields in |form| has a prediction to be a // Returns true if at least one of the fields in |form| has a prediction to be a
// new-password related field. // new-password related field.
// corresponds to a field for creating or changing a password.
bool HasNewPasswordVote(const FormStructure& form) { bool HasNewPasswordVote(const FormStructure& form) {
if (!base::FeatureList::IsEnabled(
password_manager::features::
KEnablePasswordGenerationForClearTextFields))
return false;
for (const auto& field : form) { for (const auto& field : form) {
if (field->server_type() == autofill::ACCOUNT_CREATION_PASSWORD || if (field->server_type() == autofill::ACCOUNT_CREATION_PASSWORD ||
field->server_type() == autofill::NEW_PASSWORD) { field->server_type() == autofill::NEW_PASSWORD) {
...@@ -195,10 +198,7 @@ void PasswordManager::RegisterLocalPrefs(PrefRegistrySimple* registry) { ...@@ -195,10 +198,7 @@ void PasswordManager::RegisterLocalPrefs(PrefRegistrySimple* registry) {
} }
PasswordManager::PasswordManager(PasswordManagerClient* client) PasswordManager::PasswordManager(PasswordManagerClient* client)
: client_(client), : client_(client), leak_delegate_(client) {
leak_delegate_(client)
{
DCHECK(client_); DCHECK(client_);
} }
......
...@@ -3227,6 +3227,10 @@ TEST_F(PasswordManagerTest, FillSingleUsername) { ...@@ -3227,6 +3227,10 @@ TEST_F(PasswordManagerTest, FillSingleUsername) {
// in marking the password field as eligible for password generation. // in marking the password field as eligible for password generation.
TEST_F(PasswordManagerTest, TEST_F(PasswordManagerTest,
MarkServerPredictedClearTextPasswordFieldEligibleForGeneration) { MarkServerPredictedClearTextPasswordFieldEligibleForGeneration) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(
password_manager::features::KEnablePasswordGenerationForClearTextFields);
PasswordFormManager::set_wait_for_server_predictions_for_filling(true); PasswordFormManager::set_wait_for_server_predictions_for_filling(true);
EXPECT_CALL(client_, IsSavingAndFillingEnabled(_)) EXPECT_CALL(client_, IsSavingAndFillingEnabled(_))
.WillRepeatedly(Return(true)); .WillRepeatedly(Return(true));
......
...@@ -25,6 +25,10 @@ const base::Feature kDeleteCorruptedPasswords = { ...@@ -25,6 +25,10 @@ const base::Feature kDeleteCorruptedPasswords = {
const base::Feature kEnablePasswordsAccountStorage = { const base::Feature kEnablePasswordsAccountStorage = {
"EnablePasswordsAccountStorage", base::FEATURE_DISABLED_BY_DEFAULT}; "EnablePasswordsAccountStorage", base::FEATURE_DISABLED_BY_DEFAULT};
const base::Feature KEnablePasswordGenerationForClearTextFields = {
"EnablePasswordGenerationForClearTextFields",
base::FEATURE_DISABLED_BY_DEFAULT};
// Enables the experiment for the password manager to only fill on account // Enables the experiment for the password manager to only fill on account
// selection, rather than autofilling on page load, with highlighting of fields. // selection, rather than autofilling on page load, with highlighting of fields.
const base::Feature kFillOnAccountSelect = {"fill-on-account-select", const base::Feature kFillOnAccountSelect = {"fill-on-account-select",
......
...@@ -20,6 +20,7 @@ namespace features { ...@@ -20,6 +20,7 @@ namespace features {
extern const base::Feature kEditPasswordsInDesktopSettings; extern const base::Feature kEditPasswordsInDesktopSettings;
extern const base::Feature kDeleteCorruptedPasswords; extern const base::Feature kDeleteCorruptedPasswords;
extern const base::Feature kEnablePasswordsAccountStorage; extern const base::Feature kEnablePasswordsAccountStorage;
extern const base::Feature KEnablePasswordGenerationForClearTextFields;
extern const base::Feature kFillOnAccountSelect; extern const base::Feature kFillOnAccountSelect;
extern const base::Feature kGenerationNoOverwrites; extern const base::Feature kGenerationNoOverwrites;
extern const base::Feature kGooglePasswordManager; extern const base::Feature kGooglePasswordManager;
......
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