Skip to content

Commit

Permalink
[M116][UPMLocalPwd] Correct the triggers in the save/update password …
Browse files Browse the repository at this point in the history
…message

Dismissing the save or update password message no longer triggers
showing the sheet.
The sheet also doesn't show when the message times out.

(cherry picked from commit 4fe6a50)

Bug: 1457940
Change-Id: I4601a9d5b4f331b7a5741783e7e1b44781536d09
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4650048
Reviewed-by: Ioana Pandele <ioanap@chromium.org>
Auto-Submit: Ivana Žužić <izuzic@google.com>
Commit-Queue: Ivana Žužić <izuzic@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1163115}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4660801
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/branch-heads/5845@{#252}
Cr-Branched-From: 5a5dff6-refs/heads/main@{#1160321}
  • Loading branch information
Ivana Žužić authored and Chromium LUCI CQ committed Jun 30, 2023
1 parent aa250ee commit e69c23b
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -484,8 +484,10 @@ void SaveUpdatePasswordMessageDelegate::HandleMessageDismissed(
GetSaveUpdatePasswordMessageDismissReason(dismiss_reason));
}

TryToShowPasswordMigrationWarning(create_migration_warning_callback_,
web_contents_);
if (dismiss_reason == messages::DismissReason::PRIMARY_ACTION) {
TryToShowPasswordMigrationWarning(create_migration_warning_callback_,
web_contents_);
}
ClearState();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -651,10 +651,10 @@ TEST_F(SaveUpdatePasswordMessageDelegateTest,
TriggerDialogDismissedCallback(/*dialog_accepted=*/true);
}

// Tests that the local password migration warning will show when the user
// Tests that the local password migration warning will not show when the user
// dismisses the save password message.
TEST_F(SaveUpdatePasswordMessageDelegateTest,
TriggerLocalPasswordMigrationWarning_OnSaveMessageDismissed) {
DontTriggerLocalPasswordMigrationWarning_OnSaveMessageDismissed) {
base::test::ScopedFeatureList scoped_feature_state;
scoped_feature_state.InitAndEnableFeature(
password_manager::features::
Expand All @@ -664,7 +664,7 @@ TEST_F(SaveUpdatePasswordMessageDelegateTest,
EnqueueMessage(std::move(form_manager), /*user_signed_in=*/true,
/*update_password=*/false);
EXPECT_NE(nullptr, GetMessageWrapper());
EXPECT_CALL(GetMigrationWarningCallback(), Run);
EXPECT_CALL(GetMigrationWarningCallback(), Run).Times(0);
DismissMessage(messages::DismissReason::GESTURE);
EXPECT_EQ(nullptr, GetMessageWrapper());
}
Expand Down Expand Up @@ -692,10 +692,11 @@ TEST_F(SaveUpdatePasswordMessageDelegateTest,
EXPECT_EQ(nullptr, GetMessageWrapper());
}

// Tests that the local password migration warning will show when the user
// Tests that the local password migration warning will not show when the user
// dismisses the update password message.
TEST_F(SaveUpdatePasswordMessageDelegateTest,
TriggerLocalPasswordMigrationWarning_OnUpdatePasswordMessageDismissed) {
TEST_F(
SaveUpdatePasswordMessageDelegateTest,
DontTriggerLocalPasswordMigrationWarning_OnUpdatePasswordMessageDismissed) {
base::test::ScopedFeatureList scoped_feature_state;
scoped_feature_state.InitAndEnableFeature(
password_manager::features::
Expand All @@ -708,7 +709,7 @@ TEST_F(SaveUpdatePasswordMessageDelegateTest,
EnqueueMessage(std::move(form_manager), /*user_signed_in=*/true,
/*update_password=*/true);
EXPECT_NE(nullptr, GetMessageWrapper());
EXPECT_CALL(GetMigrationWarningCallback(), Run);
EXPECT_CALL(GetMigrationWarningCallback(), Run).Times(0);
DismissMessage(messages::DismissReason::GESTURE);
EXPECT_EQ(nullptr, GetMessageWrapper());
}
Expand Down Expand Up @@ -813,6 +814,38 @@ TEST_F(SaveUpdatePasswordMessageDelegateTest, MetricOnAutodismissTimer) {
password_manager::metrics_util::NO_DIRECT_INTERACTION, 1);
}

// Tests that the local password migration warning will not show when the user
// lets the save message time out.
TEST_F(SaveUpdatePasswordMessageDelegateTest,
DontTriggerLocalPasswordMigrationWarning_OnSaveMessageAutodismissTimer) {
auto form_manager =
CreateFormManager(GURL(kDefaultUrl), empty_best_matches());
EnqueueMessage(std::move(form_manager), /*user_signed_in=*/false,
/*update_password=*/false);
EXPECT_NE(nullptr, GetMessageWrapper());
EXPECT_CALL(GetMigrationWarningCallback(), Run).Times(0);
DismissMessage(messages::DismissReason::TIMER);
EXPECT_EQ(nullptr, GetMessageWrapper());
}

// Tests that the local password migration warning will not show when the user
// lets the update message time out.
TEST_F(
SaveUpdatePasswordMessageDelegateTest,
DontTriggerLocalPasswordMigrationWarning_OnUpdateMessageAutodismissTimer) {
SetPendingCredentials(kUsername, kPassword);
PasswordForm password_form = CreatePasswordForm(kUsername, kPassword);
std::vector<const PasswordForm*> single_form_best_matches = {&password_form};
auto form_manager =
CreateFormManager(GURL(kDefaultUrl), &single_form_best_matches);
EnqueueMessage(std::move(form_manager), /*user_signed_in=*/true,
/*update_password=*/true);
EXPECT_NE(nullptr, GetMessageWrapper());
EXPECT_CALL(GetMigrationWarningCallback(), Run).Times(0);
DismissMessage(messages::DismissReason::TIMER);
EXPECT_EQ(nullptr, GetMessageWrapper());
}

// Tests that update password message with a single PasswordForm immediately
// saves the form on Update button tap and doesn't display confirmation dialog.
TEST_F(SaveUpdatePasswordMessageDelegateTest, UpdatePasswordWithSingleForm) {
Expand Down Expand Up @@ -1099,6 +1132,27 @@ TEST_F(SaveUpdatePasswordMessageDelegateTest,
1);
}

// Verifies that the password migration warning is not shown after selecting
// "Never for this site" menu option in the Save message.
TEST_F(SaveUpdatePasswordMessageDelegateTest,
DontTriggerLocalPasswordMigrationWarning_OnNeverSave) {
base::test::ScopedFeatureList scoped_feature_state;
scoped_feature_state.InitAndEnableFeature(
password_manager::features::
kUnifiedPasswordManagerLocalPasswordsMigrationWarning);

auto form_manager =
CreateFormManager(GURL(kDefaultUrl), empty_best_matches());
MockPasswordFormManagerForUI* form_manager_pointer = form_manager.get();

EnqueueMessage(std::move(form_manager), /*user_signed_in=*/false,
/*update_password=*/false);
EXPECT_NE(nullptr, GetMessageWrapper());
EXPECT_CALL(GetMigrationWarningCallback(), Run).Times(0);
EXPECT_CALL(*form_manager_pointer, Blocklist());
TriggerNeverSaveMenuItem();
}

// Verifies that:
// 1. Update password dialog is shown after clicking on cog button (secondary
// action) in the message.
Expand Down

0 comments on commit e69c23b

Please sign in to comment.