Bug 1474450 - 2. Rethrow uncaught and unhandled exceptions in GeckoResult; r?snorp draft
authorJim Chen <nchen@mozilla.com>
Tue, 10 Jul 2018 13:12:56 -0400
changeset 816193 2a1210be2ddd7ee57fb8166a1741937fbb1d44f2
parent 816192 adbc3030a047b6019d7c2ddfb5c439c7d7f0c647
child 816194 14259eade1690bc894e62c7b3d3102e1e8519643
push id115770
push userbmo:nchen@mozilla.com
push dateTue, 10 Jul 2018 17:29:29 +0000
reviewerssnorp
bugs1474450
milestone63.0a1
Bug 1474450 - 2. Rethrow uncaught and unhandled exceptions in GeckoResult; r?snorp If at the end of a chain, we have an uncaught and unhandled exception, rethrow the exception to make it more visible. Also, when a GeckoResult is completed with a value/exception, propagate the value/exception properly down the chain. MozReview-Commit-ID: F4tnST1nKe5
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoResult.java
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoResult.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoResult.java
@@ -1,32 +1,37 @@
 package org.mozilla.geckoview;
 
 import org.mozilla.gecko.util.ThreadUtils;
 
-import android.os.Handler;
 import android.os.Looper;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 
 import java.util.ArrayList;
 
 /**
  * GeckoResult is a class that represents an asynchronous result.
  *
  * @param <T> The type of the value delivered via the GeckoResult.
  */
 public class GeckoResult<T> {
     private static final String LOGTAG = "GeckoResult";
 
+    public static final class UncaughtException extends RuntimeException {
+        public UncaughtException(final Throwable cause) {
+            super(cause);
+        }
+    }
+
     private boolean mComplete;
     private T mValue;
     private Throwable mError;
-
-    private ArrayList<Runnable> mListeners = null;
+    private boolean mIsUncaughtError;
+    private ArrayList<Runnable> mListeners;
 
     /**
      * This constructs an incomplete GeckoResult. Call {@link #complete(Object)} or
      * {@link #completeExceptionally(Throwable)} in order to fulfill the result.
      */
     public GeckoResult() {
     }
 
@@ -96,97 +101,110 @@ public class GeckoResult<T> {
     /**
      * Convenience method for {@link #then(OnValueListener, OnExceptionListener)}.
      *
      * @param valueListener An instance of {@link OnValueListener}, called when the
      *                      {@link GeckoResult} is completed with a value.
      * @param <U>
      * @return
      */
-    public synchronized @NonNull <U> GeckoResult<U> then(@NonNull final OnValueListener<T, U> valueListener) {
+    public @NonNull <U> GeckoResult<U> then(@NonNull final OnValueListener<T, U> valueListener) {
         return then(valueListener, null);
     }
 
     /**
      * Convenience method for {@link #then(OnValueListener, OnExceptionListener)}.
      *
      * @param exceptionListener An instance of {@link OnExceptionListener}, called when the
      *                          {@link GeckoResult} is completed with an {@link Exception}.
      * @param <U> The type contained in the returned {@link GeckoResult}
      * @return
      */
-    public synchronized @NonNull <U> GeckoResult<U> then(@NonNull final OnExceptionListener<U> exceptionListener) {
+    public @NonNull <U> GeckoResult<U> then(@NonNull final OnExceptionListener<U> exceptionListener) {
         return then(null, exceptionListener);
     }
 
     /**
      * Adds listeners to be called when the {@link GeckoResult} is completed either with
      * a value or {@link Throwable}. Listeners will be invoked on the main thread. If the
      * result is already complete when this method is called, listeners will be invoked in
      * a future {@link Looper} iteration.
      *
      * @param valueListener An instance of {@link OnValueListener}, called when the
      *                      {@link GeckoResult} is completed with a value.
      * @param exceptionListener An instance of {@link OnExceptionListener}, called when the
      *                          {@link GeckoResult} is completed with an {@link Throwable}.
      * @param <U> The type contained in the returned {@link GeckoResult}
      */
-    public synchronized @NonNull <U> GeckoResult<U> then(@Nullable final OnValueListener<T, U> valueListener,
-                                                         @Nullable final OnExceptionListener<U> exceptionListener) {
+    public @NonNull <U> GeckoResult<U> then(@Nullable final OnValueListener<T, U> valueListener,
+                                            @Nullable final OnExceptionListener<U> exceptionListener) {
         if (valueListener == null && exceptionListener == null) {
             throw new IllegalArgumentException("At least one listener should be non-null");
         }
 
         final GeckoResult<U> result = new GeckoResult<U>();
-        final Runnable listener = new Runnable() {
+        then(new Runnable() {
             @Override
             public void run() {
                 try {
-                    if (valueListener != null && haveValue()) {
-                        result.completeFrom(valueListener.onValue(mValue));
-                    } else if (exceptionListener != null && haveError()) {
+                    if (haveValue()) {
+                        result.completeFrom(valueListener != null ? valueListener.onValue(mValue)
+                                                                  : null);
+                    } else if (!haveError()) {
+                        // Listener called without completion?
+                        throw new AssertionError();
+                    } else if (exceptionListener != null) {
                         result.completeFrom(exceptionListener.onException(mError));
+                    } else {
+                        result.mIsUncaughtError = mIsUncaughtError;
+                        result.completeExceptionally(mError);
                     }
                 } catch (Throwable e) {
                     if (!result.mComplete) {
+                        result.mIsUncaughtError = true;
                         result.completeExceptionally(e);
                     }
                 }
             }
-        };
+        });
+        return result;
+    }
 
+    private synchronized void then(@NonNull final Runnable listener) {
         if (mComplete) {
             dispatchLocked(listener);
         } else {
             if (mListeners == null) {
                 mListeners = new ArrayList<>(1);
             }
             mListeners.add(listener);
         }
-
-        return result;
     }
 
     private void dispatchLocked() {
         if (!mComplete) {
             throw new IllegalStateException("Cannot dispatch unless result is complete");
         }
 
-        if (mListeners == null) {
+        if (mListeners == null && !mIsUncaughtError) {
             return;
         }
 
         ThreadUtils.getUiHandler().post(new Runnable() {
             @Override
             public void run() {
-                for (final Runnable listener : mListeners) {
-                    listener.run();
+                if (mListeners != null) {
+                    for (final Runnable listener : mListeners) {
+                        listener.run();
+                    }
+                } else if (mIsUncaughtError) {
+                    // We have no listeners to forward the uncaught exception to;
+                    // rethrow the exception to make it visible.
+                    throw new UncaughtException(mError);
                 }
-
-                mListeners = null;
             }
         });
     }
 
     private void dispatchLocked(final Runnable runnable) {
         if (!mComplete) {
             throw new IllegalStateException("Cannot dispatch unless result is complete");
         }
@@ -205,27 +223,25 @@ public class GeckoResult<T> {
      * @param other The result that this result should mirror
      */
     private void completeFrom(final GeckoResult<T> other) {
         if (other == null) {
             complete(null);
             return;
         }
 
-        other.then(new OnValueListener<T, Void>() {
+        other.then(new Runnable() {
             @Override
-            public GeckoResult<Void> onValue(T value) {
-                complete(value);
-                return null;
-            }
-        }, new OnExceptionListener<Void>() {
-            @Override
-            public GeckoResult<Void> onException(Throwable error) {
-                completeExceptionally(error);
-                return null;
+            public void run() {
+                if (other.haveValue()) {
+                    complete(other.mValue);
+                } else {
+                    mIsUncaughtError = other.mIsUncaughtError;
+                    completeExceptionally(other.mError);
+                }
             }
         });
     }
 
     /**
      * This completes the result with the specified value. IllegalStateException is thrown
      * if the result is already complete.
      *