Commit cbd23d3e authored by Jan Wilken Dörrie's avatar Jan Wilken Dörrie Committed by Commit Bot

[Passwords] Add Feature to Control reparsing of Server Predictions

This change adds a feature flag to control reparsing server predictions
following dynamic form changes.

Bug: 1140480
Change-Id: Ic0520f197d766fedab96c5c30d089bba49ef3e23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2534857Reviewed-by: default avatarMaria Kazinova <kazinova@google.com>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826884}
parent ab5c4c3c
...@@ -100,6 +100,13 @@ bool FormContainsFieldWithName(const FormData& form, ...@@ -100,6 +100,13 @@ bool FormContainsFieldWithName(const FormData& form,
return false; return false;
} }
// Returns whether reparsing server predictions following a form change is
// enabled.
bool IsReparsingServerPredictionsEnabled() {
return base::FeatureList::IsEnabled(
features::kReparseServerPredictionsFollowingFormChange);
}
bool IsUsernameFirstFlowFeatureEnabled() { bool IsUsernameFirstFlowFeatureEnabled() {
return base::FeatureList::IsEnabled(features::kUsernameFirstFlow); return base::FeatureList::IsEnabled(features::kUsernameFirstFlow);
} }
...@@ -831,7 +838,8 @@ void PasswordFormManager::FillForm( ...@@ -831,7 +838,8 @@ void PasswordFormManager::FillForm(
bool new_predictions_available = false; bool new_predictions_available = false;
if (differences_bitmask) { if (differences_bitmask) {
UpdateFormManagerWithFormChanges(observed_form_data, predictions); UpdateFormManagerWithFormChanges(observed_form_data, predictions);
new_predictions_available = parser_.predictions().has_value(); new_predictions_available =
parser_.predictions() && IsReparsingServerPredictionsEnabled();
} }
// Fill the form if relevant form predictions were found or if the // Fill the form if relevant form predictions were found or if the
// manager is not waiting for new server predictions. // manager is not waiting for new server predictions.
...@@ -1076,6 +1084,9 @@ void PasswordFormManager::UpdateFormManagerWithFormChanges( ...@@ -1076,6 +1084,9 @@ void PasswordFormManager::UpdateFormManagerWithFormChanges(
const FormData& observed_form_data, const FormData& observed_form_data,
const std::map<FormSignature, FormPredictions>& predictions) { const std::map<FormSignature, FormPredictions>& predictions) {
*mutable_observed_form() = observed_form_data; *mutable_observed_form() = observed_form_data;
if (!IsReparsingServerPredictionsEnabled())
return;
// If the observed form has changed, it might be autofilled again. // If the observed form has changed, it might be autofilled again.
autofills_left_ = kMaxTimesAutofill; autofills_left_ = kMaxTimesAutofill;
parser_.reset_predictions(); parser_.reset_predictions();
......
...@@ -3667,6 +3667,12 @@ TEST_P(PasswordManagerTest, ...@@ -3667,6 +3667,12 @@ TEST_P(PasswordManagerTest,
} }
TEST_P(PasswordManagerTest, GenerationOnChangedForm) { TEST_P(PasswordManagerTest, GenerationOnChangedForm) {
const bool kIsReparsingEnabled = GetParam();
base::test::ScopedFeatureList feature_list;
feature_list.InitWithFeatureState(
features::kReparseServerPredictionsFollowingFormChange,
kIsReparsingEnabled);
EXPECT_CALL(client_, IsSavingAndFillingEnabled(_)) EXPECT_CALL(client_, IsSavingAndFillingEnabled(_))
.WillRepeatedly(Return(true)); .WillRepeatedly(Return(true));
EXPECT_CALL(*store_, GetLogins(_, _)) EXPECT_CALL(*store_, GetLogins(_, _))
...@@ -3706,12 +3712,16 @@ TEST_P(PasswordManagerTest, GenerationOnChangedForm) { ...@@ -3706,12 +3712,16 @@ TEST_P(PasswordManagerTest, GenerationOnChangedForm) {
manager()->ProcessAutofillPredictions(&driver_, {&form_structure}); manager()->ProcessAutofillPredictions(&driver_, {&form_structure});
autofill::PasswordFormGenerationData form_generation_data; autofill::PasswordFormGenerationData form_generation_data;
EXPECT_CALL(driver_, FormEligibleForGenerationFound(_)) if (kIsReparsingEnabled) {
.WillOnce(SaveArg<0>(&form_generation_data)); EXPECT_CALL(driver_, FormEligibleForGenerationFound)
// The change is discovered by PasswordManager. .WillOnce(SaveArg<0>(&form_generation_data));
manager()->OnPasswordFormsParsed(&driver_, {form_data}); // The change is discovered by PasswordManager.
EXPECT_EQ(new_password_field.unique_renderer_id, manager()->OnPasswordFormsParsed(&driver_, {form_data});
form_generation_data.new_password_renderer_id); EXPECT_EQ(new_password_field.unique_renderer_id,
form_generation_data.new_password_renderer_id);
} else {
EXPECT_CALL(driver_, FormEligibleForGenerationFound).Times(0);
}
} }
INSTANTIATE_TEST_SUITE_P(, PasswordManagerTest, testing::Bool()); INSTANTIATE_TEST_SUITE_P(, PasswordManagerTest, testing::Bool());
......
...@@ -84,6 +84,12 @@ const base::Feature kPasswordsWeaknessCheck = { ...@@ -84,6 +84,12 @@ const base::Feature kPasswordsWeaknessCheck = {
const base::Feature kRecoverFromNeverSaveAndroid = { const base::Feature kRecoverFromNeverSaveAndroid = {
"RecoverFromNeverSaveAndroid", base::FEATURE_DISABLED_BY_DEFAULT}; "RecoverFromNeverSaveAndroid", base::FEATURE_DISABLED_BY_DEFAULT};
// Enables reparsing server predictions once the password form manager notices a
// dynamic form change.
const base::Feature kReparseServerPredictionsFollowingFormChange = {
"ReparseServerPredictionsFollowingFormChange",
base::FEATURE_DISABLED_BY_DEFAULT};
// Enables considering secondary server field predictions during form parsing. // Enables considering secondary server field predictions during form parsing.
const base::Feature kSecondaryServerFieldPredictions = { const base::Feature kSecondaryServerFieldPredictions = {
"SecondaryServerFieldPredictions", base::FEATURE_DISABLED_BY_DEFAULT}; "SecondaryServerFieldPredictions", base::FEATURE_DISABLED_BY_DEFAULT};
......
...@@ -33,6 +33,7 @@ extern const base::Feature kPasswordImport; ...@@ -33,6 +33,7 @@ extern const base::Feature kPasswordImport;
extern const base::Feature kPasswordScriptsFetching; extern const base::Feature kPasswordScriptsFetching;
extern const base::Feature kPasswordsWeaknessCheck; extern const base::Feature kPasswordsWeaknessCheck;
extern const base::Feature kRecoverFromNeverSaveAndroid; extern const base::Feature kRecoverFromNeverSaveAndroid;
extern const base::Feature kReparseServerPredictionsFollowingFormChange;
extern const base::Feature kSecondaryServerFieldPredictions; extern const base::Feature kSecondaryServerFieldPredictions;
extern const base::Feature kTreatNewPasswordHeuristicsAsReliable; extern const base::Feature kTreatNewPasswordHeuristicsAsReliable;
extern const base::Feature kUseOfHashAffiliationFetcher; extern const base::Feature kUseOfHashAffiliationFetcher;
......
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