Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(86)

Unified Diff: chrome/browser/banners/app_banner_manager.cc

Issue 2951763003: Implement experimental app banner flow.
Patch Set: Browser test compile Created 3 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/banners/app_banner_manager.cc
diff --git a/chrome/browser/banners/app_banner_manager.cc b/chrome/browser/banners/app_banner_manager.cc
index 9524f7eff2093ecb1489a53631f279852a4d1392..7d26790a08dcfd067de6f2cacb2c4e8d2035c0f3 100644
--- a/chrome/browser/banners/app_banner_manager.cc
+++ b/chrome/browser/banners/app_banner_manager.cc
@@ -405,10 +405,12 @@ void AppBannerManager::DidFinishLoad(
has_sufficient_engagement_ = true;
// Start the pipeline immediately if we pass (or bypass) the engagement check,
- // or if the feature to run the installability check on page load is enabled.
+ // or if the feature to run the installability check on page load is enabled,
+ // or if the experimental banner flow is enabled.
if (has_sufficient_engagement_ ||
base::FeatureList::IsEnabled(
- features::kCheckInstallabilityForBannerOnLoad)) {
+ features::kCheckInstallabilityForBannerOnLoad) ||
+ IsExperimentalAppBannersEnabled()) {
RequestAppBanner(validated_url, false /* is_debug_mode */);
}
}
@@ -459,6 +461,11 @@ void AppBannerManager::OnEngagementIncreased(content::WebContents* contents,
}
}
+// static
+bool AppBannerManager::IsExperimentalAppBannersEnabled() {
+ return base::FeatureList::IsEnabled(features::kExperimentalAppBanners);
+}
+
void AppBannerManager::RecordCouldShowBanner() {
content::WebContents* contents = web_contents();
DCHECK(contents);
@@ -514,7 +521,6 @@ void AppBannerManager::OnBannerPromptReply(
// We don't need the controller any more, so reset it so the Blink-side object
// is destroyed.
controller_.reset();
- content::WebContents* contents = web_contents();
// The renderer might have requested the prompt to be canceled. They may
// request that it is redisplayed later, so don't Stop() here. However, log
@@ -525,13 +531,19 @@ void AppBannerManager::OnBannerPromptReply(
// request may be received *before* the Cancel prompt reply (e.g. if redisplay
// is requested in the beforeinstallprompt event handler).
referrer_ = referrer;
- if (reply == blink::mojom::AppBannerPromptReply::CANCEL) {
+ if (IsExperimentalAppBannersEnabled() ||
+ reply == blink::mojom::AppBannerPromptReply::CANCEL) {
UpdateState(State::PENDING_EVENT);
TrackBeforeInstallEvent(BEFORE_INSTALL_EVENT_PREVENT_DEFAULT_CALLED);
if (!page_requested_prompt_)
return;
}
+ if (!IsExperimentalAppBannersEnabled())
+ ShowBanner();
+}
+
+void AppBannerManager::ShowBanner() {
// If we haven't yet returned, but we're in the PENDING_EVENT state or
// |page_requested_prompt_| is true, the page has requested a delayed showing
// of the prompt. Otherwise, the prompt was never canceled by the page.
@@ -541,8 +553,12 @@ void AppBannerManager::OnBannerPromptReply(
UpdateState(State::ACTIVE);
} else {
TrackBeforeInstallEvent(BEFORE_INSTALL_EVENT_NO_ACTION);
+ // In the experimental flow, the banner is only shown if the site explicitly
+ // requests it to be shown.
+ DCHECK(!IsExperimentalAppBannersEnabled());
}
+ content::WebContents* contents = web_contents();
AppBannerSettingsHelper::RecordMinutesFromFirstVisitToShow(
contents, validated_url_, GetAppIdentifier(), GetCurrentTime());
@@ -552,14 +568,21 @@ void AppBannerManager::OnBannerPromptReply(
DCHECK(!primary_icon_.drawsNothing());
TrackBeforeInstallEvent(BEFORE_INSTALL_EVENT_COMPLETE);
- ShowBanner();
+ ShowBannerUI();
UpdateState(State::COMPLETE);
}
void AppBannerManager::DisplayAppBanner(bool user_gesture) {
+ // In the experimental flow, a user gesture is required for the prompt to be
+ // shown.
+ if (IsExperimentalAppBannersEnabled() && !user_gesture) {
+ // TODO: do some logging / error here
+ return;
+ }
+
if (is_pending_event()) {
// Simulate a non-canceled OnBannerPromptReply to show the delayed banner.
- OnBannerPromptReply(blink::mojom::AppBannerPromptReply::NONE, referrer_);
+ ShowBanner();
} else {
// Log that the prompt request was made for when we get the prompt reply.
page_requested_prompt_ = true;
« no previous file with comments | « chrome/browser/banners/app_banner_manager.h ('k') | chrome/browser/banners/app_banner_manager_browsertest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698