From dd27b366a361bf20071670230b783fa40d33f993 Mon Sep 17 00:00:00 2001 From: Gabriel Oliveira Date: Thu, 8 Jun 2023 11:06:40 +0000 Subject: [PATCH] [M115][NTP] Don't show Customize Chrome promo when signin modal dialog is open This CL avoids showing the Customize Chrome IPH promo when a Signin modal dialog is open (Sync Confirmation or Profile Customization). It will be shown in the next NTP. Demo of the current behavior: https://screencast.googleplex.com/cast/NTk2MDg5MzM2OTE1NTU4NHxmNzRmYWIwZC1iNQ https://screencast.googleplex.com/cast/NDUzOTUxODkzOTAzNzY5Nnw2MjU1NjUwNC0wMw Preview: https://screencast.googleplex.com/cast/NjI5NTQ2NzAyNjU0NjY4OHw1ZmNiYzFlNS0zMw (cherry picked from commit 4692c7994a336c331cb97235ad6a1a17cf18e0cb) Bug: 1444999 Change-Id: Id32d67cea6861d82f833a0299050dc66fbf10051 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4580748 Commit-Queue: Gabriel Oliveira Reviewed-by: David Roger Reviewed-by: Paul Adedeji Cr-Original-Commit-Position: refs/heads/main@{#1153865} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4598015 Cr-Commit-Position: refs/branch-heads/5790@{#487} Cr-Branched-From: 1d71a337b1f6e707a13ae074dca1e2c34905eb9f-refs/heads/main@{#1148114} --- .../customize_chrome_feature_promo_helper.cc | 8 +++++++ .../customize_chrome_feature_promo_helper.h | 1 + .../new_tab_page/new_tab_page_handler.cc | 9 ++++++- .../new_tab_page_handler_unittest.cc | 24 +++++++++++++++++++ 4 files changed, 41 insertions(+), 1 deletion(-) diff --git a/chrome/browser/new_tab_page/customize_chrome/customize_chrome_feature_promo_helper.cc b/chrome/browser/new_tab_page/customize_chrome/customize_chrome_feature_promo_helper.cc index 12cae22e09f59..01b2f98e4a452 100644 --- a/chrome/browser/new_tab_page/customize_chrome/customize_chrome_feature_promo_helper.cc +++ b/chrome/browser/new_tab_page/customize_chrome/customize_chrome_feature_promo_helper.cc @@ -5,6 +5,8 @@ #include "chrome/browser/new_tab_page/customize_chrome/customize_chrome_feature_promo_helper.h" #include "chrome/browser/feature_engagement/tracker_factory.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/browser/ui/browser_finder.h" #include "chrome/browser/ui/browser_window.h" #include "components/feature_engagement/public/event_constants.h" #include "components/feature_engagement/public/feature_constants.h" @@ -43,3 +45,9 @@ void CustomizeChromeFeaturePromoHelper::CloseCustomizeChromeFeaturePromo( } } } + +bool CustomizeChromeFeaturePromoHelper::IsSigninModalDialogOpen( + content::WebContents* web_contents) { + auto* browser = chrome::FindBrowserWithWebContents(web_contents); + return browser->signin_view_controller()->ShowsModalDialog(); +} diff --git a/chrome/browser/new_tab_page/customize_chrome/customize_chrome_feature_promo_helper.h b/chrome/browser/new_tab_page/customize_chrome/customize_chrome_feature_promo_helper.h index bb2291131e836..27c2a020a896b 100644 --- a/chrome/browser/new_tab_page/customize_chrome/customize_chrome_feature_promo_helper.h +++ b/chrome/browser/new_tab_page/customize_chrome/customize_chrome_feature_promo_helper.h @@ -15,6 +15,7 @@ class CustomizeChromeFeaturePromoHelper { content::WebContents* web_contents); virtual void CloseCustomizeChromeFeaturePromo( content::WebContents* web_contents); + virtual bool IsSigninModalDialogOpen(content::WebContents* web_contents); virtual ~CustomizeChromeFeaturePromoHelper() = default; }; diff --git a/chrome/browser/ui/webui/new_tab_page/new_tab_page_handler.cc b/chrome/browser/ui/webui/new_tab_page/new_tab_page_handler.cc index 5ece1860ee34d..600ca6c687a27 100644 --- a/chrome/browser/ui/webui/new_tab_page/new_tab_page_handler.cc +++ b/chrome/browser/ui/webui/new_tab_page/new_tab_page_handler.cc @@ -863,7 +863,14 @@ void NewTabPageHandler::MaybeShowCustomizeChromeFeaturePromo() { const auto customize_chrome_button_open_count = profile_->GetPrefs()->GetInteger( prefs::kNtpCustomizeChromeButtonOpenCount); - if (customize_chrome_button_open_count == 0) { + + // If a sign-in dialog is being currently displayed, the promo should not be + // shown to avoid conflict. The sign-in dialog would be shown as soon as the + // browser is opened, before the promo. + bool is_signin_modal_dialog_open = + customize_chrome_feature_promo_helper_->IsSigninModalDialogOpen( + web_contents_.get()); + if (customize_chrome_button_open_count == 0 && !is_signin_modal_dialog_open) { customize_chrome_feature_promo_helper_ ->MaybeShowCustomizeChromeFeaturePromo(web_contents_.get()); } diff --git a/chrome/browser/ui/webui/new_tab_page/new_tab_page_handler_unittest.cc b/chrome/browser/ui/webui/new_tab_page/new_tab_page_handler_unittest.cc index cfd4a0b8ca0cb..48b0814e60d1c 100644 --- a/chrome/browser/ui/webui/new_tab_page/new_tab_page_handler_unittest.cc +++ b/chrome/browser/ui/webui/new_tab_page/new_tab_page_handler_unittest.cc @@ -211,6 +211,10 @@ class MockCustomizeChromeFeaturePromoHelper CloseCustomizeChromeFeaturePromo, (content::WebContents*), (override)); + MOCK_METHOD(bool, + IsSigninModalDialogOpen, + (content::WebContents*), + (override)); ~MockCustomizeChromeFeaturePromoHelper() override = default; }; @@ -1136,6 +1140,9 @@ TEST_F(NewTabPageHandlerTest, IncrementCustomizeChromeButtonOpenCount) { } TEST_F(NewTabPageHandlerTest, MaybeShowCustomizeChromeFeaturePromo) { + EXPECT_CALL(*mock_customize_chrome_feature_promo_helper_, + IsSigninModalDialogOpen) + .WillRepeatedly(testing::Return(false)); EXPECT_EQ(profile_->GetPrefs()->GetInteger( prefs::kNtpCustomizeChromeButtonOpenCount), 0); @@ -1157,3 +1164,20 @@ TEST_F(NewTabPageHandlerTest, MaybeShowCustomizeChromeFeaturePromo) { mock_page_.FlushForTesting(); } + +TEST_F(NewTabPageHandlerTest, + DontShowCustomizeChromeFeaturePromoWhenModalDialogIsOpen) { + EXPECT_CALL(*mock_customize_chrome_feature_promo_helper_, + IsSigninModalDialogOpen) + .WillRepeatedly(testing::Return(true)); + EXPECT_EQ(profile_->GetPrefs()->GetInteger( + prefs::kNtpCustomizeChromeButtonOpenCount), + 0); + EXPECT_CALL(*mock_customize_chrome_feature_promo_helper_, + MaybeShowCustomizeChromeFeaturePromo) + .Times(0); + + handler_->MaybeShowCustomizeChromeFeaturePromo(); + + mock_page_.FlushForTesting(); +}