Properly throw java exceptions from shouldOverrideUrlLoading

android_webview/native/aw_contents_client_bridge.cc

Index: android_webview/native/aw_contents_client_bridge.cc
diff --git a/android_webview/native/aw_contents_client_bridge.cc b/android_webview/native/aw_contents_client_bridge.cc
index d05a32b840c823701e618cc135193d0e212752e3..8369ff4513c8a5cf04f140d03b21449a34ce503b 100644
--- a/android_webview/native/aw_contents_client_bridge.cc
+++ b/android_webview/native/aw_contents_client_bridge.cc
@@ -31,6 +31,7 @@ using base::android::AttachCurrentThread;
 using base::android::ConvertJavaStringToUTF16;
 using base::android::ConvertUTF8ToJavaString;
 using base::android::ConvertUTF16ToJavaString;
+using base::android::HasException;
 using base::android::JavaRef;
 using base::android::ScopedJavaLocalRef;
 using content::BrowserThread;
@@ -356,8 +357,17 @@ bool AwContentsClientBridge::ShouldOverrideUrlLoading(const base::string16& url,
   ScopedJavaLocalRef<jstring> jurl = ConvertUTF16ToJavaString(env, url);
   devtools_instrumentation::ScopedEmbedderCallbackTask(
       "shouldOverrideUrlLoading");
-  return Java_AwContentsClientBridge_shouldOverrideUrlLoading(
+  bool did_override = Java_AwContentsClientBridge_shouldOverrideUrlLoading(
       env, obj.obj(), jurl.obj(), has_user_gesture, is_redirect, is_main_frame);
+  if (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.
+    static_cast<base::MessageLoopForUI*>(base::MessageLoop::current())->Abort();

[Torne, 2016/07/21 13:38:25]

This stops later stages of DoRunLoopOnce from executing, but it's not obvious to
me that it prevents any other native chromium tasks from running - am I missing
something? What if there's already another queued task after this current one
for immediate execution?

[gsennton, 2016/07/26 14:42:39]

The current implementation of this (MessageLoop::DoWork() -
https://cs.chromium.org/chromium/src/base/message_loop/message_loop.cc?cl=GRO...)
runs one task and then returns immediately.

danakj@, is MessageLoop.DoWork / DoDelayedWork guaranteed to only run one task?
I.e. is this something we want to uphold in the future as well, and if so, could
we add a comment to the MessagePump::Delegate::DoWork/DoDelayedWork/DoIdleWork
declarations to show that these functions should indeed only run one task each?

+    // If we crashed we don't want to continue the navigation.
+    return true;
+  }
+  return did_override;
 }
 
 void AwContentsClientBridge::ConfirmJsResult(JNIEnv* env,