Commit 7a42abf1 authored by Colin Blundell's avatar Colin Blundell Committed by Commit Bot

[Android] Abstract ExternalNavHandler dependence on InstantAppsHandler

ExternalNavigationHandler has a dependence on InstantAppsHandler for
//chrome-level functionality of interaction with instant apps. This CL
abstracts that dependence through ExternalNavigationDelegate.java to aid
in componentization of ExternalNavigationHandler for sharing with
WebLayer. The functionality exercised by the existing tests is now split
between ExternalNavigationHandlerTest.java (testing functionality
independent of the delegate impl) and
ExternalNavigationDelegateImplTest.java (testing the production
functionality now contained in the delegate impl).

Note that there is a longer-term issue of to what extent we will
incorporate //chrome's instantapps code in WebLayer. That is a larger
issue than just the functionality covered in this CL (e.g., the
existing maybeLaunchInstantApp() method of
ExternalNavigationDelegate.java). We are examining that issue
separately.

Bug: 1031465
Change-Id: Ie22ee0083217dba65622eb1f4274a5e587b0c8b5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2085211Reviewed-by: default avatarMichael Thiessen <mthiesse@chromium.org>
Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748157}
parent 1e3ecfea
...@@ -114,6 +114,12 @@ interface ExternalNavigationDelegate { ...@@ -114,6 +114,12 @@ interface ExternalNavigationDelegate {
/** Records the pending referrer if desired. */ /** Records the pending referrer if desired. */
void maybeSetPendingReferrer(Intent intent, @NonNull String referrerUrl); void maybeSetPendingReferrer(Intent intent, @NonNull String referrerUrl);
/**
* Adjusts any desired extras related to intents to instant apps based on the value of
* |insIntentToInstantApp}.
*/
void maybeAdjustInstantAppExtras(Intent intent, boolean isIntentToInstantApp);
/** /**
* Records the pending incognito URL if desired. Called only if the * Records the pending incognito URL if desired. Called only if the
* navigation is occurring in the context of incognito mode. * navigation is occurring in the context of incognito mode.
...@@ -161,6 +167,12 @@ interface ExternalNavigationDelegate { ...@@ -161,6 +167,12 @@ interface ExternalNavigationDelegate {
*/ */
boolean isIntentForTrustedCallingApp(Intent intent); boolean isIntentForTrustedCallingApp(Intent intent);
/**
* @param intent The intent to launch.
* @return Whether the Intent points to an instant app.
*/
boolean isIntentToInstantApp(Intent intent);
/** /**
* @param packageName The package to check. * @param packageName The package to check.
* @return Whether the package is a valid WebAPK package. * @return Whether the package is a valid WebAPK package.
......
...@@ -587,6 +587,16 @@ public class ExternalNavigationDelegateImpl implements ExternalNavigationDelegat ...@@ -587,6 +587,16 @@ public class ExternalNavigationDelegateImpl implements ExternalNavigationDelegat
getSpecializedHandlersWithFilter(infos, null)); getSpecializedHandlersWithFilter(infos, null));
} }
@Override
public void maybeAdjustInstantAppExtras(Intent intent, boolean isIntentToInstantApp) {
if (isIntentToInstantApp) {
intent.putExtra(InstantAppsHandler.IS_GOOGLE_SEARCH_REFERRER, true);
} else {
// Make sure this extra is not sent unless we've done the verification.
intent.removeExtra(InstantAppsHandler.IS_GOOGLE_SEARCH_REFERRER);
}
}
@Override @Override
public void maybeSetPendingReferrer(Intent intent, String referrerUrl) { public void maybeSetPendingReferrer(Intent intent, String referrerUrl) {
IntentHandler.setPendingReferrer(intent, referrerUrl); IntentHandler.setPendingReferrer(intent, referrerUrl);
...@@ -679,6 +689,11 @@ public class ExternalNavigationDelegateImpl implements ExternalNavigationDelegat ...@@ -679,6 +689,11 @@ public class ExternalNavigationDelegateImpl implements ExternalNavigationDelegat
return false; return false;
} }
@Override
public boolean isIntentToInstantApp(Intent intent) {
return InstantAppsHandler.isIntentToInstantApp(intent);
}
@Override @Override
public boolean isValidWebApk(String packageName) { public boolean isValidWebApk(String packageName) {
return WebApkValidator.isValidWebApk(ContextUtils.getApplicationContext(), packageName); return WebApkValidator.isValidWebApk(ContextUtils.getApplicationContext(), packageName);
......
...@@ -27,7 +27,6 @@ import org.chromium.base.Log; ...@@ -27,7 +27,6 @@ import org.chromium.base.Log;
import org.chromium.base.metrics.RecordHistogram; import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.metrics.RecordUserAction; import org.chromium.base.metrics.RecordUserAction;
import org.chromium.chrome.browser.flags.ChromeFeatureList; import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.instantapps.InstantAppsHandler;
import org.chromium.chrome.browser.tab.TabRedirectHandler; import org.chromium.chrome.browser.tab.TabRedirectHandler;
import org.chromium.components.embedder_support.util.UrlConstants; import org.chromium.components.embedder_support.util.UrlConstants;
import org.chromium.components.embedder_support.util.UrlUtilities; import org.chromium.components.embedder_support.util.UrlUtilities;
...@@ -206,7 +205,7 @@ public class ExternalNavigationHandler { ...@@ -206,7 +205,7 @@ public class ExternalNavigationHandler {
private @OverrideUrlLoadingResult int handleFallbackUrl( private @OverrideUrlLoadingResult int handleFallbackUrl(
ExternalNavigationParams params, Intent targetIntent, String browserFallbackUrl) { ExternalNavigationParams params, Intent targetIntent, String browserFallbackUrl) {
if (InstantAppsHandler.isIntentToInstantApp(targetIntent)) { if (mDelegate.isIntentToInstantApp(targetIntent)) {
RecordHistogram.recordEnumeratedHistogram("Android.InstantApps.DirectInstantAppsIntent", RecordHistogram.recordEnumeratedHistogram("Android.InstantApps.DirectInstantAppsIntent",
AiaIntent.FALLBACK_USED, AiaIntent.NUM_ENTRIES); AiaIntent.FALLBACK_USED, AiaIntent.NUM_ENTRIES);
} }
...@@ -675,13 +674,11 @@ public class ExternalNavigationHandler { ...@@ -675,13 +674,11 @@ public class ExternalNavigationHandler {
if (params.isIncognito()) mDelegate.maybeSetPendingIncognitoUrl(targetIntent); if (params.isIncognito()) mDelegate.maybeSetPendingIncognitoUrl(targetIntent);
mDelegate.maybeAdjustInstantAppExtras(targetIntent, shouldProxyForInstantApps);
if (shouldProxyForInstantApps) { if (shouldProxyForInstantApps) {
RecordHistogram.recordEnumeratedHistogram("Android.InstantApps.DirectInstantAppsIntent", RecordHistogram.recordEnumeratedHistogram("Android.InstantApps.DirectInstantAppsIntent",
AiaIntent.SERP, AiaIntent.NUM_ENTRIES); AiaIntent.SERP, AiaIntent.NUM_ENTRIES);
targetIntent.putExtra(InstantAppsHandler.IS_GOOGLE_SEARCH_REFERRER, true);
} else {
// Make sure this extra is not sent unless we've done the verification.
targetIntent.removeExtra(InstantAppsHandler.IS_GOOGLE_SEARCH_REFERRER);
} }
// The intent can be used to launch Chrome itself, record the user // The intent can be used to launch Chrome itself, record the user
...@@ -883,7 +880,7 @@ public class ExternalNavigationHandler { ...@@ -883,7 +880,7 @@ public class ExternalNavigationHandler {
} }
boolean isDirectInstantAppsIntent = boolean isDirectInstantAppsIntent =
isExternalProtocol && InstantAppsHandler.isIntentToInstantApp(targetIntent); isExternalProtocol && mDelegate.isIntentToInstantApp(targetIntent);
boolean shouldProxyForInstantApps = isDirectInstantAppsIntent && mDelegate.isSerpReferrer(); boolean shouldProxyForInstantApps = isDirectInstantAppsIntent && mDelegate.isSerpReferrer();
if (preventDirectInstantAppsIntent(isDirectInstantAppsIntent, shouldProxyForInstantApps)) { if (preventDirectInstantAppsIntent(isDirectInstantAppsIntent, shouldProxyForInstantApps)) {
return OverrideUrlLoadingResult.NO_OVERRIDE; return OverrideUrlLoadingResult.NO_OVERRIDE;
......
...@@ -46,6 +46,9 @@ import java.util.List; ...@@ -46,6 +46,9 @@ import java.util.List;
+ "B.org.chromium.chrome.browser.autofill_assistant.ENABLED=true;" + "B.org.chromium.chrome.browser.autofill_assistant.ENABLED=true;"
+ "S." + ExternalNavigationHandler.EXTRA_BROWSER_FALLBACK_URL + "=" + "S." + ExternalNavigationHandler.EXTRA_BROWSER_FALLBACK_URL + "="
+ Uri.encode("https://www.example.com") + ";end"; + Uri.encode("https://www.example.com") + ";end";
private static final String[] SUPERVISOR_START_ACTIONS = {
"com.google.android.instantapps.START", "com.google.android.instantapps.nmr1.INSTALL",
"com.google.android.instantapps.nmr1.VIEW"};
class ExternalNavigationDelegateImplForTesting extends ExternalNavigationDelegateImpl { class ExternalNavigationDelegateImplForTesting extends ExternalNavigationDelegateImpl {
public ExternalNavigationDelegateImplForTesting() { public ExternalNavigationDelegateImplForTesting() {
...@@ -265,6 +268,71 @@ import java.util.List; ...@@ -265,6 +268,71 @@ import java.util.List;
Assert.assertEquals(url, IntentHandler.getPendingIncognitoUrl()); Assert.assertEquals(url, IntentHandler.getPendingIncognitoUrl());
} }
@Test
@SmallTest
public void testIsIntentToInstantApp() {
ExternalNavigationDelegateImpl delegate = new ExternalNavigationDelegateImpl(
mActivityTestRule.getActivity().getActivityTab());
// Check that the delegate correctly distinguishes instant app intents from others.
String vanillaUrl = "http://www.example.com";
Intent vanillaIntent = new Intent(Intent.ACTION_VIEW);
vanillaIntent.setData(Uri.parse(vanillaUrl));
String instantAppIntentUrl = "intent://buzzfeed.com/tasty#Intent;scheme=http;"
+ "package=com.google.android.instantapps.supervisor;"
+ "action=com.google.android.instantapps.START;"
+ "S.com.google.android.instantapps.FALLBACK_PACKAGE="
+ "com.android.chrome;S.com.google.android.instantapps.INSTANT_APP_PACKAGE="
+ "com.yelp.android;S.android.intent.extra.REFERRER_NAME="
+ "https%3A%2F%2Fwww.google.com;end";
Intent instantAppIntent;
try {
instantAppIntent = Intent.parseUri(instantAppIntentUrl, Intent.URI_INTENT_SCHEME);
} catch (Exception ex) {
Assert.assertTrue(false);
return;
}
Assert.assertFalse(delegate.isIntentToInstantApp(vanillaIntent));
Assert.assertTrue(delegate.isIntentToInstantApp(instantAppIntent));
// Check that Supervisor is detected by action even without package.
for (String action : SUPERVISOR_START_ACTIONS) {
String intentWithoutPackageUrl = "intent://buzzfeed.com/tasty#Intent;scheme=http;"
+ "action=" + action + ";"
+ "S.com.google.android.instantapps.FALLBACK_PACKAGE="
+ "com.android.chrome;S.com.google.android.instantapps.INSTANT_APP_PACKAGE="
+ "com.yelp.android;S.android.intent.extra.REFERRER_NAME="
+ "https%3A%2F%2Fwww.google.com;end";
try {
instantAppIntent =
Intent.parseUri(intentWithoutPackageUrl, Intent.URI_INTENT_SCHEME);
} catch (Exception ex) {
Assert.assertTrue(false);
return;
}
Assert.assertTrue(delegate.isIntentToInstantApp(instantAppIntent));
}
}
@Test
@SmallTest
public void testMaybeAdjustInstantAppExtras() {
ExternalNavigationDelegateImpl delegate = new ExternalNavigationDelegateImpl(
mActivityTestRule.getActivity().getActivityTab());
String url = "http://www.example.com";
Intent intent = new Intent(Intent.ACTION_VIEW);
intent.setData(Uri.parse(url));
delegate.maybeAdjustInstantAppExtras(intent, /*isIntentToInstantApp=*/true);
Assert.assertTrue(intent.hasExtra(InstantAppsHandler.IS_GOOGLE_SEARCH_REFERRER));
delegate.maybeAdjustInstantAppExtras(intent, /*isIntentToInstantApp=*/false);
Assert.assertFalse(intent.hasExtra(InstantAppsHandler.IS_GOOGLE_SEARCH_REFERRER));
}
@Test @Test
@SmallTest @SmallTest
public void testMaybeSetPendingReferrer() { public void testMaybeSetPendingReferrer() {
......
...@@ -37,7 +37,6 @@ import org.chromium.chrome.browser.ShortcutHelper; ...@@ -37,7 +37,6 @@ import org.chromium.chrome.browser.ShortcutHelper;
import org.chromium.chrome.browser.customtabs.CustomTabIntentDataProvider; import org.chromium.chrome.browser.customtabs.CustomTabIntentDataProvider;
import org.chromium.chrome.browser.externalnav.ExternalNavigationHandler.OverrideUrlLoadingResult; import org.chromium.chrome.browser.externalnav.ExternalNavigationHandler.OverrideUrlLoadingResult;
import org.chromium.chrome.browser.flags.ChromeFeatureList; import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.instantapps.InstantAppsHandler;
import org.chromium.chrome.browser.tab.TabRedirectHandler; import org.chromium.chrome.browser.tab.TabRedirectHandler;
import org.chromium.chrome.browser.webapps.WebappInfo; import org.chromium.chrome.browser.webapps.WebappInfo;
import org.chromium.chrome.browser.webapps.WebappScopePolicy; import org.chromium.chrome.browser.webapps.WebappScopePolicy;
...@@ -132,6 +131,8 @@ public class ExternalNavigationHandlerTest { ...@@ -132,6 +131,8 @@ public class ExternalNavigationHandlerTest {
+ "S." + ExternalNavigationHandler.EXTRA_BROWSER_FALLBACK_URL + "=" + "S." + ExternalNavigationHandler.EXTRA_BROWSER_FALLBACK_URL + "="
+ Uri.encode("https://www.example.com") + ";end"; + Uri.encode("https://www.example.com") + ";end";
private static final String IS_INSTANT_APP_EXTRA = "IS_INSTANT_APP";
private Context mContext; private Context mContext;
private final TestExternalNavigationDelegate mDelegate; private final TestExternalNavigationDelegate mDelegate;
private ExternalNavigationHandler mUrlHandler; private ExternalNavigationHandler mUrlHandler;
...@@ -744,8 +745,7 @@ public class ExternalNavigationHandlerTest { ...@@ -744,8 +745,7 @@ public class ExternalNavigationHandlerTest {
@Test @Test
@SmallTest @SmallTest
public void public void testHandlingOfInstantApps() {
testInstantAppsIntent_serpReferrer() {
String intentUrl = "intent://buzzfeed.com/tasty#Intent;scheme=http;" String intentUrl = "intent://buzzfeed.com/tasty#Intent;scheme=http;"
+ "package=com.google.android.instantapps.supervisor;" + "package=com.google.android.instantapps.supervisor;"
+ "action=com.google.android.instantapps.START;" + "action=com.google.android.instantapps.START;"
...@@ -753,43 +753,28 @@ public class ExternalNavigationHandlerTest { ...@@ -753,43 +753,28 @@ public class ExternalNavigationHandlerTest {
+ "com.android.chrome;S.com.google.android.instantapps.INSTANT_APP_PACKAGE=" + "com.android.chrome;S.com.google.android.instantapps.INSTANT_APP_PACKAGE="
+ "com.yelp.android;S.android.intent.extra.REFERRER_NAME=" + "com.yelp.android;S.android.intent.extra.REFERRER_NAME="
+ "https%3A%2F%2Fwww.google.com;end"; + "https%3A%2F%2Fwww.google.com;end";
mDelegate.setIsSerpReferrer(true); mDelegate.setIsSerpReferrer(true);
mDelegate.setIsIntentToInstantApp(true);
checkUrl(intentUrl) checkUrl(intentUrl)
.expecting(OverrideUrlLoadingResult.OVERRIDE_WITH_EXTERNAL_INTENT, .expecting(OverrideUrlLoadingResult.OVERRIDE_WITH_EXTERNAL_INTENT,
START_OTHER_ACTIVITY | PROXY_FOR_INSTANT_APPS); START_OTHER_ACTIVITY | PROXY_FOR_INSTANT_APPS);
Assert.assertTrue(mDelegate.startActivityIntent.hasExtra( Assert.assertTrue(
InstantAppsHandler.IS_GOOGLE_SEARCH_REFERRER)); mDelegate.startActivityIntent.getBooleanExtra(IS_INSTANT_APP_EXTRA, false));
// Check that we block all instant app intent:// URLs not from SERP // Check that we block all instant app intent:// URLs not from SERP.
mDelegate.setIsSerpReferrer(false); mDelegate.setIsSerpReferrer(false);
checkUrl(intentUrl) checkUrl(intentUrl)
.expecting(OverrideUrlLoadingResult.NO_OVERRIDE, IGNORE); .expecting(OverrideUrlLoadingResult.NO_OVERRIDE, IGNORE);
// Check that IS_GOOGLE_SEARCH_REFERRER param is stripped on non-supervisor intents. // Check that that just having the SERP referrer alone doesn't cause intents to be treated
// as intents to instant apps if the delegate indicates that they shouldn't be.
mDelegate.setIsSerpReferrer(true); mDelegate.setIsSerpReferrer(true);
String nonSupervisor = "intent://buzzfeed.com/tasty#Intent;scheme=http;" mDelegate.setIsIntentToInstantApp(false);
+ "package=com.imdb;action=com.google.VIEW;" checkUrl(intentUrl).expecting(
+ "S.com.google.android.gms.instantapps.IS_GOOGLE_SEARCH_REFERRER=" OverrideUrlLoadingResult.OVERRIDE_WITH_EXTERNAL_INTENT, START_OTHER_ACTIVITY);
+ "true;S.android.intent.extra.REFERRER_NAME=" Assert.assertFalse(
+ "https%3A%2F%2Fwww.google.com;end"; mDelegate.startActivityIntent.getBooleanExtra(IS_INSTANT_APP_EXTRA, true));
checkUrl(nonSupervisor)
.expecting(OverrideUrlLoadingResult.OVERRIDE_WITH_EXTERNAL_INTENT,
START_OTHER_ACTIVITY);
Assert.assertFalse(mDelegate.startActivityIntent.hasExtra(
InstantAppsHandler.IS_GOOGLE_SEARCH_REFERRER));
// Check that Supervisor is detected by action even without package
for (String action : SUPERVISOR_START_ACTIONS) {
String intentWithoutPackage = "intent://buzzfeed.com/tasty#Intent;scheme=http;"
+ "action=" + action + ";"
+ "S.com.google.android.instantapps.FALLBACK_PACKAGE="
+ "com.android.chrome;S.com.google.android.instantapps.INSTANT_APP_PACKAGE="
+ "com.yelp.android;S.android.intent.extra.REFERRER_NAME="
+ "https%3A%2F%2Fwww.google.com;end";
mDelegate.setIsSerpReferrer(false);
checkUrl(intentWithoutPackage)
.expecting(OverrideUrlLoadingResult.NO_OVERRIDE, IGNORE);
}
} }
@Test @Test
...@@ -1868,6 +1853,15 @@ public class ExternalNavigationHandlerTest { ...@@ -1868,6 +1853,15 @@ public class ExternalNavigationHandlerTest {
intent.putExtra(Intent.EXTRA_REFERRER, Uri.parse(referrerUrl)); intent.putExtra(Intent.EXTRA_REFERRER, Uri.parse(referrerUrl));
} }
@Override
public void maybeAdjustInstantAppExtras(Intent intent, boolean isIntentToInstantApp) {
if (isIntentToInstantApp) {
intent.putExtra(IS_INSTANT_APP_EXTRA, true);
} else {
intent.putExtra(IS_INSTANT_APP_EXTRA, false);
}
}
@Override @Override
public void maybeSetPendingIncognitoUrl(Intent intent) {} public void maybeSetPendingIncognitoUrl(Intent intent) {}
...@@ -1907,6 +1901,11 @@ public class ExternalNavigationHandlerTest { ...@@ -1907,6 +1901,11 @@ public class ExternalNavigationHandlerTest {
return mIsCallingAppTrusted; return mIsCallingAppTrusted;
} }
@Override
public boolean isIntentToInstantApp(Intent intent) {
return mIsIntentToInstantApp;
}
@Override @Override
public boolean isValidWebApk(String packageName) { public boolean isValidWebApk(String packageName) {
for (IntentActivity activity : mIntentActivities) { for (IntentActivity activity : mIntentActivities) {
...@@ -1982,6 +1981,10 @@ public class ExternalNavigationHandlerTest { ...@@ -1982,6 +1981,10 @@ public class ExternalNavigationHandlerTest {
mIsCallingAppTrusted = trusted; mIsCallingAppTrusted = trusted;
} }
public void setIsIntentToInstantApp(boolean value) {
mIsIntentToInstantApp = value;
}
public Intent startActivityIntent; public Intent startActivityIntent;
public boolean startIncognitoIntentCalled; public boolean startIncognitoIntentCalled;
public boolean startFileIntentCalled; public boolean startFileIntentCalled;
...@@ -2001,6 +2004,7 @@ public class ExternalNavigationHandlerTest { ...@@ -2001,6 +2004,7 @@ public class ExternalNavigationHandlerTest {
public boolean mCalledWithProxy; public boolean mCalledWithProxy;
public boolean mIsChromeAppInForeground = true; public boolean mIsChromeAppInForeground = true;
private boolean mIsCallingAppTrusted; private boolean mIsCallingAppTrusted;
private boolean mIsIntentToInstantApp;
public boolean shouldRequestFileAccess; public boolean shouldRequestFileAccess;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment