Skip to content

Commit

Permalink
webview: Fix exception handling in evaluateJavascript.
Browse files Browse the repository at this point in the history
This case was converted to use PostTask along with all the other cases
in the file, but because the PostTask queue is managed in native (once
native is initialized) this fails to achieve the original goal of
posting it described in the comment: to make sure there is no native
code on the stack, so that we can handle the app's callback throwing an
exception without crashing in native.

Introduce an AwThreadUtils class that contains specifically-documented
methods for posting tasks in the desired way, and use it in
evaluateJavascript and other methods which want this exception-handling
behaviour, to make it clear that using Handler directly is intentional
and necessary.

Fixed: 719396
Bug: 944437
Change-Id: I95cc029801e632a22987eb92e00cc28d4c292332
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2144461
Commit-Queue: Richard Coles <torne@chromium.org>
Commit-Queue: Bo <boliu@chromium.org>
Auto-Submit: Richard Coles <torne@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759845}
  • Loading branch information
tornewuff authored and Commit Bot committed Apr 16, 2020
1 parent 0d2c068 commit ebaae77
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 14 deletions.
1 change: 1 addition & 0 deletions android_webview/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@ android_library("browser_java") {
"java/src/org/chromium/android_webview/AwServiceWorkerSettings.java",
"java/src/org/chromium/android_webview/AwSettings.java",
"java/src/org/chromium/android_webview/AwSupportLibIsomorphic.java",
"java/src/org/chromium/android_webview/AwThreadUtils.java",
"java/src/org/chromium/android_webview/AwTracingController.java",
"java/src/org/chromium/android_webview/AwVariationsSeedBridge.java",
"java/src/org/chromium/android_webview/AwViewAndroidDelegate.java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2660,7 +2660,7 @@ public void evaluateJavaScript(String script, final Callback<String> callback) {
// application callback is executed without any native code on the stack. This
// so that any exception thrown by the application callback won't have to be
// propagated through a native call stack.
PostTask.postTask(UiThreadTaskTraits.DEFAULT, () -> callback.onResult(jsonResult));
AwThreadUtils.postToCurrentLooper(() -> callback.onResult(jsonResult));
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
package org.chromium.android_webview;

import org.chromium.base.Log;
import org.chromium.base.ThreadUtils;
import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.JNINamespace;

Expand Down Expand Up @@ -36,12 +35,9 @@ private AwWebResourceInterceptResponse shouldInterceptRequestFromNative(String u
Log.e(TAG,
"Client raised exception in shouldInterceptRequest. Re-throwing on UI thread.");

ThreadUtils.getUiThreadHandler().post(new Runnable() {
@Override
public void run() {
Log.e(TAG, "The following exception was raised by shouldInterceptRequest:");
throw e;
}
AwThreadUtils.postToUiThreadLooper(() -> {
Log.e(TAG, "The following exception was raised by shouldInterceptRequest:");
throw e;
});

return new AwWebResourceInterceptResponse(null, /*raisedException=*/true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import android.content.Context;
import android.net.http.SslCertificate;
import android.net.http.SslError;
import android.os.Handler;
import android.util.Log;

import org.chromium.android_webview.safe_browsing.AwSafeBrowsingConversionHelper;
Expand Down Expand Up @@ -171,7 +170,7 @@ private boolean allowCertificateError(int certError, byte[] derBytes, final Stri
// callback is executed without any native code on the stack. This so that any exception
// thrown by the application callback won't have to be propagated through a native call
// stack.
new Handler().post(() -> mClient.onReceivedSslError(callback, sslError));
AwThreadUtils.postToCurrentLooper(() -> mClient.onReceivedSslError(callback, sslError));

// Record UMA on ssl error
// Use sparse histogram in case new values are added in future releases
Expand Down Expand Up @@ -236,7 +235,7 @@ private void handleJsAlert(final String url, final String message, final int id)
// callback is executed without any native code on the stack. This so that any exception
// thrown by the application callback won't have to be propagated through a native call
// stack.
new Handler().post(() -> {
AwThreadUtils.postToCurrentLooper(() -> {
JsResultHandler handler = new JsResultHandler(AwContentsClientBridge.this, id);
mClient.handleJsAlert(url, message, handler);
});
Expand All @@ -248,7 +247,7 @@ private void handleJsConfirm(final String url, final String message, final int i
// callback is executed without any native code on the stack. This so that any exception
// thrown by the application callback won't have to be propagated through a native call
// stack.
new Handler().post(() -> {
AwThreadUtils.postToCurrentLooper(() -> {
JsResultHandler handler = new JsResultHandler(AwContentsClientBridge.this, id);
mClient.handleJsConfirm(url, message, handler);
});
Expand All @@ -261,7 +260,7 @@ private void handleJsPrompt(
// callback is executed without any native code on the stack. This so that any exception
// thrown by the application callback won't have to be propagated through a native call
// stack.
new Handler().post(() -> {
AwThreadUtils.postToCurrentLooper(() -> {
JsResultHandler handler = new JsResultHandler(AwContentsClientBridge.this, id);
mClient.handleJsPrompt(url, message, defaultValue, handler);
});
Expand All @@ -273,7 +272,7 @@ private void handleJsBeforeUnload(final String url, final String message, final
// callback is executed without any native code on the stack. This so that any exception
// thrown by the application callback won't have to be propagated through a native call
// stack.
new Handler().post(() -> {
AwThreadUtils.postToCurrentLooper(() -> {
JsResultHandler handler = new JsResultHandler(AwContentsClientBridge.this, id);
mClient.handleJsBeforeUnload(url, message, handler);
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// 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.android_webview;

import android.os.Handler;

import org.chromium.base.ThreadUtils;

/**
* Provides WebView-specific threading utilities.
*/
public class AwThreadUtils {
/**
* Post a task to the current thread, ensuring that it runs on the underlying Android looper
* without any native code present on the stack. This allows uncaught Java exceptions to be
* handled correctly by Android's crash reporting mechanisms.
*/
public static void postToCurrentLooper(Runnable r) {
new Handler().post(r);
}

/**
* Post a task to the UI thread, ensuring that it runs on the underlying Android looper without
* any native code present on the stack. This allows uncaught Java exceptions to be handled
* correctly by Android's crash reporting mechanisms.
*/
public static void postToUiThreadLooper(Runnable r) {
ThreadUtils.getUiThreadHandler().post(r);
}
}

0 comments on commit ebaae77

Please sign in to comment.