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 497449 480c167630359dfb2b683f3f4c876a27a637635e
parent 497448 c203c7e7a7fa5e6c519d390dd67d9826febcf7cd
child 497450 c4a8a6256d7f800bd2d592bc6467bcfef5cd9309
push id9996
push userarchaeopteryx@coole-files.de
push dateThu, 18 Oct 2018 18:37:15 +0000
treeherdermozilla-beta@8efe26839243 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1496745
milestone64.0a1
backs out2fee8f9b283dac85340edf7815fd62b93352bd33
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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> {