Refactor so that mBridge can not be null and can be final

Reviewed By: astreet

Differential Revision: D2717983

fb-gh-sync-id: 969499eb062d54e0271f910c06c4539e2cb20030

ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java

@@ -14,6 +14,7 @@ import javax.annotation.Nullable;
 import java.io.IOException;
 import java.io.StringWriter;
 import java.util.Collection;
+import java.util.concurrent.Callable;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
@@ -63,7 +64,7 @@ public class CatalystInstanceImpl implements CatalystInstance {
   private boolean mInitialized = false;
 
   // Access from JS thread
-  private @Nullable ReactBridge mBridge;
+  private final ReactBridge mBridge;
   private boolean mJSBundleHasLoaded;
 
   private CatalystInstanceImpl(
@@ -83,39 +84,34 @@ public class CatalystInstanceImpl implements CatalystInstance {
     mNativeModuleCallExceptionHandler = nativeModuleCallExceptionHandler;
     mTraceListener = new JSProfilerTraceListener();
 
-    final CountDownLatch initLatch = new CountDownLatch(1);
-    mCatalystQueueConfiguration.getJSQueueThread().runOnQueue(
-        new Runnable() {
-          @Override
-          public void run() {
-            Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "initializeBridge");
-            try {
-              initializeBridge(jsExecutor, jsModulesConfig);
-              initLatch.countDown();
-            } finally {
-              Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE);
-            }
-          }
-        });
-
     try {
-      Assertions.assertCondition(
+      mBridge = mCatalystQueueConfiguration.getJSQueueThread().callOnQueue(
-          initLatch.await(BRIDGE_SETUP_TIMEOUT_MS, TimeUnit.MILLISECONDS),
+          new Callable<ReactBridge>() {
-          "Timed out waiting for bridge to initialize!");
+            @Override
-    } catch (InterruptedException e) {
+            public ReactBridge call() throws Exception {
-      throw new RuntimeException(e);
+              Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "initializeBridge");
+              try {
+                return initializeBridge(jsExecutor, jsModulesConfig);
+              } finally {
+                Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE);
+              }
+            }
+          }).get(BRIDGE_SETUP_TIMEOUT_MS, TimeUnit.MILLISECONDS);
+    } catch (Exception t) {
+      throw new RuntimeException("Failed to initialize bridge", t);
     }
   }
 
-  private void initializeBridge(
+  private ReactBridge initializeBridge(
       JavaScriptExecutor jsExecutor,
       JavaScriptModulesConfig jsModulesConfig) {
     mCatalystQueueConfiguration.getJSQueueThread().assertIsOnThread();
     Assertions.assertCondition(mBridge == null, "initializeBridge should be called once");
 
     Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "ReactBridgeCtor");
+    ReactBridge bridge;
     try {
-      mBridge = new ReactBridge(
+      bridge = new ReactBridge(
           jsExecutor,
           new NativeModulesReactCallback(),
           mCatalystQueueConfiguration.getNativeModulesQueueThread());
@@ -125,15 +121,17 @@ public class CatalystInstanceImpl implements CatalystInstance {
 
     Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "setBatchedBridgeConfig");
     try {
-      mBridge.setGlobalVariable(
+      bridge.setGlobalVariable(
           "__fbBatchedBridgeConfig",
           buildModulesConfigJSONProperty(mJavaRegistry, jsModulesConfig));
-      mBridge.setGlobalVariable(
+      bridge.setGlobalVariable(
           "__RCTProfileIsProfiling",
           Systrace.isTracing(Systrace.TRACE_TAG_REACT_APPS) ? "true" : "false");
     } finally {
       Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE);
     }
+
+    return bridge;
   }
 
   @Override
@@ -260,13 +258,11 @@ public class CatalystInstanceImpl implements CatalystInstance {
       }
     }
 
-    if (mBridge != null) {
+    Systrace.unregisterListener(mTraceListener);
-      Systrace.unregisterListener(mTraceListener);
-    }
 
     // We can access the Bridge from any thread now because we know either we are on the JS thread
     // or the JS thread has finished via CatalystQueueConfiguration#destroy()
-    Assertions.assertNotNull(mBridge).dispose();
+    mBridge.dispose();
   }
 
   @Override
@@ -294,8 +290,7 @@ public class CatalystInstanceImpl implements CatalystInstance {
   }
 
   @VisibleForTesting
-  public @Nullable
+  public ReactBridge getBridge() {
-  ReactBridge getBridge() {
     return mBridge;
   }
 
@@ -341,25 +336,16 @@ public class CatalystInstanceImpl implements CatalystInstance {
 
   @Override
   public boolean supportsProfiling() {
-    if (mBridge == null) {
-      return false;
-    }
     return mBridge.supportsProfiling();
   }
 
   @Override
   public void startProfiler(String title) {
-    if (mBridge == null) {
-      return;
-    }
     mBridge.startProfiler(title);
   }
 
   @Override
   public void stopProfiler(String title, String filename) {
-    if (mBridge == null) {
-      return;
-    }
     mBridge.stopProfiler(title, filename);
   }
 

ReactAndroid/src/main/java/com/facebook/react/bridge/queue/MessageQueueThread.java

@@ -9,6 +9,9 @@
 
 package com.facebook.react.bridge.queue;
 
+import java.util.concurrent.Callable;
+import java.util.concurrent.Future;
+
 import com.facebook.proguard.annotations.DoNotStrip;
 
 /**
@@ -23,6 +26,13 @@ public interface MessageQueueThread {
   @DoNotStrip
   void runOnQueue(Runnable runnable);
 
+  /**
+   * Runs the given Callable on this Thread. It will be submitted to the end of the event queue even
+   * if it is being submitted from the same queue Thread.
+   */
+  @DoNotStrip
+  <T> Future<T> callOnQueue(final Callable<T> callable);
+
   /**
    * @return whether the current Thread is also the Thread associated with this MessageQueueThread.
    */

ReactAndroid/src/main/java/com/facebook/react/bridge/queue/MessageQueueThreadImpl.java

@@ -9,6 +9,10 @@
 
 package com.facebook.react.bridge.queue;
 
+import java.util.concurrent.Callable;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+
 import android.os.Looper;
 
 import com.facebook.common.logging.FLog;
@@ -45,6 +49,7 @@ import com.facebook.react.common.futures.SimpleSettableFuture;
    * if it is being submitted from the same queue Thread.
    */
   @DoNotStrip
+  @Override
   public void runOnQueue(Runnable runnable) {
     if (mIsFinished) {
       FLog.w(
@@ -55,9 +60,29 @@ import com.facebook.react.common.futures.SimpleSettableFuture;
     mHandler.post(runnable);
   }
 
+
+  @DoNotStrip
+  @Override
+  public <T> Future<T> callOnQueue(final Callable<T> callable) {
+    final SimpleSettableFuture<T> future = new SimpleSettableFuture<>();
+    runOnQueue(
+        new Runnable() {
+          @Override
+          public void run() {
+            try {
+              future.set(callable.call());
+            } catch (Exception e) {
+              future.setException(e);
+            }
+          }
+        });
+    return future;
+  }
+
   /**
    * @return whether the current Thread is also the Thread associated with this MessageQueueThread.
    */
+  @Override
   public boolean isOnThread() {
     return mLooper.getThread() == Thread.currentThread();
   }
@@ -66,6 +91,7 @@ import com.facebook.react.common.futures.SimpleSettableFuture;
    * Asserts {@link #isOnThread()}, throwing a {@link AssertionException} (NOT an
    * {@link AssertionError}) if the assertion fails.
    */
+  @Override
   public void assertIsOnThread() {
     SoftAssertions.assertCondition(isOnThread(), mAssertionErrorMessage);
   }
@@ -139,6 +165,13 @@ import com.facebook.react.common.futures.SimpleSettableFuture;
         }, "mqt_" + name);
     bgThread.start();
 
-    return new MessageQueueThreadImpl(name, simpleSettableFuture.get(5000), exceptionHandler);
+    try {
+      return new MessageQueueThreadImpl(
+          name,
+          simpleSettableFuture.get(5000, TimeUnit.MILLISECONDS),
+          exceptionHandler);
+    } catch (Throwable t) {
+      throw new RuntimeException(t);
+    }
   }
 }

ReactAndroid/src/main/java/com/facebook/react/common/futures/SimpleSettableFuture.java

@@ -12,29 +12,61 @@ package com.facebook.react.common.futures;
 import javax.annotation.Nullable;
 
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 
 /**
  * A super simple Future-like class that can safely notify another Thread when a value is ready.
- * Does not support setting errors or canceling.
+ * Does not support canceling.
  */
-public class SimpleSettableFuture<T> {
+public class SimpleSettableFuture<T> implements Future<T> {
-
   private final CountDownLatch mReadyLatch = new CountDownLatch(1);
-  private volatile @Nullable T mResult;
+  private @Nullable T mResult;
+  private @Nullable Exception mException;
 
   /**
    * Sets the result. If another thread has called {@link #get}, they will immediately receive the
-   * value. Must only be called once.
+   * value. set or setException must only be called once.
    */
   public void set(T result) {
-    if (mReadyLatch.getCount() == 0) {
+    checkNotSet();
-      throw new RuntimeException("Result has already been set!");
-    }
     mResult = result;
     mReadyLatch.countDown();
   }
 
+  /**
+   * Sets the eception. If another thread has called {@link #get}, they will immediately receive the
+   * exception. set or setException must only be called once.
+   */
+  public void setException(Exception exception) {
+    checkNotSet();
+    mException = exception;
+    mReadyLatch.countDown();
+  }
+
+  @Override
+  public boolean cancel(boolean mayInterruptIfRunning) {
+    throw new UnsupportedOperationException();
+  }
+
+  @Override
+  public boolean isCancelled() {
+    return false;
+  }
+
+  @Override
+  public boolean isDone() {
+    return mReadyLatch.getCount() == 0;
+  }
+
+  @Deprecated
+  @Override
+  public T get() throws InterruptedException, ExecutionException {
+    throw new UnsupportedOperationException("Must use a timeout");
+  }
+
   /**
    * Wait up to the timeout time for another Thread to set a value on this future. If a value has
    * already been set, this method will return immediately.
@@ -42,21 +74,26 @@ public class SimpleSettableFuture<T> {
    * NB: For simplicity, we catch and wrap InterruptedException. Do NOT use this class if you
    * are in the 1% of cases where you actually want to handle that.
    */
-  public @Nullable T get(long timeoutMS) {
+  @Override
+  public @Nullable T get(long timeout, TimeUnit unit) throws
+      InterruptedException, ExecutionException, TimeoutException {
     try {
-      if (!mReadyLatch.await(timeoutMS, TimeUnit.MILLISECONDS)) {
+      if (!mReadyLatch.await(timeout, unit)) {
-        throw new TimeoutException();
+        throw new TimeoutException("Timed out waiting for result");
       }
     } catch (InterruptedException e) {
       throw new RuntimeException(e);
     }
+    if (mException != null) {
+      throw new ExecutionException(mException);
+    }
+
     return mResult;
   }
 
-  public static class TimeoutException extends RuntimeException {
+  private void checkNotSet() {
-
+    if (mReadyLatch.getCount() == 0) {
-    public TimeoutException() {
+      throw new RuntimeException("Result has already been set!");
-      super("Timed out waiting for future");
     }
   }
 }