Commit d60faf20 authored by Michael Thiessen's avatar Michael Thiessen Committed by Commit Bot

Make ShouldOverrideUrlLoadingInternal more readable (3 of N)

No functional changes intended, please revert if things break.

This function is a large, and hard to follow. I'm hoping to break out a
bunch of functions that are clearer about what they're doing and make
ordering clearer.

Bug: 1006927
Change-Id: I5c0efdb274222b1af861acf7cd31dade9a3ab2a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1822509
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: default avatarYaron Friedman <yfriedman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699892}
parent 40d10010
...@@ -344,6 +344,73 @@ public class ExternalNavigationHandler { ...@@ -344,6 +344,73 @@ public class ExternalNavigationHandler {
return true; return true;
} }
// http://crbug.com/159153: Don't override navigation from a chrome:* url to http or https.
// For example when clicking a link in bookmarks or most visited. When navigating from such
// a page, there is clear intent to complete the navigation in Chrome.
private boolean isLinkFromChromeInternalPage(ExternalNavigationParams params) {
if (params.getReferrerUrl() == null) return false;
if (params.getReferrerUrl().startsWith(UrlConstants.CHROME_URL_PREFIX)
&& (params.getUrl().startsWith(UrlConstants.HTTP_URL_PREFIX)
|| params.getUrl().startsWith(UrlConstants.HTTPS_URL_PREFIX))) {
if (DEBUG) Log.i(TAG, "NO_OVERRIDE: Link from an internal chrome:// page");
return true;
}
return false;
}
private boolean handleWtaiMcProtocol(ExternalNavigationParams params) {
if (!params.getUrl().startsWith(WTAI_MC_URL_PREFIX)) return false;
// wtai://wp/mc;number
// number=string(phone-number)
mDelegate.startActivity(
new Intent(Intent.ACTION_VIEW,
Uri.parse(WebView.SCHEME_TEL
+ params.getUrl().substring(WTAI_MC_URL_PREFIX.length()))),
false);
if (DEBUG) Log.i(TAG, "OVERRIDE_WITH_EXTERNAL_INTENT wtai:// link handled");
RecordUserAction.record("Android.PhoneIntent");
return true;
}
private boolean isUnhandledWtaiProtocol(ExternalNavigationParams params) {
if (!params.getUrl().startsWith(WTAI_URL_PREFIX)) return false;
if (DEBUG) Log.i(TAG, "NO_OVERRIDE: Unsupported wtai:// link");
return true;
}
// The "about:", "chrome:", "chrome-native:", "chrome-devtools:", and "devtools:" schemes
// are internal to the browser; don't want these to be dispatched to other apps.
private boolean hasInternalScheme(ExternalNavigationParams params) {
if (params.getUrl().startsWith(ContentUrlConstants.ABOUT_URL_SHORT_PREFIX)
|| params.getUrl().startsWith(UrlConstants.CHROME_URL_SHORT_PREFIX)
|| params.getUrl().startsWith(UrlConstants.CHROME_NATIVE_URL_SHORT_PREFIX)
|| params.getUrl().startsWith(UrlConstants.DEVTOOLS_URL_SHORT_PREFIX)
|| params.getUrl().startsWith(UrlConstants.DEVTOOLS_FALLBACK_URL_SHORT_PREFIX)) {
if (DEBUG) Log.i(TAG, "NO_OVERRIDE: Navigating to a chrome-internal page");
return true;
}
return false;
}
// The "content:" scheme is disabled in Clank. Do not try to start an activity.
private boolean hasContentScheme(ExternalNavigationParams params) {
if (!params.getUrl().startsWith(UrlConstants.CONTENT_URL_SHORT_PREFIX)) return false;
if (DEBUG) Log.i(TAG, "NO_OVERRIDE: Navigation to content: URL");
return true;
}
// Special case - It makes no sense to use an external application for a YouTube
// pairing code URL, since these match the current tab with a device (Chromecast
// or similar) it is supposed to be controlling. Using a different application
// that isn't expecting this (in particular YouTube) doesn't work.
private boolean isYoutubePairingCode(ExternalNavigationParams params) {
if (params.getUrl().matches(".*youtube\\.com(\\/.*)?\\?(.+&)?pairingCode=[^&].+")) {
if (DEBUG) Log.i(TAG, "NO_OVERRIDE: YouTube URL with a pairing code");
return true;
}
return false;
}
private @OverrideUrlLoadingResult int shouldOverrideUrlLoadingInternal( private @OverrideUrlLoadingResult int shouldOverrideUrlLoadingInternal(
ExternalNavigationParams params, Intent intent, boolean hasBrowserFallbackUrl, ExternalNavigationParams params, Intent intent, boolean hasBrowserFallbackUrl,
String browserFallbackUrl) { String browserFallbackUrl) {
...@@ -393,59 +460,19 @@ public class ExternalNavigationHandler { ...@@ -393,59 +460,19 @@ public class ExternalNavigationHandler {
return OverrideUrlLoadingResult.NO_OVERRIDE; return OverrideUrlLoadingResult.NO_OVERRIDE;
} }
// http://crbug.com/159153: Don't override navigation from a chrome:* url to http or https. if (isLinkFromChromeInternalPage(params)) return OverrideUrlLoadingResult.NO_OVERRIDE;
// For example when clicking a link in bookmarks or most visited. When navigating from such
// a page, there is clear intent to complete the navigation in Chrome.
if (params.getReferrerUrl() != null
&& params.getReferrerUrl().startsWith(UrlConstants.CHROME_URL_PREFIX)
&& (params.getUrl().startsWith(UrlConstants.HTTP_URL_PREFIX)
|| params.getUrl().startsWith(UrlConstants.HTTPS_URL_PREFIX))) {
if (DEBUG) Log.i(TAG, "NO_OVERRIDE: Link from an internal chrome:// page");
return OverrideUrlLoadingResult.NO_OVERRIDE;
}
if (params.getUrl().startsWith(WTAI_MC_URL_PREFIX)) { if (handleWtaiMcProtocol(params)) {
// wtai://wp/mc;number
// number=string(phone-number)
mDelegate.startActivity(new Intent(Intent.ACTION_VIEW,
Uri.parse(WebView.SCHEME_TEL
+ params.getUrl().substring(WTAI_MC_URL_PREFIX.length()))), false);
if (DEBUG) Log.i(TAG, "OVERRIDE_WITH_EXTERNAL_INTENT wtai:// link handled");
RecordUserAction.record("Android.PhoneIntent");
return OverrideUrlLoadingResult.OVERRIDE_WITH_EXTERNAL_INTENT; return OverrideUrlLoadingResult.OVERRIDE_WITH_EXTERNAL_INTENT;
} }
// TODO: handle other WTAI schemes.
if (isUnhandledWtaiProtocol(params)) return OverrideUrlLoadingResult.NO_OVERRIDE;
if (params.getUrl().startsWith(WTAI_URL_PREFIX)) { if (hasInternalScheme(params)) return OverrideUrlLoadingResult.NO_OVERRIDE;
// TODO: handle other WTAI schemes.
if (DEBUG) Log.i(TAG, "NO_OVERRIDE: Unsupported wtai:// link");
return OverrideUrlLoadingResult.NO_OVERRIDE;
}
// The "about:", "chrome:", "chrome-native:", "chrome-devtools:", and "devtools:" schemes if (hasContentScheme(params)) return OverrideUrlLoadingResult.NO_OVERRIDE;
// are internal to the browser; don't want these to be dispatched to other apps.
if (params.getUrl().startsWith(ContentUrlConstants.ABOUT_URL_SHORT_PREFIX)
|| params.getUrl().startsWith(UrlConstants.CHROME_URL_SHORT_PREFIX)
|| params.getUrl().startsWith(UrlConstants.CHROME_NATIVE_URL_SHORT_PREFIX)
|| params.getUrl().startsWith(UrlConstants.DEVTOOLS_URL_SHORT_PREFIX)
|| params.getUrl().startsWith(UrlConstants.DEVTOOLS_FALLBACK_URL_SHORT_PREFIX)) {
if (DEBUG) Log.i(TAG, "NO_OVERRIDE: Navigating to a chrome-internal page");
return OverrideUrlLoadingResult.NO_OVERRIDE;
}
// The "content:" scheme is disabled in Clank. Do not try to start an activity. if (isYoutubePairingCode(params)) return OverrideUrlLoadingResult.NO_OVERRIDE;
if (params.getUrl().startsWith(UrlConstants.CONTENT_URL_SHORT_PREFIX)) {
if (DEBUG) Log.i(TAG, "NO_OVERRIDE: Navigation to content: URL");
return OverrideUrlLoadingResult.NO_OVERRIDE;
}
// Special case - It makes no sense to use an external application for a YouTube
// pairing code URL, since these match the current tab with a device (Chromecast
// or similar) it is supposed to be controlling. Using a different application
// that isn't expecting this (in particular YouTube) doesn't work.
if (params.getUrl().matches(".*youtube\\.com(\\/.*)?\\?(.+&)?pairingCode=[^&].+")) {
if (DEBUG) Log.i(TAG, "NO_OVERRIDE: YouTube URL with a pairing code");
return OverrideUrlLoadingResult.NO_OVERRIDE;
}
// TODO(changwan): check if we need to handle URL even when external intent is off. // TODO(changwan): check if we need to handle URL even when external intent is off.
if (CommandLine.getInstance().hasSwitch( if (CommandLine.getInstance().hasSwitch(
......
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