[WebLayer] Check for intent launching via NavigationThrottle

The current mechanism via which WebLayer intercepts navigations in order
to handle launching of external intents is based on WebView's. It turns
out that this mechanism
(ContentBrowserClient::ShouldOverrideUrlLoading()) was added only due to
specific complexities in WebView. It is not the mechanism used in
Chrome, and as WebLayer does not inherit WebView's complexities here, it
is desirable to instead use Chrome's model for intercepting these
navigations.

This CL changes WebLayer to follow that model, which is a
NavigationThrottle-based approach. As part of this change, we also
adapt //weblayer's ExternalNavigationHandler.java where necessary to more
closely follow the //components-level that //chrome uses. The current
WebLayer logic for determining when to override the navigation (which was
also adapted from WebView) is preserved with two exceptions:

1. The current short-circut out if the navigation is not a redirect and
   is either browser-initiated or a back-forward navigation is changed
   to be a short-circuit out if the navigation is a back-forward
   navigation. The reason is that this simplified check follows what
   //chrome does; nowhere in //chrome's InterceptNavigationDelegateImpl
   or ExternalNavigationHandler does it check anything about the
   navigation being browser-initiated, and its ExternalNavigationHandler
   short-circuit on back-forward navigations does not check whether the
   navigation is a redirect.
2. There is no longer any check against the request method. Again, in
   //chrome's navigation interception there is also no such check.

These minor changes were made in light of the overall goal of moving
WebLayer's logic for intent launching to follow Chrome's; WebLayer will
*share* this logic (including these checks) once it is using the
//components-level ExternalNavigationHandler rather than the current
custom one. This CL also will enable WebLayer to easily adapt
(and potentially share) logic in //chrome's
InterceptNavigationDelegateImpl.java in followup work.

I verified manually that basic intenting out of WebLayer works after
this change.

Change-Id: I2ca54a802430496f9b9852e7a024eb979c49c93b
Bug: 1031465
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2102652Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Tobias Sargeant <tobiasjs@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#750969}

content/public/browser/content_browser_client.h

@@ -1493,7 +1493,7 @@ class CONTENT_EXPORT ContentBrowserClient {
   virtual std::vector<base::FilePath> GetNetworkContextsParentDirectory();
 
 #if defined(OS_ANDROID)
-  // Only used by Android WebView/WebLayer.
+  // Only used by Android WebView.
   // Returns:
   //   true  - The check was successfully performed without throwing a
   //           Java exception. |*ignore_navigation| is set to the

weblayer/BUILD.gn

@@ -324,6 +324,7 @@ source_set("weblayer_lib_base") {
       "browser/error_page_callback_proxy.h",
       "browser/fullscreen_callback_proxy.cc",
       "browser/fullscreen_callback_proxy.h",
+      "browser/intercept_navigation_delegate_impl.cc",
       "browser/new_tab_callback_proxy.cc",
       "browser/new_tab_callback_proxy.h",
       "browser/tab_callback_proxy.cc",
@@ -352,6 +353,7 @@ source_set("weblayer_lib_base") {
       "//components/javascript_dialogs",
       "//components/metrics",
       "//components/minidump_uploader",
+      "//components/navigation_interception",
       "//components/permissions/android:native",
       "//components/safe_browsing/content/common:interfaces",
       "//components/safe_browsing/content/renderer:throttles",

weblayer/browser/DEPS

@@ -15,6 +15,7 @@ include_rules = [
   "+components/javascript_dialogs",
   "+components/keyed_service/content",
   "+components/metrics",
+  "+components/navigation_interception",
   "+components/network_time",
   "+components/permissions",
   "+components/pref_registry",

weblayer/browser/content_browser_client_impl.cc

@@ -71,6 +71,7 @@
 #include "base/bind.h"
 #include "base/task/post_task.h"
 #include "components/crash/content/browser/crash_handler_host_linux.h"
+#include "components/navigation_interception/intercept_navigation_delegate.h"
 #include "components/spellcheck/browser/spell_check_host_impl.h"  // nogncheck
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
@@ -148,26 +149,6 @@ void HandleSSLErrorWrapper(
       std::make_unique<weblayer::WebLayerSecurityBlockingPageFactory>());
 }
 
-#if defined(OS_ANDROID)
-// Returns true if |scheme| identifies one that is handled/known by WebLayer.
-bool IsHandledScheme(base::StringPiece scheme) {
-  DCHECK_EQ(scheme, base::ToLowerASCII(scheme));
-  static const base::NoDestructor<base::flat_set<base::StringPiece>>
-      kKnownSchemes(base::flat_set<base::StringPiece>({
-        content::kChromeDevToolsScheme, content::kChromeUIScheme,
-            content::kChromeUIUntrustedScheme, url::kAboutScheme,
-            url::kBlobScheme, url::kDataScheme, url::kFileScheme,
-            url::kFileSystemScheme, url::kHttpScheme, url::kHttpsScheme,
-            url::kJavaScriptScheme,
-#if BUILDFLAG(ENABLE_WEBSOCKETS)
-            url::kWsScheme, url::kWssScheme,
-#endif  // BUILDFLAG(ENABLE_WEBSOCKETS)
-            url::kContentScheme,
-      }));
-  return kKnownSchemes->contains(scheme);
-}
-#endif  // defined(OS_ANDROID)
-
 }  // namespace
 
 namespace weblayer {
@@ -434,6 +415,12 @@ ContentBrowserClientImpl::CreateThrottlesForNavigation(
       throttles.push_back(
           GetSafeBrowsingService()->CreateSafeBrowsingNavigationThrottle(
               handle));
+      if (handle->IsInMainFrame()) {
+        throttles.push_back(
+            navigation_interception::InterceptNavigationDelegate::
+                CreateThrottleFor(
+                    handle, navigation_interception::SynchronyMode::kAsync));
+      }
     }
   }
 #endif
@@ -546,68 +533,4 @@ void ContentBrowserClientImpl::GetAdditionalMappedFilesForChildProcess(
 }
 #endif  // defined(OS_LINUX) || defined(OS_ANDROID)
 
-#if defined(OS_ANDROID)
-bool ContentBrowserClientImpl::ShouldOverrideUrlLoading(
-    int frame_tree_node_id,
-    bool browser_initiated,
-    const GURL& gurl,
-    const std::string& request_method,
-    bool has_user_gesture,
-    bool is_redirect,
-    bool is_main_frame,
-    ui::PageTransition transition,
-    bool* ignore_navigation) {
-  *ignore_navigation = false;
-
-  // Only GETs can be overridden.
-  if (request_method != "GET")
-    return true;
-
-  bool application_initiated =
-      browser_initiated || transition & ui::PAGE_TRANSITION_FORWARD_BACK;
-
-  // Don't offer application-initiated navigations unless it's a redirect.
-  if (application_initiated && !is_redirect)
-    return true;
-
-  // For HTTP schemes, only top-level navigations can be overridden. Similarly,
-  // WebView Classic lets app override only top level about:blank navigations.
-  // So we filter out non-top about:blank navigations here.
-  if (!is_main_frame &&
-      (gurl.SchemeIs(url::kHttpScheme) || gurl.SchemeIs(url::kHttpsScheme) ||
-       gurl.SchemeIs(url::kAboutScheme)))
-    return true;
-
-  content::WebContents* web_contents =
-      content::WebContents::FromFrameTreeNodeId(frame_tree_node_id);
-  if (web_contents == nullptr)
-    return true;
-
-  if (gurl.is_valid() && IsHandledScheme(gurl.scheme()))
-    return true;
-
-  JNIEnv* env = base::android::AttachCurrentThread();
-
-  base::string16 url = base::UTF8ToUTF16(gurl.possibly_invalid_spec());
-  base::android::ScopedJavaLocalRef<jstring> jurl =
-      base::android::ConvertUTF16ToJavaString(env, url);
-
-  TabImpl* tab_impl = TabImpl::FromWebContents(web_contents);
-  *ignore_navigation = tab_impl->ShouldOverrideUrlLoading(
-      env, 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
-    // current one - we want to make sure we return to Java cleanly without
-    // first making any new JNI calls.
-    base::MessageLoopCurrentForUI::Get()->Abort();
-    // If we crashed we don't want to continue the navigation.
-    *ignore_navigation = true;
-    return false;
-  }
-
-  return true;
-}
-#endif
-
 }  // namespace weblayer

weblayer/browser/content_browser_client_impl.h

@@ -94,18 +94,6 @@ class ContentBrowserClientImpl : public content::ContentBrowserClient {
       content::PosixFileDescriptorInfo* mappings) override;
 #endif  // defined(OS_LINUX) || defined(OS_ANDROID)
 
-#if defined(OS_ANDROID)
-  bool ShouldOverrideUrlLoading(int frame_tree_node_id,
-                                bool browser_initiated,
-                                const GURL& gurl,
-                                const std::string& request_method,
-                                bool has_user_gesture,
-                                bool is_redirect,
-                                bool is_main_frame,
-                                ui::PageTransition transition,
-                                bool* ignore_navigation) override;
-#endif
-
   void CreateFeatureListAndFieldTrials();
 
  private:

weblayer/browser/intercept_navigation_delegate_impl.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/navigation_interception/intercept_navigation_delegate.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/web_contents.h"
+#include "weblayer/browser/java/jni/InterceptNavigationDelegateImpl_jni.h"
+
+static void JNI_InterceptNavigationDelegateImpl_AssociateWithWebContents(
+    JNIEnv* env,
+    const base::android::JavaParamRef<jobject>& jdelegate,
+    const base::android::JavaParamRef<jobject>& jweb_contents) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  content::WebContents* web_contents =
+      content::WebContents::FromJavaWebContents(jweb_contents);
+  navigation_interception::InterceptNavigationDelegate::Associate(
+      web_contents,
+      std::make_unique<navigation_interception::InterceptNavigationDelegate>(
+          env, jdelegate, /*escape_external_handler_value=*/true));
+}

weblayer/browser/java/BUILD.gn

@@ -49,6 +49,7 @@ android_library("java") {
     "org/chromium/weblayer_private/FragmentAndroidPermissionDelegate.java",
     "org/chromium/weblayer_private/FragmentWindowAndroid.java",
     "org/chromium/weblayer_private/FullscreenCallbackProxy.java",
+    "org/chromium/weblayer_private/InterceptNavigationDelegateImpl.java",
     "org/chromium/weblayer_private/LocaleChangedBroadcastReceiver.java",
     "org/chromium/weblayer_private/NavigationControllerImpl.java",
     "org/chromium/weblayer_private/NavigationImpl.java",
@@ -87,12 +88,14 @@ android_library("java") {
     "//components/download/internal/common:internal_java",
     "//components/embedder_support/android:application_java",
     "//components/embedder_support/android:context_menu_java",
+    "//components/embedder_support/android:util_java",
     "//components/embedder_support/android:web_contents_delegate_java",
     "//components/embedder_support/android/metrics:java",
     "//components/find_in_page/android:java",
     "//components/javascript_dialogs/android:java",
     "//components/metrics:metrics_java",
     "//components/minidump_uploader:minidump_uploader_java",
+    "//components/navigation_interception/android:navigation_interception_java",
     "//components/omnibox/browser:browser_java",
     "//components/permissions/android:java",
     "//components/security_interstitials/content/android:java",
@@ -149,6 +152,7 @@ generate_jni("jni") {
     "org/chromium/weblayer_private/DownloadImpl.java",
     "org/chromium/weblayer_private/ErrorPageCallbackProxy.java",
     "org/chromium/weblayer_private/FullscreenCallbackProxy.java",
+    "org/chromium/weblayer_private/InterceptNavigationDelegateImpl.java",
     "org/chromium/weblayer_private/LocaleChangedBroadcastReceiver.java",
     "org/chromium/weblayer_private/NavigationControllerImpl.java",
     "org/chromium/weblayer_private/NavigationImpl.java",

weblayer/browser/java/org/chromium/weblayer_private/ExternalNavigationHandler.java

@@ -10,25 +10,59 @@ import android.content.Intent;
 import android.provider.Browser;
 
 import org.chromium.base.Log;
+import org.chromium.components.embedder_support.util.UrlConstants;
+import org.chromium.components.embedder_support.util.UrlUtilities;
+import org.chromium.content_public.common.ContentUrlConstants;
+import org.chromium.ui.base.PageTransition;
 
 /**
  * A class that handles navigations that should be transformed to intents. Logic taken primarly from
  * //android_webview's AwContentsClient.java:sendBrowsingIntent(), with some additional logic
- * from //android_webview's WebViewBrowserActivity.java:startBrowsingIntent().
+ * from //android_webview's WebViewBrowserActivity.java:startBrowsingIntent() and
+ * //components/external_intents' ExternalNavigationHandler.java.
+ * TODO(crbug.com/1031465): Eliminate this custom class entirely in favor of using
+ * the //component-level ExternalNavigationHandler.java that //chrome uses.
  */
 public class ExternalNavigationHandler {
     private static final String TAG = "ExternalNavHandler";
 
-    static boolean shouldOverrideUrlLoading(TabImpl tab, String url, boolean hasUserGesture,
-            boolean isRedirect, boolean isMainFrame) {
-        // TODO(blundell): Port over WebViewBrowserActivity's
-        // isSpecializedHandlerAvailable() check that checks whether there's an app for handling
-        // the scheme?
-
-        if (!hasUserGesture && !isRedirect) {
-            Log.w(TAG, "Denied starting an intent without a user gesture, URI %s", url);
+    /**
+     * The "about:", "chrome:", "chrome-native:", and "devtools:" schemes
+     * are internal to the browser; don't want these to be dispatched to other apps.
+     */
+    private static boolean hasInternalScheme(
+            String url, Intent targetIntent, boolean hasIntentScheme) {
+        if (hasIntentScheme) {
+            // TODO(https://crbug.com/783819): When this function is converted to GURL, we should
+            // also call fixUpUrl on this user-provided URL as the fixed-up URL is what we would end
+            // up navigating to.
+            url = targetIntent.getDataString();
+        }
+        if (url.startsWith(ContentUrlConstants.ABOUT_SCHEME)
+                || url.startsWith(UrlConstants.CHROME_URL_SHORT_PREFIX)
+                || url.startsWith(UrlConstants.CHROME_NATIVE_URL_SHORT_PREFIX)
+                || url.startsWith(UrlConstants.DEVTOOLS_URL_SHORT_PREFIX)) {
             return true;
         }
+        return false;
+    }
+
+    /** The "content:" scheme is disabled in WebLayer. Do not try to start an activity. */
+    private static boolean hasContentScheme(
+            String url, Intent targetIntent, boolean hasIntentScheme) {
+        if (hasIntentScheme) {
+            url = targetIntent.getDataString();
+        }
+        if (!url.startsWith(UrlConstants.CONTENT_URL_SHORT_PREFIX)) return false;
+        return true;
+    }
+
+    static boolean shouldOverrideUrlLoading(TabImpl tab, String url, boolean hasUserGesture,
+            boolean isRedirect, boolean isMainFrame, int pageTransition) {
+        if (UrlUtilities.isAcceptedScheme(url)) return false;
+
+        // A back-forward navigation should never trigger an intent.
+        if ((pageTransition & PageTransition.FORWARD_BACK) != 0) return false;
 
         Intent intent;
         // Perform generic parsing of the URI to turn it into an Intent.
@@ -38,6 +72,22 @@ public class ExternalNavigationHandler {
             Log.w(TAG, "Bad URI %s", url, ex);
             return false;
         }
+
+        boolean hasIntentScheme = url.startsWith(UrlConstants.INTENT_URL_SHORT_PREFIX)
+                || url.startsWith(UrlConstants.APP_INTENT_URL_SHORT_PREFIX);
+        if (hasInternalScheme(url, intent, hasIntentScheme)) {
+            return false;
+        }
+
+        if (hasContentScheme(url, intent, hasIntentScheme)) {
+            return false;
+        }
+
+        if (!hasUserGesture && !isRedirect) {
+            Log.w(TAG, "Denied starting an intent without a user gesture, URI %s", url);
+            return true;
+        }
+
         // Sanitize the Intent, ensuring web pages can not bypass browser
         // security (only access to BROWSABLE activities).
         intent.addCategory(Intent.CATEGORY_BROWSABLE);

weblayer/browser/java/org/chromium/weblayer_private/InterceptNavigationDelegateImpl.java

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.weblayer_private;
+
+import org.chromium.base.annotations.NativeMethods;
+import org.chromium.components.navigation_interception.InterceptNavigationDelegate;
+import org.chromium.components.navigation_interception.NavigationParams;
+import org.chromium.content_public.browser.WebContents;
+
+/**
+ * Class that controls navigations and allows to intercept them. It is used on Android to 'convert'
+ * certain navigations to Intents to 3rd party applications.
+ */
+public class InterceptNavigationDelegateImpl implements InterceptNavigationDelegate {
+    private TabImpl mTab;
+
+    /**
+     * Default constructor of {@link InterceptNavigationDelegateImpl}.
+     */
+    InterceptNavigationDelegateImpl(TabImpl tab) {
+        mTab = tab;
+        InterceptNavigationDelegateImplJni.get().associateWithWebContents(
+                this, mTab.getWebContents());
+    }
+
+    @Override
+    public boolean shouldIgnoreNavigation(NavigationParams navigationParams) {
+        return ExternalNavigationHandler.shouldOverrideUrlLoading(mTab, navigationParams.url,
+                navigationParams.hasUserGesture, navigationParams.isRedirect,
+                navigationParams.isMainFrame, navigationParams.pageTransitionType);
+    }
+
+    @NativeMethods
+    interface Natives {
+        void associateWithWebContents(
+                InterceptNavigationDelegateImpl nativeInterceptNavigationDelegateImpl,
+                WebContents webContents);
+    }
+}

weblayer/browser/java/org/chromium/weblayer_private/TabImpl.java

@@ -89,6 +89,7 @@ public final class TabImpl extends ITab.Stub {
     private FindResultBar mFindResultBar;
     // See usage note in {@link #onFindResultAvailable}.
     boolean mWaitingForMatchRects;
+    private InterceptNavigationDelegateImpl mInterceptNavigationDelegate;
 
     private static class InternalAccessDelegateImpl
             implements ViewEventSink.InternalAccessDelegate {
@@ -163,6 +164,8 @@ public final class TabImpl extends ITab.Stub {
         }
         mConstraintsUpdatedCallback = (constraints) -> onBrowserControlsStateUpdated(constraints);
         mBrowserControlsVisibility.addObserver(mConstraintsUpdatedCallback);
+
+        mInterceptNavigationDelegate = new InterceptNavigationDelegateImpl(this);
     }
 
     public ProfileImpl getProfile() {
@@ -501,13 +504,6 @@ public final class TabImpl extends ITab.Stub {
         }
     }
 
-    @CalledByNative
-    private boolean shouldOverrideUrlLoading(
-            String url, boolean hasUserGesture, boolean isRedirect, boolean isMainFrame) {
-        return ExternalNavigationHandler.shouldOverrideUrlLoading(
-                this, url, hasUserGesture, isRedirect, isMainFrame);
-    }
-
     public void destroy() {
         if (mTabCallbackProxy != null) {
             mTabCallbackProxy.destroy();

weblayer/browser/tab_impl.cc

@@ -429,16 +429,6 @@ ScopedJavaLocalRef<jstring> TabImpl::GetGuid(JNIEnv* env) {
   return base::android::ConvertUTF8ToJavaString(AttachCurrentThread(),
                                                 GetGuid());
 }
-
-bool TabImpl::ShouldOverrideUrlLoading(
-    JNIEnv* env,
-    base::android::ScopedJavaLocalRef<jstring> jurl,
-    bool has_user_gesture,
-    bool is_redirect,
-    bool is_main_frame) {
-  return Java_TabImpl_shouldOverrideUrlLoading(
-      env, java_impl_, jurl, has_user_gesture, is_redirect, is_main_frame);
-}
 #endif
 
 content::WebContents* TabImpl::OpenURLFromTab(

weblayer/browser/tab_impl.h

@@ -119,14 +119,6 @@ class TabImpl : public Tab,
   void UpdateBrowserControlsState(JNIEnv* env, jint constraint);
 
   base::android::ScopedJavaLocalRef<jstring> GetGuid(JNIEnv* env);
-
-  // Calls through to Java to determine whether the loading of |jurl| should be
-  // overridden, returning true if so.
-  bool ShouldOverrideUrlLoading(JNIEnv* env,
-                                base::android::ScopedJavaLocalRef<jstring> jurl,
-                                bool has_user_gesture,
-                                bool is_redirect,
-                                bool is_main_frame);
 #endif
 
   DownloadDelegate* download_delegate() { return download_delegate_; }