Skip to content

Commit

Permalink
[NTP] Don't show Customize Chrome promo when signin modal dialog is open
Browse files Browse the repository at this point in the history
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

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-Commit-Position: refs/heads/main@{#1153865}
  • Loading branch information
Gabriel Oliveira authored and Chromium LUCI CQ committed Jun 6, 2023
1 parent 82d3fef commit 4692c79
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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();
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down
9 changes: 8 additions & 1 deletion chrome/browser/ui/webui/new_tab_page/new_tab_page_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,10 @@ class MockCustomizeChromeFeaturePromoHelper
CloseCustomizeChromeFeaturePromo,
(content::WebContents*),
(override));
MOCK_METHOD(bool,
IsSigninModalDialogOpen,
(content::WebContents*),
(override));

~MockCustomizeChromeFeaturePromoHelper() override = default;
};
Expand Down Expand Up @@ -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);
Expand All @@ -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();
}

0 comments on commit 4692c79

Please sign in to comment.