Skip to content

Commit

Permalink
🤝 Disable app banners on Trusted Web Activities.
Browse files Browse the repository at this point in the history
Chances are the user of a TWA already has an app installed for the site
they are viewing. In this case we should not show the app install
banner.

Bug: 906673
Change-Id: Ic09242e902cb4a76cc3deb879a1dd62b71614bf7
Reviewed-on: https://chromium-review.googlesource.com/c/1342618
Reviewed-by: Michael van Ouwerkerk <mvanouwerkerk@chromium.org>
Commit-Queue: Peter Conn <peconn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609660}
  • Loading branch information
Peter E Conn authored and Commit Bot committed Nov 20, 2018
1 parent dae7384 commit 41be107
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -527,8 +527,12 @@ public TabDelegateFactory createDefaultTabDelegateFactory() {
}

private CustomTabDelegateFactory createCustomTabDelegateFactory() {
// Don't show an app install banner for the user of a Trusted Web Activity - they've already
// got an app installed!
// TODO(peconn): Look into allowing the banner again if the user leaves the Trusted origin.
boolean allowAppBanners = !mIntentDataProvider.isTrustedWebActivity();
return new CustomTabDelegateFactory(mIntentDataProvider.shouldEnableUrlBarHiding(),
mIntentDataProvider.isOpenedByChrome(),
mIntentDataProvider.isOpenedByChrome(), allowAppBanners,
getComponent().resolveControlsVisibilityDelegate());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ public void openNewTab(String url, String extraHeaders, ResourceRequestBody post

private final boolean mShouldHideBrowserControls;
private final boolean mIsOpenedByChrome;
private final boolean mShouldAllowAppBanners;
private final BrowserControlsVisibilityDelegate mBrowserStateVisibilityDelegate;

private ExternalNavigationDelegateImpl mNavigationDelegate;
Expand All @@ -200,16 +201,26 @@ public void openNewTab(String url, String extraHeaders, ResourceRequestBody post
/**
* @param shouldHideBrowserControls Whether or not the browser controls may auto-hide.
* @param isOpenedByChrome Whether the CustomTab was originally opened by Chrome.
* @param shouldAllowAppBanners Whether app install banners can be shown.
* @param visibilityDelegate The delegate that handles browser control visibility associated
* with browser actions (as opposed to tab state).
*/
public CustomTabDelegateFactory(boolean shouldHideBrowserControls, boolean isOpenedByChrome,
BrowserControlsVisibilityDelegate visibilityDelegate) {
boolean shouldAllowAppBanners, BrowserControlsVisibilityDelegate visibilityDelegate) {
mShouldHideBrowserControls = shouldHideBrowserControls;
mIsOpenedByChrome = isOpenedByChrome;
mShouldAllowAppBanners = shouldAllowAppBanners;
mBrowserStateVisibilityDelegate = visibilityDelegate;
}

/**
* Creates a basic/empty CustomTabDelegateFactory for use when creating a hidden tab. It will
* be replaced when the hidden Tab becomes shown.
*/
static CustomTabDelegateFactory createDummy() {
return new CustomTabDelegateFactory(false, false, false, null);
}

@Override
public BrowserControlsVisibilityDelegate createBrowserControlsVisibilityDelegate(Tab tab) {
TabStateBrowserControlsVisibilityDelegate tabDelegate =
Expand Down Expand Up @@ -247,6 +258,11 @@ public ContextMenuPopulator createContextMenuPopulator(Tab tab) {
ChromeContextMenuPopulator.ContextMenuMode.CUSTOM_TAB);
}

@Override
public boolean canShowAppBanners(Tab tab) {
return mShouldAllowAppBanners;
}

/**
* @return The {@link ExternalNavigationHandler} in this tab. For test purpose only.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1445,7 +1445,7 @@ private void launchUrlInHiddenTab(
if (extras != null) extrasIntent.putExtras(extras);
if (IntentHandler.getExtraHeadersFromIntent(extrasIntent) != null) return;

Tab tab = Tab.createDetached(new CustomTabDelegateFactory(false, false, null));
Tab tab = Tab.createDetached(CustomTabDelegateFactory.createDummy());
HiddenTabObserver observer = new HiddenTabObserver(this);
tab.addObserver(observer);

Expand Down

0 comments on commit 41be107

Please sign in to comment.