Properly throw java exceptions from shouldOverrideUrlLoading

base/message_loop/message_pump_android.cc

Index: base/message_loop/message_pump_android.cc
diff --git a/base/message_loop/message_pump_android.cc b/base/message_loop/message_pump_android.cc
index a0eee12c85c3034e9b42ec82d259e96619b06759..38791fc0c775cb6fab7c02b68c15fc023989756d 100644
--- a/base/message_loop/message_pump_android.cc
+++ b/base/message_loop/message_pump_android.cc
@@ -24,10 +24,14 @@ using base::android::ScopedJavaLocalRef;
 static void DoRunLoopOnce(JNIEnv* env,
                           const JavaParamRef<jobject>& obj,
                           jlong native_delegate,
+                          jlong native_message_pump,
                           jlong delayed_scheduled_time_ticks) {
   base::MessagePump::Delegate* delegate =
       reinterpret_cast<base::MessagePump::Delegate*>(native_delegate);
   DCHECK(delegate);
+  base::MessagePumpForUI* pump =
+      reinterpret_cast<base::MessagePumpForUI*>(native_message_pump);
+  DCHECK(pump);
   // This is based on MessagePumpForUI::DoRunLoop() from desktop.
   // Note however that our system queue is handled in the java side.
   // In desktop we inspect and process a single system message and then
@@ -35,6 +39,11 @@ static void DoRunLoopOnce(JNIEnv* env,
   // On Android, the java message queue may contain messages for other handlers
   // that will be processed before calling here again.
   bool did_work = delegate->DoWork();
+  if (pump->ShouldAbort()) {
+    // There is a pending JNI exception, return to Java so that the exception is
+    // thrown correctly.
+    return;
+  }
 
   // In the java side, |SystemMessageHandler| keeps a single "delayed" message.
   // It's an expensive operation to |removeMessage| there, so this is optimized
@@ -60,6 +69,11 @@ static void DoRunLoopOnce(JNIEnv* env,
   // avoid comparisons with TimeDelta / Now() (expensive).
   base::TimeTicks next_delayed_work_time;
   did_work |= delegate->DoDelayedWork(&next_delayed_work_time);
+  if (pump->ShouldAbort()) {
+    // There is a pending JNI exception, return to Java so that the exception is
+    // thrown correctly
+    return;
+  }
 
   if (!next_delayed_work_time.is_null()) {
     // Schedule a new message if there's nothing already scheduled or there's a
@@ -81,13 +95,26 @@ static void DoRunLoopOnce(JNIEnv* env,
     return;
 
   delegate->DoIdleWork();
+  // Note that we do not check whether we should abort here since we are
+  // returning to the JVM anyway. If, in the future, we add any more code after
+  // the call to DoIdleWork() here, we should add an abort-check and return
+  // immediately if the check passes.
+}
+
+// This should only be called during tests
+static void NotifyTestDone(JNIEnv* env,
+                           const JavaParamRef<jclass>& jcaller,
+                           jlong native_message_pump) {
+  base::MessagePumpForUI* pump =
+      reinterpret_cast<base::MessagePumpForUI*>(native_message_pump);
+  DCHECK(pump);
+  pump->NotifyTestDone();
 }
 
 namespace base {
 
 MessagePumpForUI::MessagePumpForUI()
-    : run_loop_(NULL) {
-}
+    : run_loop_(nullptr), should_abort_(false) {}
 
 MessagePumpForUI::~MessagePumpForUI() {
 }
@@ -109,9 +136,15 @@ void MessagePumpForUI::Start(Delegate* delegate) {
   JNIEnv* env = base::android::AttachCurrentThread();
   DCHECK(env);
 
-  system_message_handler_obj_.Reset(
-      Java_SystemMessageHandler_create(
-          env, reinterpret_cast<intptr_t>(delegate)));
+  system_message_handler_obj_.Reset(Java_SystemMessageHandler_create(
+      env, reinterpret_cast<intptr_t>(delegate),
+      reinterpret_cast<intptr_t>(this)));
+}
+
+void MessagePumpForUI::StartForTesting(Delegate* delegate,
+                                       WaitableEvent* test_event) {
+  test_event_ = test_event;
+  Start(delegate);

[nyquist, 2016/08/15 19:06:14]

Could this instantiate a subclass of SystemMessageHandler that overrides the
handleMessage method to do what you want?
If so, maybe then it'd be possible to clean up some of the other code as well
since then it's more contained to that subclass?

[gsennton, 2016/08/16 15:27:36]

Done.

 }
 
 void MessagePumpForUI::Quit() {
@@ -156,6 +189,11 @@ void MessagePumpForUI::ScheduleDelayedWork(const TimeTicks& delayed_work_time) {
       delayed_work_time.ToInternalValue(), millis);
 }
 
+void MessagePumpForUI::NotifyTestDone() {
+  DCHECK(test_event_);
+  test_event_->Signal();
+}
+
 // static
 bool MessagePumpForUI::RegisterBindings(JNIEnv* env) {
   return RegisterNativesImpl(env);