Backed out changeset 2fee8f9b283d (bug 1496745) for bustage on GeckoResult.java:317. CLOSED TREE
authorCsoregi Natalia <ncsoregi@mozilla.com>
Wed, 17 Oct 2018 18:59:03 +0300
changeset 490090 480c167630359dfb2b683f3f4c876a27a637635e
parent 490089 c203c7e7a7fa5e6c519d390dd67d9826febcf7cd
child 490091 c4a8a6256d7f800bd2d592bc6467bcfef5cd9309
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
bugs1496745
milestone64.0a1
backs out2fee8f9b283dac85340edf7815fd62b93352bd33
Backed out changeset 2fee8f9b283d (bug 1496745) for bustage on GeckoResult.java:317. CLOSED TREE
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/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,16 +48,23 @@ 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();
@@ -330,105 +337,9 @@ 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,20 +1,18 @@
 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;();
@@ -24,19 +22,18 @@ import java.util.concurrent.TimeoutExcep
  *         } 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. 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>
+ * listeners on the result. All listeners are run on the application main thread. 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) {
@@ -112,23 +109,16 @@ import java.util.concurrent.TimeoutExcep
  *     }
  * }).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 {
@@ -164,49 +154,36 @@ 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 final Handler mHandler;
+    private 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 if (Looper.myLooper() != null) {
+        } else {
             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);
     }
@@ -285,18 +262,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 {@link Looper} returned from
-     * {@link #getLooper()}. If null, this method will throw {@link IllegalThreadStateException}.
+     * 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.
      *
      * 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}.
@@ -304,39 +281,38 @@ 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(() -> {
-            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(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);
+                    }
                 }
             }
         });
         return result;
     }
 
     private synchronized void then(@NonNull final Runnable listener) {
         if (mComplete) {
@@ -344,178 +320,94 @@ 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;
         }
 
-        final Runnable dispatcher = () -> {
-            if (mListeners != null) {
-                for (final Runnable listener : mListeners) {
-                    listener.run();
+        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);
                 }
-            } 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");
         }
 
-        if (mHandler == null) {
-            runnable.run();
-            return;
-        }
-
-        mHandler.post(runnable);
+        mHandler.post(new Runnable() {
+            @Override
+            public void run() {
+                runnable.run();
+            }
+        });
     }
 
     /**
      * 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(() -> {
-            if (other.haveValue()) {
-                complete(other.mValue);
-            } else {
-                mIsUncaughtError = other.mIsUncaughtError;
-                completeExceptionally(other.mError);
+        other.then(new Runnable() {
+            @Override
+            public void run() {
+                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.
@@ -528,17 +420,16 @@ 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> {