Commit 31eb61b8 authored by Marc Treib's avatar Marc Treib Committed by Commit Bot

PasswordManagerClientHelper: don't prompt to move in incognito

This CL adds a check in PasswordManagerClientHelper so that it doesn't
call PromptUserToMovePasswordToAccount() in incognito mode.
Note that this does not actually change the overall behavior: The move
prompt was already suppressed in incognito, but in a somewhat
convoluted (and potentially fragile) way (see bug).

Bug: 1094918, 1093310
Change-Id: Iccfaaae7095d585c5ea39d3bf409c75c288ab723
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2247828
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: default avatarMohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779310}
parent 5f87dca8
...@@ -40,9 +40,7 @@ void PasswordManagerClientHelper::NotifySuccessfulLoginWithExistingPassword( ...@@ -40,9 +40,7 @@ void PasswordManagerClientHelper::NotifySuccessfulLoginWithExistingPassword(
// Check if it is necessary to prompt user to enable auto sign-in. // Check if it is necessary to prompt user to enable auto sign-in.
if (possible_auto_sign_in_) { if (possible_auto_sign_in_) {
delegate_->PromptUserToEnableAutosignin(); delegate_->PromptUserToEnableAutosignin();
} else if (base::FeatureList::IsEnabled( } else if (ShouldPromptToMovePasswordToAccount(*submitted_manager)) {
password_manager::features::kEnablePasswordsAccountStorage) &&
submitted_manager->IsMovableToAccountStore()) {
delegate_->PromptUserToMovePasswordToAccount(std::move(submitted_manager)); delegate_->PromptUserToMovePasswordToAccount(std::move(submitted_manager));
} }
} }
...@@ -84,4 +82,12 @@ bool PasswordManagerClientHelper::ShouldPromptToEnableAutoSignIn() const { ...@@ -84,4 +82,12 @@ bool PasswordManagerClientHelper::ShouldPromptToEnableAutoSignIn() const {
!delegate_->IsIncognito(); !delegate_->IsIncognito();
} }
bool PasswordManagerClientHelper::ShouldPromptToMovePasswordToAccount(
const PasswordFormManagerForUI& submitted_manager) const {
return base::FeatureList::IsEnabled(
password_manager::features::kEnablePasswordsAccountStorage) &&
submitted_manager.IsMovableToAccountStore() &&
!delegate_->IsIncognito();
}
} // namespace password_manager } // namespace password_manager
...@@ -49,6 +49,12 @@ class PasswordManagerClientHelper { ...@@ -49,6 +49,12 @@ class PasswordManagerClientHelper {
// is the case for first run experience, and only for non-incognito mode. // is the case for first run experience, and only for non-incognito mode.
bool ShouldPromptToEnableAutoSignIn() const; bool ShouldPromptToEnableAutoSignIn() const;
// Returns whether the user should be prompted to move the submitted password
// to the account-scoped store. This is the case if the password is movable,
// the corresponding feature flag is enabled, and only for non-incognito mode.
bool ShouldPromptToMovePasswordToAccount(
const PasswordFormManagerForUI& submitted_manager) const;
PasswordManagerClient* delegate_; PasswordManagerClient* delegate_;
// Set during 'NotifyUserCouldBeAutoSignedIn' in order to store the // Set during 'NotifyUserCouldBeAutoSignedIn' in order to store the
......
...@@ -104,10 +104,13 @@ TEST_F(PasswordManagerClientHelperTest, PromptAutosigninAfterSuccessfulLogin) { ...@@ -104,10 +104,13 @@ TEST_F(PasswordManagerClientHelperTest, PromptAutosigninAfterSuccessfulLogin) {
CreateFormManager(&form, /*is_movable=*/true)); CreateFormManager(&form, /*is_movable=*/true));
} }
TEST_F(PasswordManagerClientHelperTest, PromptAutosigninDisabledInIncognito) { TEST_F(PasswordManagerClientHelperTest,
PromptAutosigninAndMoveDisabledInIncognito) {
EXPECT_CALL(*client(), IsIncognito) EXPECT_CALL(*client(), IsIncognito)
.Times(AnyNumber()) .Times(AnyNumber())
.WillRepeatedly(Return(true)); .WillRepeatedly(Return(true));
// In Incognito, both the auto-signin and the "Move password to account?"
// bubbles should be disabled.
EXPECT_CALL(*client(), PromptUserToEnableAutosignin).Times(0); EXPECT_CALL(*client(), PromptUserToEnableAutosignin).Times(0);
EXPECT_CALL(*client(), PromptUserToMovePasswordToAccount).Times(0); EXPECT_CALL(*client(), PromptUserToMovePasswordToAccount).Times(0);
......
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