From 39dc7b949d0ffc704ef3debe3c79b4c4b9082cb8 Mon Sep 17 00:00:00 2001 From: siashah Date: Thu, 8 Jun 2023 20:26:04 +0000 Subject: [PATCH] Do not show a snackbar if already showing one 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 721bef843b216bd9bcd57f671112c8a0f449a3ae) Bug: 1450568 Change-Id: I064d76eccc06c6e23fcde9fd1122e87068ffa1c5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4583539 Reviewed-by: Christoph Schwering Commit-Queue: Siddharth Shah Reviewed-by: Vinny Persky Cr-Original-Commit-Position: refs/heads/main@{#1153332} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4599286 Bot-Commit: Rubber Stamper Cr-Commit-Position: refs/branch-heads/5790@{#499} Cr-Branched-From: 1d71a337b1f6e707a13ae074dca1e2c34905eb9f-refs/heads/main@{#1148114} --- .../autofill_snackbar_controller_impl.cc | 13 ++++----- .../autofill_snackbar_controller_impl.h | 1 - ...ofill_snackbar_controller_impl_unittest.cc | 29 ++++++++++++------- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/chrome/browser/ui/autofill/payments/autofill_snackbar_controller_impl.cc b/chrome/browser/ui/autofill/payments/autofill_snackbar_controller_impl.cc index 42fe3c1e3f986..687bda90ed794 100644 --- a/chrome/browser/ui/autofill/payments/autofill_snackbar_controller_impl.cc +++ b/chrome/browser/ui/autofill/payments/autofill_snackbar_controller_impl.cc @@ -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: diff --git a/chrome/browser/ui/autofill/payments/autofill_snackbar_controller_impl.h b/chrome/browser/ui/autofill/payments/autofill_snackbar_controller_impl.h index b51db632d89cf..e886ef0d3e0d7 100644 --- a/chrome/browser/ui/autofill/payments/autofill_snackbar_controller_impl.h +++ b/chrome/browser/ui/autofill/payments/autofill_snackbar_controller_impl.h @@ -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; diff --git a/chrome/browser/ui/autofill/payments/autofill_snackbar_controller_impl_unittest.cc b/chrome/browser/ui/autofill/payments/autofill_snackbar_controller_impl_unittest.cc index 71c052dd83b3b..b72843882b50f 100644 --- a/chrome/browser/ui/autofill/payments/autofill_snackbar_controller_impl_unittest.cc +++ b/chrome/browser/ui/autofill/payments/autofill_snackbar_controller_impl_unittest.cc @@ -20,13 +20,6 @@ using testing::NiceMock; namespace autofill { -class MockAutofillSnackbarView : public AutofillSnackbarView { - public: - MockAutofillSnackbarView() = default; - void Show() override {} - void Dismiss() override {} -}; - class AutofillSnackbarControllerImplTest : public ChromeRenderViewHostTestHarness { public: @@ -34,7 +27,6 @@ class AutofillSnackbarControllerImplTest 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(), @@ -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. @@ -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);