[M115][NTP] Don't show Customize Chrome promo when signin modal dialo…

…g 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 4692c79) Bug: 1444999 Change-Id: Id32d67cea6861d82f833a0299050dc66fbf10051 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4580748 Commit-Queue: Gabriel Oliveira <gabolvr@google.com> Reviewed-by: David Roger <droger@chromium.org> Reviewed-by: Paul Adedeji <pauladedeji@google.com> 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: 1d71a33-refs/heads/main@{#1148114}
chromium · Jun 8, 2023 · dd27b36 · dd27b36
1 parent 237af12
commit dd27b36
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 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
@@ -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
@@ -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
@@ -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
@@ -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();
+}