Skip to content

Commit

Permalink
Do not show a snackbar if already showing one
Browse files Browse the repository at this point in the history
This is a temporary fix and in the future we would overwrite the
dismiss the currently showing snackbar and show the new one with proper
garbage collection. crbug.com/1450942

(cherry picked from commit 721bef8)

Bug: 1450568
Change-Id: I064d76eccc06c6e23fcde9fd1122e87068ffa1c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4583539
Reviewed-by: Christoph Schwering <schwering@google.com>
Commit-Queue: Siddharth Shah <siashah@chromium.org>
Reviewed-by: Vinny Persky <vinnypersky@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1153332}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4599286
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5790@{#499}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
siashah authored and Chromium LUCI CQ committed Jun 8, 2023
1 parent 9923b92 commit 39dc7b9
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 19 deletions.
Expand Up @@ -27,20 +27,17 @@ AutofillSnackbarControllerImpl::~AutofillSnackbarControllerImpl() {
void AutofillSnackbarControllerImpl::Show(
AutofillSnackbarType autofill_snackbar_type) {
CHECK_NE(autofill_snackbar_type, AutofillSnackbarType::kUnspecified);
autofill_snackbar_type_ = autofill_snackbar_type;
if (!autofill_snackbar_view_) {
autofill_snackbar_view_ = AutofillSnackbarView::Create(this);
if (autofill_snackbar_view_) {
// A snackbar is already showing. Ignore the new request.
return;
}
autofill_snackbar_type_ = autofill_snackbar_type;
autofill_snackbar_view_ = AutofillSnackbarView::Create(this);
autofill_snackbar_view_->Show();
base::UmaHistogramBoolean(
"Autofill.Snackbar." + GetSnackbarTypeForLogging() + ".Shown", true);
}

void AutofillSnackbarControllerImpl::SetViewForTesting(
AutofillSnackbarView* view) {
autofill_snackbar_view_ = view;
}

void AutofillSnackbarControllerImpl::OnActionClicked() {
switch (autofill_snackbar_type_) {
case AutofillSnackbarType::kVirtualCard:
Expand Down
Expand Up @@ -27,7 +27,6 @@ class AutofillSnackbarControllerImpl : public AutofillSnackbarController {

// Show the snackbar.
void Show(AutofillSnackbarType autofill_snackbar_type);
void SetViewForTesting(AutofillSnackbarView* view);

// AutofillSnackbarController:
void OnActionClicked() override;
Expand Down
Expand Up @@ -20,21 +20,13 @@ using testing::NiceMock;

namespace autofill {

class MockAutofillSnackbarView : public AutofillSnackbarView {
public:
MockAutofillSnackbarView() = default;
void Show() override {}
void Dismiss() override {}
};

class AutofillSnackbarControllerImplTest
: public ChromeRenderViewHostTestHarness {
public:
AutofillSnackbarControllerImplTest() = default;

void SetUp() override {
ChromeRenderViewHostTestHarness::SetUp();
controller()->SetViewForTesting(new MockAutofillSnackbarView());
ManualFillingControllerImpl::CreateForWebContentsForTesting(
web_contents(), mock_pwd_controller_.AsWeakPtr(),
mock_address_controller_.AsWeakPtr(), mock_cc_controller_.AsWeakPtr(),
Expand Down Expand Up @@ -65,8 +57,6 @@ TEST_F(AutofillSnackbarControllerImplTest, VirtualCardTypeMetricsTest) {
"Autofill.Snackbar.VirtualCard.ActionClicked", 1, 0);
controller()->OnDismissed();

// Reset the mock view.
controller()->SetViewForTesting(new MockAutofillSnackbarView());
controller()->Show(AutofillSnackbarType::kVirtualCard);
controller()->OnActionClicked();
// Verify that the count for both Shown and ActionClicked is incremented.
Expand All @@ -76,6 +66,25 @@ TEST_F(AutofillSnackbarControllerImplTest, VirtualCardTypeMetricsTest) {
"Autofill.Snackbar.VirtualCard.ActionClicked", 1, 1);
}

TEST_F(AutofillSnackbarControllerImplTest,
AttemptToShowDialogWhileAlreadyShowing) {
base::HistogramTester histogram_tester;
controller()->Show(AutofillSnackbarType::kVirtualCard);
// Verify that the count for Shown is incremented and ActionClicked hasn't
// changed.
histogram_tester.ExpectUniqueSample("Autofill.Snackbar.VirtualCard.Shown", 1,
1);
histogram_tester.ExpectUniqueSample(
"Autofill.Snackbar.VirtualCard.ActionClicked", 1, 0);

// Attempt to show another dialog without dismissing the previous one.
controller()->Show(AutofillSnackbarType::kVirtualCard);

// Verify that the count for both Shown is not incremented.
histogram_tester.ExpectUniqueSample("Autofill.Snackbar.VirtualCard.Shown", 1,
1);
}

TEST_F(AutofillSnackbarControllerImplTest, MandatoryReauthTypeMetricsTest) {
base::HistogramTester histogram_tester;
controller()->Show(AutofillSnackbarType::kMandatoryReauth);
Expand Down

0 comments on commit 39dc7b9

Please sign in to comment.