Commit ebaae77d authored by Torne (Richard Coles)'s avatar Torne (Richard Coles) Committed by Commit Bot

webview: Fix exception handling in evaluateJavascript.

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: default avatarBo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759845}
parent 0d2c0688
......@@ -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",
......
......@@ -2660,7 +2660,7 @@ public class AwContents implements SmartClipProvider {
// 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));
};
}
......
......@@ -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;
......@@ -36,12 +35,9 @@ public abstract class AwContentsBackgroundThreadClient {
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);
......
......@@ -7,7 +7,6 @@ package org.chromium.android_webview;
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;
......@@ -171,7 +170,7 @@ public class AwContentsClientBridge {
// 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
......@@ -236,7 +235,7 @@ public class AwContentsClientBridge {
// 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);
});
......@@ -248,7 +247,7 @@ public class AwContentsClientBridge {
// 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);
});
......@@ -261,7 +260,7 @@ public class AwContentsClientBridge {
// 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);
});
......@@ -273,7 +272,7 @@ public class AwContentsClientBridge {
// 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);
});
......
// 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);
}
}
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