Commit 611a2aff authored by Vadym Doroshenko's avatar Vadym Doroshenko Committed by Commit Bot

Do not show password save prompt for credit card forms.

Along the way the function CanProvisionalManagerSave is renamed to
IsReadyForAutomaticSaving because it shows better what this function
checks.

Bug: 831123
Change-Id: Iabf125fd8b46a15cccbe5e6152e98352e5facde2
Reviewed-on: https://chromium-review.googlesource.com/c/1391685
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: default avatarMaxim Kolosovskiy <kolos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619948}
parent 873fae30
...@@ -305,7 +305,7 @@ std::string SavePasswordProgressLogger::GetStringFromID( ...@@ -305,7 +305,7 @@ std::string SavePasswordProgressLogger::GetStringFromID(
case SavePasswordProgressLogger::STRING_ON_ASK_USER_OR_SAVE_PASSWORD: case SavePasswordProgressLogger::STRING_ON_ASK_USER_OR_SAVE_PASSWORD:
return "PasswordManager::AskUserOrSavePassword"; return "PasswordManager::AskUserOrSavePassword";
case SavePasswordProgressLogger::STRING_CAN_PROVISIONAL_MANAGER_SAVE_METHOD: case SavePasswordProgressLogger::STRING_CAN_PROVISIONAL_MANAGER_SAVE_METHOD:
return "PasswordManager::CanProvisionalManagerSave"; return "PasswordManager::IsReadyForAutomaticSaving";
case SavePasswordProgressLogger::STRING_NO_PROVISIONAL_SAVE_MANAGER: case SavePasswordProgressLogger::STRING_NO_PROVISIONAL_SAVE_MANAGER:
return "No provisional save manager"; return "No provisional save manager";
case SavePasswordProgressLogger::STRING_NUMBER_OF_VISIBLE_FORMS: case SavePasswordProgressLogger::STRING_NUMBER_OF_VISIBLE_FORMS:
......
...@@ -653,7 +653,7 @@ void PasswordManager::OnPasswordFormSubmittedNoChecks( ...@@ -653,7 +653,7 @@ void PasswordManager::OnPasswordFormSubmittedNoChecks(
ProvisionallySavePassword(password_form, driver); ProvisionallySavePassword(password_form, driver);
if (CanProvisionalManagerSave()) if (IsReadyForAutomaticSaving())
OnLoginSuccessful(); OnLoginSuccessful();
} }
...@@ -957,7 +957,7 @@ void PasswordManager::ProvisionallySaveManager( ...@@ -957,7 +957,7 @@ void PasswordManager::ProvisionallySaveManager(
provisional_save_manager_.swap(manager); provisional_save_manager_.swap(manager);
} }
bool PasswordManager::CanProvisionalManagerSave() { bool PasswordManager::IsReadyForAutomaticSaving() {
std::unique_ptr<BrowserSavePasswordProgressLogger> logger; std::unique_ptr<BrowserSavePasswordProgressLogger> logger;
if (password_manager_util::IsLoggingActive(client_)) { if (password_manager_util::IsLoggingActive(client_)) {
logger.reset( logger.reset(
...@@ -983,7 +983,8 @@ bool PasswordManager::CanProvisionalManagerSave() { ...@@ -983,7 +983,8 @@ bool PasswordManager::CanProvisionalManagerSave() {
submitted_manager->GetOrigin(), logger.get()); submitted_manager->GetOrigin(), logger.get());
return false; return false;
} }
return true;
return !submitted_manager->GetPendingCredentials().only_for_fallback_saving;
} }
bool PasswordManager::ShouldBlockPasswordForSameOriginButDifferentScheme( bool PasswordManager::ShouldBlockPasswordForSameOriginButDifferentScheme(
...@@ -1007,7 +1008,7 @@ void PasswordManager::OnPasswordFormsRendered( ...@@ -1007,7 +1008,7 @@ void PasswordManager::OnPasswordFormsRendered(
logger->LogMessage(Logger::STRING_ON_PASSWORD_FORMS_RENDERED_METHOD); logger->LogMessage(Logger::STRING_ON_PASSWORD_FORMS_RENDERED_METHOD);
} }
if (!CanProvisionalManagerSave()) if (!IsReadyForAutomaticSaving())
return; return;
PasswordFormManagerInterface* submitted_manager = GetSubmittedManager(); PasswordFormManagerInterface* submitted_manager = GetSubmittedManager();
......
...@@ -200,7 +200,7 @@ class PasswordManager : public LoginModel, public FormSubmissionObserver { ...@@ -200,7 +200,7 @@ class PasswordManager : public LoginModel, public FormSubmissionObserver {
// Returns true if |provisional_save_manager_| is ready for saving and // Returns true if |provisional_save_manager_| is ready for saving and
// non-blacklisted. // non-blacklisted.
bool CanProvisionalManagerSave(); bool IsReadyForAutomaticSaving();
// Returns true if there already exists a provisionally saved password form // Returns true if there already exists a provisionally saved password form
// from the same origin as |form|, but with a different and secure scheme. // from the same origin as |form|, but with a different and secure scheme.
......
...@@ -2984,6 +2984,29 @@ TEST_F(PasswordManagerTest, NoSavePromptWhenPasswordManagerDisabled) { ...@@ -2984,6 +2984,29 @@ TEST_F(PasswordManagerTest, NoSavePromptWhenPasswordManagerDisabled) {
manager()->OnPasswordFormSubmittedNoChecks(&driver_, submitted_form); manager()->OnPasswordFormSubmittedNoChecks(&driver_, submitted_form);
} }
TEST_F(PasswordManagerTest, NoSavePromptForNotPasswordForm) {
base::test::ScopedFeatureList scoped_feature_list;
TurnOnNewParsingForSaving(&scoped_feature_list);
EXPECT_CALL(client_, IsSavingAndFillingEnabledForCurrentPage())
.WillRepeatedly(Return(true));
EXPECT_CALL(*store_, GetLogins(_, _))
.WillRepeatedly(WithArg<1>(InvokeEmptyConsumerWithForms()));
PasswordForm form(MakeSimpleForm());
// Make the form to be credit card form.
form.form_data.fields[1].autocomplete_attribute = "cc-csc";
manager()->OnPasswordFormsParsed(&driver_, {form});
auto submitted_form = form;
submitted_form.form_data.fields[0].value = ASCIIToUTF16("text");
submitted_form.form_data.fields[1].value = ASCIIToUTF16("1234");
EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0);
manager()->OnPasswordFormSubmittedNoChecks(&driver_, submitted_form);
}
// Check that when autofill predictions are received before a form is found then // Check that when autofill predictions are received before a form is found then
// server predictions are not ignored and used for filling. // server predictions are not ignored and used for filling.
TEST_F(PasswordManagerTest, AutofillPredictionBeforeFormParsed) { TEST_F(PasswordManagerTest, AutofillPredictionBeforeFormParsed) {
......
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