Skip to content

Commit

Permalink
Fix CCT treating intents as background due to timing issue
Browse files Browse the repository at this point in the history
Sometimes CCT starts processing an intent before native initialization
has finished, which means the tab may still be hidden. This causes CCT
to treat these intents as background intents. In other cases, CCT may handle the intent before the application state
has been updated to "running". To solve this, we can allow background
navigations if incomingIntentRedirect is true.

(cherry picked from commit ed79acd)

Bug: 1307848
Change-Id: I4cb1ede3ad50bf112262178610fa3a3537776829
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3547199
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#984952}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3551496
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Krishna Govind <govind@chromium.org>
Commit-Queue: Krishna Govind <govind@chromium.org>
Owners-Override: Krishna Govind <govind@chromium.org>
Auto-Submit: Krishna Govind <govind@chromium.org>
Cr-Commit-Position: refs/branch-heads/4962@{#5}
Cr-Branched-From: 0234f19-refs/heads/main@{#984778}
  • Loading branch information
clarkduvall authored and Krishna Govind committed Mar 24, 2022
1 parent b3a56fa commit aa21594
Showing 1 changed file with 33 additions and 19 deletions.
Expand Up @@ -367,7 +367,7 @@ private OverrideUrlLoadingResult handleFallbackUrl(ExternalNavigationParams para
}

if (canLaunchExternalFallback) {
if (shouldBlockAllExternalAppLaunches(params)) {
if (shouldBlockAllExternalAppLaunches(params, false /* incomingIntentRedirect */)) {
throw new SecurityException("Context is not allowed to launch an external app.");
}
if (!params.isIncognito()) {
Expand Down Expand Up @@ -471,7 +471,12 @@ private boolean blockExternalNavFromAutoSubframe(ExternalNavigationParams params
/**
* http://crbug.com/441284 : Disallow firing external intent while the app is in the background.
*/
private boolean blockExternalNavWhileBackgrounded(ExternalNavigationParams params) {
private boolean blockExternalNavWhileBackgrounded(
ExternalNavigationParams params, boolean incomingIntentRedirect) {
// If the redirect is from an intent Chrome could still be transitioning to the foreground.
// Alternatively, the user may have sent Chrome to the background by this point, but for
// navigations started by another app that should still be safe.
if (incomingIntentRedirect) return false;
if (params.isApplicationMustBeInForeground() && !mDelegate.isApplicationInForeground()) {
if (DEBUG) Log.i(TAG, "App is not in foreground");
return true;
Expand All @@ -480,7 +485,12 @@ private boolean blockExternalNavWhileBackgrounded(ExternalNavigationParams param
}

/** http://crbug.com/464669 : Disallow firing external intent from background tab. */
private boolean blockExternalNavFromBackgroundTab(ExternalNavigationParams params) {
private boolean blockExternalNavFromBackgroundTab(
ExternalNavigationParams params, boolean incomingIntentRedirect) {
// See #blockExternalNavWhileBackgrounded - isBackgroundTabNavigation is effectively
// checking both that the tab is foreground, and the app is foreground, so we can skip it
// for intent launches for the same reason.
if (incomingIntentRedirect) return false;
if (params.isBackgroundTabNavigation()
&& !params.areIntentLaunchesAllowedInBackgroundTabs()) {
if (DEBUG) Log.i(TAG, "Navigation in background tab");
Expand Down Expand Up @@ -1264,9 +1274,12 @@ params, targetIntent, browserFallbackUrl, isGoogleReferrer())) {
}

// Check if we're navigating under conditions that should never launch an external app.
private boolean shouldBlockAllExternalAppLaunches(ExternalNavigationParams params) {
return blockExternalNavFromAutoSubframe(params) || blockExternalNavWhileBackgrounded(params)
|| blockExternalNavFromBackgroundTab(params) || ignoreBackForwardNav(params);
private boolean shouldBlockAllExternalAppLaunches(
ExternalNavigationParams params, boolean incomingIntentRedirect) {
return blockExternalNavFromAutoSubframe(params)
|| blockExternalNavWhileBackgrounded(params, incomingIntentRedirect)
|| blockExternalNavFromBackgroundTab(params, incomingIntentRedirect)
|| ignoreBackForwardNav(params);
}

private void recordIntentSelectorMetrics(GURL targetUrl, Intent targetIntent) {
Expand All @@ -1285,7 +1298,20 @@ private OverrideUrlLoadingResult shouldOverrideUrlLoadingInternal(
// Don't allow external fallback URLs by default.
canLaunchExternalFallbackResult.set(false);

if (shouldBlockAllExternalAppLaunches(params)) {
int pageTransitionCore = params.getPageTransition() & PageTransition.CORE_MASK;
boolean isLink = pageTransitionCore == PageTransition.LINK;
boolean isFromIntent = (params.getPageTransition() & PageTransition.FROM_API) != 0;

boolean isOnEffectiveIntentRedirect = params.getRedirectHandler() == null
? false
: params.getRedirectHandler().isOnEffectiveIntentRedirectChain();

// http://crbug.com/170925: We need to show the intent picker when we receive an intent from
// another app that 30x redirects to a YouTube/Google Maps/Play Store/Google+ URL etc.
boolean incomingIntentRedirect =
(isLink && isFromIntent && params.isRedirect()) || isOnEffectiveIntentRedirect;

if (shouldBlockAllExternalAppLaunches(params, incomingIntentRedirect)) {
return OverrideUrlLoadingResult.forNoOverride();
}

Expand Down Expand Up @@ -1317,21 +1343,9 @@ private OverrideUrlLoadingResult shouldOverrideUrlLoadingInternal(
return OverrideUrlLoadingResult.forNoOverride();
}

int pageTransitionCore = params.getPageTransition() & PageTransition.CORE_MASK;
boolean isLink = pageTransitionCore == PageTransition.LINK;
boolean isFormSubmit = pageTransitionCore == PageTransition.FORM_SUBMIT;
boolean isFromIntent = (params.getPageTransition() & PageTransition.FROM_API) != 0;
boolean linkNotFromIntent = isLink && !isFromIntent;

boolean isOnEffectiveIntentRedirect = params.getRedirectHandler() == null
? false
: params.getRedirectHandler().isOnEffectiveIntentRedirectChain();

// http://crbug.com/170925: We need to show the intent picker when we receive an intent from
// another app that 30x redirects to a YouTube/Google Maps/Play Store/Google+ URL etc.
boolean incomingIntentRedirect =
(isLink && isFromIntent && params.isRedirect()) || isOnEffectiveIntentRedirect;

if (handleCCTRedirectsToInstantApps(params, isExternalProtocol, incomingIntentRedirect)) {
return OverrideUrlLoadingResult.forExternalIntent();
} else if (redirectShouldStayInApp(params, isExternalProtocol, targetIntent)) {
Expand Down

0 comments on commit aa21594

Please sign in to comment.