Commit 8ce966f6 authored by Colin Blundell's avatar Colin Blundell Committed by Commit Bot

[WebLayer] Use Activity instead of ApplicationContext to launch Intents

This CL implements the feedback from boliu@ that the Activity is more
suitable than the ApplicationContext for launching external Intents.
This change should also make it easier to add an integration test of
launching external Intents in WebLayer.

A couple notes on the implementation changes:
- I explored various ways to change the API but ended up just passing
  the Java Tab from content_browser_client_impl.cc to the static Java
  method that implements intent launching. Given the fact that the
  content_browser_client_impl.cc callsite needs to interact with the
  JNIEnv object after making the call, this seemed like the most
  straightforward change.
- It's debatable what action to take if the Activity is not available:
  bail out or use the ApplicationContext? In this CL I decided on the
  former; if we find any evidence that there are user flows for this
  use case we could change it to the latter.

Bug: 1031465, 1029710
Change-Id: I23f3006422d47818e3873678fa936b3350cd4b35
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2054864Reviewed-by: default avatarBo <boliu@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#741388}
parent 0e50d769
......@@ -540,7 +540,8 @@ bool ContentBrowserClientImpl::ShouldOverrideUrlLoading(
base::android::ConvertUTF16ToJavaString(env, url);
*ignore_navigation = Java_ExternalNavigationHandler_shouldOverrideUrlLoading(
env, jurl, has_user_gesture, is_redirect, is_main_frame);
env, TabImpl::FromWebContents(web_contents)->GetJavaTab(), jurl,
has_user_gesture, is_redirect, is_main_frame);
if (base::android::HasException(env)) {
// Tell the chromium message loop to not perform any tasks after the
......
......@@ -9,7 +9,6 @@ import android.content.Context;
import android.content.Intent;
import android.provider.Browser;
import org.chromium.base.ContextUtils;
import org.chromium.base.Log;
import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.JNINamespace;
......@@ -35,8 +34,8 @@ public class ExternalNavigationHandler {
+ ".*");
@CalledByNative
private static boolean shouldOverrideUrlLoading(
String url, boolean hasUserGesture, boolean isRedirect, boolean isMainFrame) {
private static boolean shouldOverrideUrlLoading(TabImpl tab, String url, boolean hasUserGesture,
boolean isRedirect, boolean isMainFrame) {
// Check for regular URIs that WebLayer supports by itself.
// TODO(blundell): Port over WebViewBrowserActivity's
// isSpecializedHandlerAvailable() check that checks whether there's an app for handling
......@@ -75,7 +74,11 @@ public class ExternalNavigationHandler {
selector.setComponent(null);
}
Context context = ContextUtils.getApplicationContext();
Context context = tab.getBrowser().getContext();
if (context == null) {
return false;
}
// Pass the package name as application ID so that the intent from the
// same application can be opened in the same tab.
......
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