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

Make ShouldOverrideUrlLoadingInternal more readable (9 of N)

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.

In this change I do fix an exception handler to actually catch the
exception it was intending to catch (and you can easily repro the crash
by disabling the targeted Activity while the leaving Chrome alert dialog
is up).

I also make sure that when we fail to show the alert dialog for reasons
other than a BadTokenException, we also propagate this back so we don't
get into a state where we report an async action action is taking place
when it isn't.

Bug: 1006927
Change-Id: Ie09214f64ead85c31fb8e83e187878d984f2afc2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1835144Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702483}
parent ed15802b
...@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.externalnav; ...@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.externalnav;
import android.Manifest.permission; import android.Manifest.permission;
import android.app.Activity; import android.app.Activity;
import android.content.ActivityNotFoundException;
import android.content.Context; import android.content.Context;
import android.content.DialogInterface; import android.content.DialogInterface;
import android.content.DialogInterface.OnCancelListener; import android.content.DialogInterface.OnCancelListener;
...@@ -365,18 +366,18 @@ public class ExternalNavigationDelegateImpl implements ExternalNavigationDelegat ...@@ -365,18 +366,18 @@ public class ExternalNavigationDelegateImpl implements ExternalNavigationDelegat
final String fallbackUrl, final Tab tab, final boolean needsToCloseTab, final String fallbackUrl, final Tab tab, final boolean needsToCloseTab,
final boolean proxy) { final boolean proxy) {
try { try {
startIncognitoIntentInternal(intent, referrerUrl, fallbackUrl, needsToCloseTab, proxy); return startIncognitoIntentInternal(
intent, referrerUrl, fallbackUrl, needsToCloseTab, proxy);
} catch (BadTokenException e) { } catch (BadTokenException e) {
return false; return false;
} }
return true;
} }
private void startIncognitoIntentInternal(final Intent intent, final String referrerUrl, private boolean startIncognitoIntentInternal(final Intent intent, final String referrerUrl,
final String fallbackUrl, final boolean needsToCloseTab, final boolean proxy) { final String fallbackUrl, final boolean needsToCloseTab, final boolean proxy) {
if (!hasValidTab()) return; if (!hasValidTab()) return false;
Context context = mTab.getWindowAndroid().getContext().get(); Context context = mTab.getWindowAndroid().getContext().get();
if (!(context instanceof Activity)) return; if (!(context instanceof Activity)) return false;
Activity activity = (Activity) context; Activity activity = (Activity) context;
new UiUtils.CompatibleAlertDialogBuilder(activity, R.style.Theme_Chromium_AlertDialog) new UiUtils.CompatibleAlertDialogBuilder(activity, R.style.Theme_Chromium_AlertDialog)
...@@ -386,10 +387,18 @@ public class ExternalNavigationDelegateImpl implements ExternalNavigationDelegat ...@@ -386,10 +387,18 @@ public class ExternalNavigationDelegateImpl implements ExternalNavigationDelegat
new OnClickListener() { new OnClickListener() {
@Override @Override
public void onClick(DialogInterface dialog, int which) { public void onClick(DialogInterface dialog, int which) {
startActivity(intent, proxy); try {
if (mTab != null && !mTab.isClosing() && mTab.isInitialized() startActivity(intent, proxy);
&& needsToCloseTab) { if (mTab != null && !mTab.isClosing() && mTab.isInitialized()
closeTab(); && needsToCloseTab) {
closeTab();
}
} catch (ActivityNotFoundException e) {
// The activity that we thought was going to handle the intent
// no longer exists, so catch the exception and assume Chrome
// can handle it.
loadIntent(intent, referrerUrl, fallbackUrl, mTab,
needsToCloseTab, true);
} }
} }
}) })
...@@ -408,6 +417,7 @@ public class ExternalNavigationDelegateImpl implements ExternalNavigationDelegat ...@@ -408,6 +417,7 @@ public class ExternalNavigationDelegateImpl implements ExternalNavigationDelegat
} }
}) })
.show(); .show();
return true;
} }
@Override @Override
......
...@@ -682,6 +682,22 @@ public class ExternalNavigationHandler { ...@@ -682,6 +682,22 @@ public class ExternalNavigationHandler {
} }
} }
private @OverrideUrlLoadingResult int handleExternalIncognitoIntent(Intent targetIntent,
ExternalNavigationParams params, String browserFallbackUrl,
boolean shouldProxyForInstantApps) {
// This intent may leave Chrome. Warn the user that incognito does not carry over
// to apps out side of Chrome.
if (mDelegate.startIncognitoIntent(targetIntent, params.getReferrerUrl(),
browserFallbackUrl, params.getTab(),
params.shouldCloseContentsOnOverrideUrlLoadingAndLaunchIntent(),
shouldProxyForInstantApps)) {
if (DEBUG) Log.i(TAG, "Incognito navigation out");
return OverrideUrlLoadingResult.OVERRIDE_WITH_ASYNC_ACTION;
}
if (DEBUG) Log.i(TAG, "Failed to show incognito alert dialog.");
return OverrideUrlLoadingResult.NO_OVERRIDE;
}
private @OverrideUrlLoadingResult int shouldOverrideUrlLoadingInternal( private @OverrideUrlLoadingResult int shouldOverrideUrlLoadingInternal(
ExternalNavigationParams params, Intent targetIntent, ExternalNavigationParams params, Intent targetIntent,
@Nullable String browserFallbackUrl) { @Nullable String browserFallbackUrl) {
...@@ -794,24 +810,8 @@ public class ExternalNavigationHandler { ...@@ -794,24 +810,8 @@ public class ExternalNavigationHandler {
assert intentResolutionMatches(debugIntent, targetIntent); assert intentResolutionMatches(debugIntent, targetIntent);
if (params.isIncognito() && !mDelegate.willChromeHandleIntent(targetIntent)) { if (params.isIncognito() && !mDelegate.willChromeHandleIntent(targetIntent)) {
// This intent may leave Chrome. Warn the user that incognito does not carry over return handleExternalIncognitoIntent(
// to apps out side of Chrome. targetIntent, params, browserFallbackUrl, shouldProxyForInstantApps);
try {
if (!mDelegate.startIncognitoIntent(targetIntent, params.getReferrerUrl(),
browserFallbackUrl, params.getTab(),
params.shouldCloseContentsOnOverrideUrlLoadingAndLaunchIntent(),
shouldProxyForInstantApps)) {
if (DEBUG) Log.i(TAG, "Failed to show incognito alert dialog.");
return OverrideUrlLoadingResult.NO_OVERRIDE;
}
} catch (ActivityNotFoundException e) {
// The activity that we thought was going to handle the intent no longer exists,
// so catch the exception and assume Chrome can handle it.
Log.i(TAG, "Not showing alert dialog with no handler for intent");
return OverrideUrlLoadingResult.NO_OVERRIDE;
}
if (DEBUG) Log.i(TAG, "Incognito navigation out");
return OverrideUrlLoadingResult.OVERRIDE_WITH_ASYNC_ACTION;
} }
// Some third-party app launched Chrome with an intent, and the URL got redirected. The // Some third-party app launched Chrome with an intent, and the URL got redirected. The
......
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