Add test to ensure shouldOverrideUrlLoading throws Java exception

android_webview/test/shell/src/org/chromium/android_webview/test/ShouldOverrideUrlLoadingJniCrashTestRunner.java

Index: android_webview/test/shell/src/org/chromium/android_webview/test/ShouldOverrideUrlLoadingJniCrashTestRunner.java
diff --git a/android_webview/test/shell/src/org/chromium/android_webview/test/ShouldOverrideUrlLoadingJniCrashTestRunner.java b/android_webview/test/shell/src/org/chromium/android_webview/test/ShouldOverrideUrlLoadingJniCrashTestRunner.java
new file mode 100644
index 0000000000000000000000000000000000000000..3e625cf7756e2a82629c6d4275450dcd8e6f6643
--- /dev/null
+++ b/android_webview/test/shell/src/org/chromium/android_webview/test/ShouldOverrideUrlLoadingJniCrashTestRunner.java
@@ -0,0 +1,92 @@
+// Copyright 2016 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.test;
+
+import android.app.Service;
+import android.os.Message;
+import android.os.Messenger;
+import android.os.RemoteException;
+
+import org.chromium.android_webview.AwBrowserContext;
+import org.chromium.android_webview.AwContents;
+import org.chromium.android_webview.test.AwTestBaseUtility.TestDependencyFactory;
+import org.chromium.base.Log;
+import org.chromium.base.test.util.InMemorySharedPreferences;
+
+/**
+ * Class containing code to be run in a service in a separate process to ensure a Java crash in
+ * ShouldOverrideUrlLoading is propagated to the uncaughtExceptionHandler of the Service.
+ */
+public class ShouldOverrideUrlLoadingJniCrashTestRunner extends ServiceTestRunner {

[boliu, 2016/09/15 04:57:53]

ditto, can this live in JniCrashTest?

+    private static final String TAG = "SOULJniCrashTest";
+    public static final int RETURN_JNI_CRASH_TEST_SUCCESS = 1;
+    public static final int RETURN_JNI_CRASH_TEST_FAILED = 2;
+
+    /**
+     * Ensure that when we navigate a WebView using a shouldOverrideUrlLoading callback that
+     * throws a java exception, and ensure the exception is properly thrown (as opposed to
+     * causing a native crash).
+     */
+    @Override
+    public void runTestOnMainThread(final Messenger testMessenger, Service service) {
+        final Message successMessage = Message.obtain(null, RETURN_JNI_CRASH_TEST_SUCCESS);
+        final Message failureMessage = Message.obtain(null, RETURN_JNI_CRASH_TEST_FAILED);
+        Thread.currentThread().setUncaughtExceptionHandler(new Thread.UncaughtExceptionHandler() {
+            @Override
+            public void uncaughtException(Thread t, Throwable e) {
+                try {
+                    if (e instanceof MySpecialException) {
+                        testMessenger.send(successMessage);
+                    } else {
+                        Log.e(TAG, "Wrong exception thrown from shouldOverrideUrlLoading", e);
+                        testMessenger.send(failureMessage);
+                    }
+                } catch (RemoteException re) {
+                    Log.e(TAG, "RemoteException in uncaughtExceptionHandler", re);
+                }
+                // We don't call the original uncaughtExceptionHandler here - instead we kill the

[Torne, 2016/09/15 10:26:44]

This will mean that the current thread dies, but the process doesn't (the
current thread *always* exits when there's an uncaught exception, no matter what
the handler does or whether you call the original one) - so if that's the UI
thread, the system is going to ANR you five seconds after the next time it sends
you a message.

The original handler kills the whole process when this happens and that's the
expected behaviour *on android* - the system expects this and if you're
continuing to run without a UI thread then weird behaviour is likely..

[boliu, 2016/09/15 16:47:51]

That was the original code. Then I assumed killing the process here will race
with sending the message (probably?).

If there is a way to ensure the message is sent won't get dropped from killing
here, or if there is a way to wait for it to be sent, then just calling the
original handler is fine.
I remember asking something to be moved off the UI thread earlier... now I can't
remember where I said that :|

ANR won't happen in service I think. But the other point still stands I guess.

[gsennton, 2016/09/15 17:08:35]

Yeah, if the thread + process don't die while this UEH call is still active then
we could just block here until we receive a message from the test-process
ensuring us that the test has received the successMessage.. is this true though?
Can we even receive binder calls at this point? (before the UEH finishes)

[boliu, 2016/09/15 17:13:12]

You are talking about nested looper. And I have no idea if that's supported on
android or not. Either way, should be avoided if possible..

[gsennton, 2016/09/15 17:17:21]

I don't see the connection between this case and a nested looper, can you
explain? :P

[boliu, 2016/09/15 17:22:46]

You can't block the thread and then wait for a event that will be posted to the
thread you just blocked.. assuming the event does get added to the queue, you
could say, lets just keep running through the event queue, and that's a nested
looper (or nested message loop in chrome parlance..)

You can probably structure the thing such that the thread that just got nuked is
*not* the thread that receives messages. Then you can probably wait for it. But
it's like all so complex....

+                // service from the test when done (to ensure any messages sent from the service to
+                // the test are received before the service is killed).
+            }
+        });
+        try {
+            // Set up WebView and navigate on it to cause shouldOverrideUrlLoading to be called.
+            AwTestBaseUtility.startBrowserProcess(service);
+        } catch (Exception e) {
+            try {
+                testMessenger.send(failureMessage);
+            } catch (RemoteException re) {
+                Log.e(TAG, "RemoteException when trying to send fail message to test process", re);
+            }
+            return;
+        }
+
+        TestAwContentsClient contentsClient = new TestAwContentsClient() {
+            @Override
+            public boolean shouldOverrideUrlLoading(AwWebResourceRequest request) {
+                throw new MySpecialException();
+            }
+        };
+
+        // Create a WebView with a failing shouldOverrideUrlLoading callback
+        AwBrowserContext browserContext = new AwBrowserContext(
+                new InMemorySharedPreferences(), service.getApplicationContext());
+        AwTestContainerView testContainerView = AwTestBaseUtility.createDetachedAwTestContainerView(
+                service, browserContext, new TestDependencyFactory(), contentsClient,
+                false /* supportsLegacyQuirks */, true /* allowHardwareAcceleration */);
+        AwContents awContents = testContainerView.getAwContents();
+
+        // Navigate -> causes shouldOverrideUrlLoading to be called
+        awContents.getSettings().setJavaScriptEnabled(true);
+        contentsClient.getOnEvaluateJavaScriptResultHelper().evaluateJavaScriptForTests(
+                awContents.getWebContents(), "window.location.href = 'about:blank'");
+    }
+
+    private static class MySpecialException extends RuntimeException {
+        MySpecialException() {
+            super();
+        }
+    }
+}