Commit 1861d485 authored by Troy Hildebrandt's avatar Troy Hildebrandt Committed by Commit Bot

Remove unnecessary Tab params from ExternalNavHandler(Delegate).

The ExternalNavigationHandlerDelegateImpl gets passed a Tab as a
parameter when we create an ExternalNavigationHandler, and this
doesn't change for its lifespan. The Tab in question is passed in from
the call to Tab#attach, which creates a brand new
InterceptNavigationDelegate which in turn ends up creating the
ExternalNavigationHandlerDelegate.

Bug: 651205
Change-Id: Iba4cfb154c5374ff931b35211246975f9cdd7892
Reviewed-on: https://chromium-review.googlesource.com/952274Reviewed-by: default avatarMaria Khomenko <mariakhomenko@chromium.org>
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541445}
parent 67018e63
...@@ -88,10 +88,9 @@ interface ExternalNavigationDelegate { ...@@ -88,10 +88,9 @@ interface ExternalNavigationDelegate {
/** /**
* @param url The requested url. * @param url The requested url.
* @param tab The current tab.
* @return Whether we should block the navigation and request file access before proceeding. * @return Whether we should block the navigation and request file access before proceeding.
*/ */
boolean shouldRequestFileAccess(String url, Tab tab); boolean shouldRequestFileAccess(String url);
/** /**
* Trigger a UI affordance that will ask the user to grant file access. After the access * Trigger a UI affordance that will ask the user to grant file access. After the access
...@@ -99,10 +98,9 @@ interface ExternalNavigationDelegate { ...@@ -99,10 +98,9 @@ interface ExternalNavigationDelegate {
* *
* @param intent The intent to continue loading the file URL. * @param intent The intent to continue loading the file URL.
* @param referrerUrl The HTTP referrer URL. * @param referrerUrl The HTTP referrer URL.
* @param tab The current tab.
* @param needsToCloseTab Whether this action should close the current tab. * @param needsToCloseTab Whether this action should close the current tab.
*/ */
void startFileIntent(Intent intent, String referrerUrl, Tab tab, boolean needsToCloseTab); void startFileIntent(Intent intent, String referrerUrl, boolean needsToCloseTab);
/** /**
* Launches a Chrome Custom Tab to be shown on top of a WebappActivity. * Launches a Chrome Custom Tab to be shown on top of a WebappActivity.
...@@ -117,11 +115,10 @@ interface ExternalNavigationDelegate { ...@@ -117,11 +115,10 @@ interface ExternalNavigationDelegate {
* *
* @param url The new URL after clobbering the current tab. * @param url The new URL after clobbering the current tab.
* @param referrerUrl The HTTP referrer URL. * @param referrerUrl The HTTP referrer URL.
* @param tab The current tab.
* @return OverrideUrlLoadingResult (if the tab has been clobbered, or we're launching an * @return OverrideUrlLoadingResult (if the tab has been clobbered, or we're launching an
* intent.) * intent.)
*/ */
OverrideUrlLoadingResult clobberCurrentTab(String url, String referrerUrl, Tab tab); OverrideUrlLoadingResult clobberCurrentTab(String url, String referrerUrl);
/** Adds a window id to the intent, if necessary. */ /** Adds a window id to the intent, if necessary. */
void maybeSetWindowId(Intent intent); void maybeSetWindowId(Intent intent);
...@@ -147,20 +144,17 @@ interface ExternalNavigationDelegate { ...@@ -147,20 +144,17 @@ interface ExternalNavigationDelegate {
/** /**
* Check if the URL should be handled by an instant app, or kick off an async request for an * Check if the URL should be handled by an instant app, or kick off an async request for an
* instant app banner. * instant app banner.
* @param tab The current tab.
* @param url The current URL. * @param url The current URL.
* @param referrerUrl The referrer URL. * @param referrerUrl The referrer URL.
* @param isIncomingRedirect Whether we are handling an incoming redirect to an instant app. * @param isIncomingRedirect Whether we are handling an incoming redirect to an instant app.
* @return Whether we launched an instant app. * @return Whether we launched an instant app.
*/ */
boolean maybeLaunchInstantApp(Tab tab, String url, String referrerUrl, boolean maybeLaunchInstantApp(String url, String referrerUrl, boolean isIncomingRedirect);
boolean isIncomingRedirect);
/** /**
* @param tab The current tab.
* @return whether this navigation is from the search results page. * @return whether this navigation is from the search results page.
*/ */
boolean isSerpReferrer(Tab tab); boolean isSerpReferrer();
/** /**
* @return The previously committed URL from the WebContents. * @return The previously committed URL from the WebContents.
......
...@@ -230,9 +230,8 @@ public class ExternalNavigationHandler { ...@@ -230,9 +230,8 @@ public class ExternalNavigationHandler {
// to Chrome. This check should happen for reloads, navigations, etc..., which is why // to Chrome. This check should happen for reloads, navigations, etc..., which is why
// it occurs before the subsequent blocks. // it occurs before the subsequent blocks.
if (params.getUrl().startsWith(UrlConstants.FILE_URL_SHORT_PREFIX) if (params.getUrl().startsWith(UrlConstants.FILE_URL_SHORT_PREFIX)
&& mDelegate.shouldRequestFileAccess(params.getUrl(), params.getTab())) { && mDelegate.shouldRequestFileAccess(params.getUrl())) {
mDelegate.startFileIntent( mDelegate.startFileIntent(intent, params.getReferrerUrl(),
intent, params.getReferrerUrl(), params.getTab(),
params.shouldCloseContentsOnOverrideUrlLoadingAndLaunchIntent()); params.shouldCloseContentsOnOverrideUrlLoadingAndLaunchIntent());
if (DEBUG) Log.i(TAG, "OVERRIDE_WITH_ASYNC_ACTION: Requesting filesystem access"); if (DEBUG) Log.i(TAG, "OVERRIDE_WITH_ASYNC_ACTION: Requesting filesystem access");
return OverrideUrlLoadingResult.OVERRIDE_WITH_ASYNC_ACTION; return OverrideUrlLoadingResult.OVERRIDE_WITH_ASYNC_ACTION;
...@@ -259,12 +258,10 @@ public class ExternalNavigationHandler { ...@@ -259,12 +258,10 @@ public class ExternalNavigationHandler {
if (handler.shouldStayInChrome(isExternalProtocol) if (handler.shouldStayInChrome(isExternalProtocol)
|| handler.shouldNotOverrideUrlLoading()) { || handler.shouldNotOverrideUrlLoading()) {
// http://crbug.com/659301: Handle redirects to Instant Apps out of Custom Tabs. // http://crbug.com/659301: Handle redirects to Instant Apps out of Custom Tabs.
if (handler.isFromCustomTabIntent() if (handler.isFromCustomTabIntent() && !isExternalProtocol && incomingIntentRedirect
&& !isExternalProtocol
&& incomingIntentRedirect
&& !handler.shouldNavigationTypeStayInChrome() && !handler.shouldNavigationTypeStayInChrome()
&& mDelegate.maybeLaunchInstantApp(params.getTab(), params.getUrl(), && mDelegate.maybeLaunchInstantApp(
params.getReferrerUrl(), true)) { params.getUrl(), params.getReferrerUrl(), true)) {
if (DEBUG) { if (DEBUG) {
Log.i(TAG, "OVERRIDE_WITH_EXTERNAL_INTENT: Launching redirect to " Log.i(TAG, "OVERRIDE_WITH_EXTERNAL_INTENT: Launching redirect to "
+ "an instant app"); + "an instant app");
...@@ -439,13 +436,14 @@ public class ExternalNavigationHandler { ...@@ -439,13 +436,14 @@ public class ExternalNavigationHandler {
// startActivityIfNeeded or startActivity. // startActivityIfNeeded or startActivity.
if (!isExternalProtocol) { if (!isExternalProtocol) {
if (mDelegate.countSpecializedHandlers(resolvingInfos) == 0) { if (mDelegate.countSpecializedHandlers(resolvingInfos) == 0) {
if (incomingIntentRedirect && mDelegate.maybeLaunchInstantApp( if (incomingIntentRedirect
params.getTab(), params.getUrl(), params.getReferrerUrl(), true)) { && mDelegate.maybeLaunchInstantApp(
params.getUrl(), params.getReferrerUrl(), true)) {
if (DEBUG) Log.i(TAG, "OVERRIDE_WITH_EXTERNAL_INTENT: Instant Apps redirect"); if (DEBUG) Log.i(TAG, "OVERRIDE_WITH_EXTERNAL_INTENT: Instant Apps redirect");
return OverrideUrlLoadingResult.OVERRIDE_WITH_EXTERNAL_INTENT; return OverrideUrlLoadingResult.OVERRIDE_WITH_EXTERNAL_INTENT;
} else if (linkNotFromIntent && !params.isIncognito() } else if (linkNotFromIntent && !params.isIncognito()
&& mDelegate.maybeLaunchInstantApp(params.getTab(), params.getUrl(), && mDelegate.maybeLaunchInstantApp(
params.getReferrerUrl(), false)) { params.getUrl(), params.getReferrerUrl(), false)) {
if (DEBUG) Log.i(TAG, "OVERRIDE_WITH_EXTERNAL_INTENT: Instant Apps link"); if (DEBUG) Log.i(TAG, "OVERRIDE_WITH_EXTERNAL_INTENT: Instant Apps link");
return OverrideUrlLoadingResult.OVERRIDE_WITH_EXTERNAL_INTENT; return OverrideUrlLoadingResult.OVERRIDE_WITH_EXTERNAL_INTENT;
} }
...@@ -511,8 +509,7 @@ public class ExternalNavigationHandler { ...@@ -511,8 +509,7 @@ public class ExternalNavigationHandler {
boolean isDirectInstantAppsIntent = boolean isDirectInstantAppsIntent =
isExternalProtocol && InstantAppsHandler.isIntentToInstantApp(intent); isExternalProtocol && InstantAppsHandler.isIntentToInstantApp(intent);
boolean shouldProxyForInstantApps = isDirectInstantAppsIntent boolean shouldProxyForInstantApps = isDirectInstantAppsIntent && mDelegate.isSerpReferrer();
&& mDelegate.isSerpReferrer(params.getTab());
if (shouldProxyForInstantApps) { if (shouldProxyForInstantApps) {
RecordHistogram.recordEnumeratedHistogram("Android.InstantApps.DirectInstantAppsIntent", RecordHistogram.recordEnumeratedHistogram("Android.InstantApps.DirectInstantAppsIntent",
AIA_INTENT_SERP, AIA_INTENT_BOUNDARY); AIA_INTENT_SERP, AIA_INTENT_BOUNDARY);
...@@ -674,8 +671,7 @@ public class ExternalNavigationHandler { ...@@ -674,8 +671,7 @@ public class ExternalNavigationHandler {
params.getRedirectHandler().setShouldNotOverrideUrlLoadingUntilNewUrlLoading(); params.getRedirectHandler().setShouldNotOverrideUrlLoadingUntilNewUrlLoading();
} }
if (DEBUG) Log.i(TAG, "OVERRIDE: clobberCurrentTab called"); if (DEBUG) Log.i(TAG, "OVERRIDE: clobberCurrentTab called");
return mDelegate.clobberCurrentTab( return mDelegate.clobberCurrentTab(browserFallbackUrl, params.getReferrerUrl());
browserFallbackUrl, params.getReferrerUrl(), params.getTab());
} }
/** /**
......
...@@ -1561,13 +1561,12 @@ public class ExternalNavigationHandlerTest { ...@@ -1561,13 +1561,12 @@ public class ExternalNavigationHandlerTest {
} }
@Override @Override
public boolean shouldRequestFileAccess(String url, Tab tab) { public boolean shouldRequestFileAccess(String url) {
return shouldRequestFileAccess; return shouldRequestFileAccess;
} }
@Override @Override
public void startFileIntent(Intent intent, String referrerUrl, Tab tab, public void startFileIntent(Intent intent, String referrerUrl, boolean needsToCloseTab) {
boolean needsToCloseTab) {
startFileIntentCalled = true; startFileIntentCalled = true;
} }
...@@ -1577,8 +1576,7 @@ public class ExternalNavigationHandlerTest { ...@@ -1577,8 +1576,7 @@ public class ExternalNavigationHandlerTest {
} }
@Override @Override
public OverrideUrlLoadingResult clobberCurrentTab( public OverrideUrlLoadingResult clobberCurrentTab(String url, String referrerUrl) {
String url, String referrerUrl, Tab tab) {
mNewUrlAfterClobbering = url; mNewUrlAfterClobbering = url;
mReferrerUrlForClobbering = referrerUrl; mReferrerUrlForClobbering = referrerUrl;
return OverrideUrlLoadingResult.OVERRIDE_WITH_CLOBBERING_TAB; return OverrideUrlLoadingResult.OVERRIDE_WITH_CLOBBERING_TAB;
...@@ -1608,13 +1606,13 @@ public class ExternalNavigationHandlerTest { ...@@ -1608,13 +1606,13 @@ public class ExternalNavigationHandlerTest {
} }
@Override @Override
public boolean maybeLaunchInstantApp(Tab tab, String url, String referrerUrl, public boolean maybeLaunchInstantApp(
boolean isIncomingRedirect) { String url, String referrerUrl, boolean isIncomingRedirect) {
return mCanHandleWithInstantApp; return mCanHandleWithInstantApp;
} }
@Override @Override
public boolean isSerpReferrer(Tab tab) { public boolean isSerpReferrer() {
return mIsSerpReferrer; return mIsSerpReferrer;
} }
......
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