Bug 1496745 - Allow GeckoResult to be used without a Looper r=agi,jchen
authorJames Willcox <snorp@snorp.net>
Thu, 18 Oct 2018 18:01:18 +0000
changeset 490329 740c3623a44aed64203d830a03682e14f90917f9
parent 490328 50cc3c94b20d7ba104fc24b887e15064cad69d41
child 490330 9782e0b1657da7b2742e693222f8ac912920b521
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersagi, jchen
bugs1496745
milestone64.0a1
Bug 1496745 - Allow GeckoResult to be used without a Looper r=agi,jchen GeckoResult can now be created on a thread with no Looper present. You can use `then` as before after creating a derived GeckoResult with a Handler via `withHandler`, or poll for the value via the new `poll` method. Differential Revision: https://phabricator.services.mozilla.com/D7896
mobile/android/geckoview/build.gradle
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/GeckoResultTest.java
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoResult.java
--- a/mobile/android/geckoview/build.gradle
+++ b/mobile/android/geckoview/build.gradle
@@ -258,17 +258,17 @@ android.libraryVariants.all { variant ->
             variant.generateBuildConfig.sourceOutputDir +
             variant.aidlCompile.sourceOutputDir)
 
         // javadoc 8 has a bug that requires the rt.jar file from the JRE to be
         // in the bootclasspath (https://stackoverflow.com/a/30458820).
         options.bootClasspath = [
             file("${System.properties['java.home']}/lib/rt.jar")] + android.bootClasspath
         options.memberLevel = JavadocMemberLevel.PROTECTED
-        options.source = 7
+        options.source = 8
         options.links("https://d.android.com/reference/")
 
         options.docTitle = "GeckoView ${mozconfig.substs.MOZ_APP_VERSION} API"
         options.header = "GeckoView ${mozconfig.substs.MOZ_APP_VERSION} API"
         options.noTimestamp = true
         options.noIndex = true
         options.noQualifiers = ['java.lang']
         options.tags = ['hide:a:']
--- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/GeckoResultTest.java
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/GeckoResultTest.java
@@ -1,28 +1,28 @@
 package org.mozilla.geckoview.test;
 
 import org.mozilla.geckoview.GeckoResult;
 import org.mozilla.geckoview.GeckoResult.OnExceptionListener;
 import org.mozilla.geckoview.GeckoResult.OnValueListener;
 import org.mozilla.geckoview.test.util.UiThreadUtils;
 
+import android.os.Handler;
 import android.os.Looper;
-import android.support.annotation.NonNull;
-import android.support.annotation.Nullable;
 import android.support.test.annotation.UiThreadTest;
 import android.support.test.filters.MediumTest;
-import android.support.test.rule.UiThreadTestRule;
 import android.support.test.runner.AndroidJUnit4;
 
 import org.junit.Before;
-import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import java.util.concurrent.SynchronousQueue;
+import java.util.concurrent.TimeoutException;
+
 import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.assertThat;
 
 @RunWith(AndroidJUnit4.class)
 @MediumTest
 public class GeckoResultTest {
     private static final long DEFAULT_TIMEOUT = 5000;
 
@@ -48,23 +48,16 @@ public class GeckoResultTest {
         });
     }
 
     @Before
     public void setup() {
         mDone = false;
     }
 
-    @Test(expected = RuntimeException.class)
-    public void createWithoutLooper() {
-        // Without @UiThreadTest this will be run in a worker
-        // thread that does not have a Looper.
-        new GeckoResult<Integer>();
-    }
-
     @Test
     @UiThreadTest
     public void thenWithResult() {
         GeckoResult.fromValue(42).then(new OnValueListener<Integer, Void>() {
             @Override
             public GeckoResult<Void> onValue(Integer value) {
                 assertThat("Value should match", value, equalTo(42));
                 done();
@@ -337,9 +330,105 @@ public class GeckoResultTest {
                            exception, instanceOf(MockException.class));
                 done();
                 return null;
             }
         });
 
         waitUntilDone();
     }
+
+    @Test(expected = IllegalThreadStateException.class)
+    public void noLooperThenThrows() {
+        assertThat("We shouldn't have a Looper", Looper.myLooper(), nullValue());
+        GeckoResult.fromValue(42).then(value -> null);
+    }
+
+    @Test
+    public void noLooperPoll() throws Throwable {
+        assertThat("We shouldn't have a Looper", Looper.myLooper(), nullValue());
+        assertThat("Value should match",
+                GeckoResult.fromValue(42).poll(0), equalTo(42));
+    }
+
+    @Test
+    public void withHandler() {
+
+        final SynchronousQueue<Handler> queue = new SynchronousQueue<>();
+        final Thread thread = new Thread(() -> {
+            Looper.prepare();
+
+            try {
+                queue.put(new Handler());
+            } catch (InterruptedException e) {
+                throw new RuntimeException(e);
+            }
+
+            Looper.loop();
+        });
+
+        thread.start();
+
+        final GeckoResult<Integer> result = GeckoResult.fromValue(42);
+        assertThat("We shouldn't have a Looper", result.getLooper(), nullValue());
+
+        try {
+            result.withHandler(queue.take()).then(value -> {
+                assertThat("Thread should match", Thread.currentThread(), equalTo(thread));
+                assertThat("Value should match", value, equalTo(42));
+                Looper.myLooper().quit();
+                return null;
+            });
+
+            thread.join();
+        } catch (InterruptedException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    @Test
+    public void pollCompleteWithValue() throws Throwable {
+        assertThat("Value should match",
+                GeckoResult.fromValue(42).poll(0), equalTo(42));
+    }
+
+    @Test(expected = MockException.class)
+    public void pollCompleteWithError() throws Throwable {
+        GeckoResult.fromException(new MockException()).poll(0);
+    }
+
+    @Test
+    public void pollIncompleteWithValue() throws Throwable {
+        final GeckoResult<Integer> result = new GeckoResult<>();
+
+        final Thread thread = new Thread(() -> result.complete(42));
+
+        thread.start();
+        assertThat("Value should match", result.poll(), equalTo(42));
+    }
+
+    @Test(expected = MockException.class)
+    public void pollIncompleteWithError() throws Throwable {
+        final GeckoResult<Void> result = new GeckoResult<>();
+
+        final Thread thread = new Thread(() -> result.completeExceptionally(new MockException()));
+
+        thread.start();
+        result.poll();
+    }
+
+    @Test(expected = TimeoutException.class)
+    public void pollTimeout() throws Throwable {
+        new GeckoResult<Void>().poll(1);
+    }
+
+    @UiThreadTest
+    @Test(expected = TimeoutException.class)
+    public void pollTimeoutWithLooper() throws Throwable {
+        new GeckoResult<Void>().poll(1);
+    }
+
+    @UiThreadTest
+    @Test(expected = IllegalThreadStateException.class)
+    public void pollWithLooper() throws Throwable {
+        new GeckoResult<Void>().poll();
+    }
 }
--- 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,18 +1,20 @@
 package org.mozilla.geckoview;
 
 import org.mozilla.gecko.util.ThreadUtils;
 
 import android.os.Handler;
 import android.os.Looper;
+import android.os.SystemClock;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 
 import java.util.ArrayList;
+import java.util.concurrent.TimeoutException;
 
 /**
  * GeckoResult is a class that represents an asynchronous result. The result is initially pending,
  * and at a later time, the result may be completed with {@link #complete a value} or {@link
  * #completeExceptionally an exception} depending on the outcome of the asynchronous operation. For
  * example,<pre>
  * public GeckoResult&lt;Integer&gt; divide(final int dividend, final int divisor) {
  *     final GeckoResult&lt;Integer&gt; result = new GeckoResult&lt;&gt;();
@@ -22,18 +24,19 @@ import java.util.ArrayList;
  *         } else {
  *             result.completeExceptionally(new ArithmeticException("Dividing by zero"));
  *         }
  *     })).start();
  *     return result;
  * }</pre>
  * <p>
  * To retrieve the completed value or exception, use one of the {@link #then} methods to register
- * listeners on the result. All listeners are run on the application main thread. For example, to
- * retrieve a completed value,<pre>
+ * listeners on the result. Listeners are run on the thread where the GeckoResult is created if a
+ * {@link Looper} is present. For example,
+ * to retrieve a completed value,<pre>
  * divide(42, 2).then(new GeckoResult.OnValueListener&lt;Integer, Void&gt;() {
  *     &#64;Override
  *     public GeckoResult&lt;Void&gt; onValue(final Integer value) {
  *         // value == 21
  *     }
  * }, new GeckoResult.OnExceptionListener&lt;Void&gt;() {
  *     &#64;Override
  *     public GeckoResult&lt;Void&gt; onException(final Throwable exception) {
@@ -109,16 +112,23 @@ import java.util.ArrayList;
  *     }
  * }).then(new GeckoResult.OnValueListener&lt;String, Void&gt;() {
  *     &#64;Override
  *     public GeckoResult&lt;Void&gt; onValue(final String value) {
  *         // value == null
  *     }
  * });</pre>
  * <p>
+ * If a GeckoResult is created on a thread without a {@link Looper},
+ * {@link #then(OnValueListener, OnExceptionListener)} is unusable (and will throw
+ * {@link IllegalThreadStateException}). In this scenario, the value is only
+ * available via {@link #poll(long)}. Alternatively, you may also chain the GeckoResult to one with a
+ * {@link Handler} via {@link #withHandler(Handler)}. You may then use
+ * {@link #then(OnValueListener, OnExceptionListener)} on the returned GeckoResult normally.
+ * <p>
  * Any exception thrown by a listener are automatically used to complete the result. At the end of
  * every chain, there is an implicit exception listener that rethrows any uncaught and unhandled
  * exception as {@link UncaughtException}. The following example will cause {@link
  * UncaughtException} to be thrown because {@code BazException} is uncaught and unhandled at the
  * end of the chain,<pre>
  * GeckoResult.fromValue(42).then(new GeckoResult.OnValueListener&lt;Integer, Void&gt;() {
  *     &#64;Override
  *     public GeckoResult&lt;Void&gt; onValue(final Integer value) throws FooException {
@@ -154,36 +164,49 @@ public class GeckoResult<T> {
      */
     public static final GeckoResult<AllowOrDeny> ALLOW = GeckoResult.fromValue(AllowOrDeny.ALLOW);
 
     /**
      * A GeckoResult that resolves to AllowOrDeny.DENY
      */
     public static final GeckoResult<AllowOrDeny> DENY = GeckoResult.fromValue(AllowOrDeny.DENY);
 
-    private Handler mHandler;
+    private final Handler mHandler;
     private boolean mComplete;
     private T mValue;
     private Throwable mError;
     private boolean mIsUncaughtError;
     private ArrayList<Runnable> mListeners;
 
     /**
      * Construct an incomplete GeckoResult. Call {@link #complete(Object)} or
      * {@link #completeExceptionally(Throwable)} in order to fulfill the result.
      */
     public GeckoResult() {
         if (ThreadUtils.isOnUiThread()) {
             mHandler = ThreadUtils.getUiHandler();
-        } else {
+        } else if (Looper.myLooper() != null) {
             mHandler = new Handler();
+        } else {
+            mHandler = null;
         }
     }
 
     /**
+     * Construct an incomplete GeckoResult. Call {@link #complete(Object)} or
+     * {@link #completeExceptionally(Throwable)} in order to fulfill the result.
+     *
+     * @param handler This {@link Handler} will be used for dispatching
+     *                listeners registered via {@link #then(OnValueListener, OnExceptionListener)}.
+     */
+    public GeckoResult(final Handler handler) {
+        mHandler = handler;
+    }
+
+    /**
      * This constructs a result that is chained to the specified result.
      *
      * @param from The {@link GeckoResult} to copy.
      */
     public GeckoResult(GeckoResult<T> from) {
         this();
         completeFrom(from);
     }
@@ -262,18 +285,18 @@ public class GeckoResult<T> {
      * @return A new {@link GeckoResult} that the listener will complete.
      */
     public @NonNull <U> GeckoResult<U> exceptionally(@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 thread where the
-     * {@link GeckoResult} was created, which must have a {@link Looper} installed.
+     * a value or {@link Throwable}. Listeners will be invoked on the {@link Looper} returned from
+     * {@link #getLooper()}. If null, this method will throw {@link IllegalThreadStateException}.
      *
      * 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}.
@@ -281,38 +304,39 @@ public class GeckoResult<T> {
      * @return A new {@link GeckoResult} that the listeners will complete.
      */
     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");
         }
 
+        if (mHandler == null) {
+            throw new IllegalThreadStateException("Must have a Handler");
+        }
+
         final GeckoResult<U> result = new GeckoResult<U>();
-        then(new Runnable() {
-            @Override
-            public void run() {
-                try {
-                    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);
-                    }
+        then(() -> {
+            try {
+                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) {
@@ -320,94 +344,178 @@ public class GeckoResult<T> {
         } else {
             if (mListeners == null) {
                 mListeners = new ArrayList<>(1);
             }
             mListeners.add(listener);
         }
     }
 
+    /**
+     * @return Get the {@link Looper} that will be used to schedule listeners registered via
+     *         {@link #then(OnValueListener, OnExceptionListener)}.
+     */
+    public @Nullable Looper getLooper() {
+        if (mHandler == null) {
+            return null;
+        }
+
+        return mHandler.getLooper();
+    }
+
+    /**
+     * Returns a new GeckoResult that will be completed by this instance. Listeners registered
+     * via {@link #then(OnValueListener, OnExceptionListener)} will be run on the specified
+     * {@link Handler}.
+     *
+     * @param handler A {@link Handler} where listeners will be run. May be null.
+     * @return A new GeckoResult.
+     */
+    public @NonNull GeckoResult<T> withHandler(final @Nullable Handler handler) {
+        final GeckoResult<T> result = new GeckoResult<>(handler);
+        result.completeFrom(this);
+        return result;
+    }
+
     private void dispatchLocked() {
         if (!mComplete) {
             throw new IllegalStateException("Cannot dispatch unless result is complete");
         }
 
         if (mListeners == null && !mIsUncaughtError) {
             return;
         }
 
-        mHandler.post(new Runnable() {
-            @Override
-            public void 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);
+        final Runnable dispatcher = () -> {
+            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);
             }
-        });
+        };
+
+        dispatchLocked(dispatcher);
     }
 
     private void dispatchLocked(final Runnable runnable) {
         if (!mComplete) {
             throw new IllegalStateException("Cannot dispatch unless result is complete");
         }
 
-        mHandler.post(new Runnable() {
-            @Override
-            public void run() {
-                runnable.run();
-            }
-        });
+        if (mHandler == null) {
+            runnable.run();
+            return;
+        }
+
+        mHandler.post(runnable);
     }
 
     /**
      * Completes this result based on another result.
      *
      * @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 Runnable() {
-            @Override
-            public void run() {
-                if (other.haveValue()) {
-                    complete(other.mValue);
-                } else {
-                    mIsUncaughtError = other.mIsUncaughtError;
-                    completeExceptionally(other.mError);
-                }
+        other.then(() -> {
+            if (other.haveValue()) {
+                complete(other.mValue);
+            } else {
+                mIsUncaughtError = other.mIsUncaughtError;
+                completeExceptionally(other.mError);
             }
         });
     }
 
     /**
+     * Return the value of this result, waiting for it to be completed
+     * if necessary. If the result is completed with an exception
+     * it will be rethrown here.
+     * <p>
+     * You must not call this method if the current thread has a {@link Looper} due to
+     * the possibility of a deadlock. If this occurs, {@link IllegalStateException}
+     * is thrown.
+     *
+     * @return The value of this result.
+     * @throws Throwable The {@link Throwable} contained in this result, if any.
+     * @throws IllegalThreadStateException if this method is called on a thread that has a {@link Looper}.
+     */
+    public synchronized T poll() throws Throwable {
+        if (Looper.myLooper() != null) {
+            throw new IllegalThreadStateException("Cannot poll indefinitely from thread with Looper");
+        }
+
+        return poll(Long.MAX_VALUE);
+    }
+
+    /**
+     * Return the value of this result, waiting for it to be completed
+     * if necessary. If the result is completed with an exception
+     * it will be rethrown here.
+     *
+     * Caution is advised if the caller is on a thread with a {@link Looper}, as it's possible to
+     * effectively deadlock in cases when the work is being completed on the calling thread. It's
+     * preferable to use {@link #then(OnValueListener, OnExceptionListener)} in such circumstances,
+     * but if you must use this method consider a small timeout value.
+     *
+     * @param timeoutMillis Number of milliseconds to wait for the result
+     *                      to complete.
+     * @return The value of this result.
+     * @throws Throwable The {@link Throwable} contained in this result, if any.
+     * @throws TimeoutException if we wait more than timeoutMillis before the result
+     *                          is completed.
+     */
+    public synchronized T poll(long timeoutMillis) throws Throwable {
+        final long start = SystemClock.uptimeMillis();
+        long remaining = timeoutMillis;
+        while (!mComplete && remaining > 0) {
+            try {
+                wait(remaining);
+            } catch (InterruptedException e) {
+            }
+
+            remaining = timeoutMillis - (SystemClock.uptimeMillis() - start);
+        }
+
+        if (!mComplete) {
+            throw new TimeoutException();
+        }
+
+        if (haveError()) {
+            throw mError;
+        }
+
+        return mValue;
+    }
+
+    /**
      * Complete the result with the specified value. IllegalStateException is thrown
      * if the result is already complete.
      *
      * @param value The value used to complete the result.
      * @throws IllegalStateException If the result is already completed.
      */
     public synchronized void complete(final T value) {
         if (mComplete) {
             throw new IllegalStateException("result is already complete");
         }
 
         mValue = value;
         mComplete = true;
 
         dispatchLocked();
+        notifyAll();
     }
 
     /**
      * Complete the result with the specified {@link Throwable}. IllegalStateException is thrown
      * if the result is already complete.
      *
      * @param exception The {@link Throwable} used to complete the result.
      * @throws IllegalStateException If the result is already completed.
@@ -420,16 +528,17 @@ public class GeckoResult<T> {
         if (exception == null) {
             throw new IllegalArgumentException("Throwable must not be null");
         }
 
         mError = exception;
         mComplete = true;
 
         dispatchLocked();
+        notifyAll();
     }
 
     /**
      * An interface used to deliver values to listeners of a {@link GeckoResult}
      * @param <T> Type of the value delivered via {@link #onValue(Object)}
      * @param <U> Type of the value for the result returned from {@link #onValue(Object)}
      */
     public interface OnValueListener<T, U> {