move NativeModule initialization off UI thread

Summary: Initializing natives modules on the UI thread blocks the JS thread if the UI thread is busy.

Reviewed By: yungsters

Differential Revision: D4611211

fbshipit-source-id: cd4fb9cb5e52a478b6692b784cfd9e3bf34c0d34

ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactTestHelper.java

@@ -23,6 +23,7 @@ import com.facebook.react.bridge.CatalystInstance;
 import com.facebook.react.bridge.JavaScriptModuleRegistry;
 import com.facebook.react.bridge.NativeModule;
 import com.facebook.react.bridge.NativeModuleCallExceptionHandler;
+import com.facebook.react.bridge.ReactApplicationContext;
 import com.facebook.react.bridge.WritableNativeMap;
 import com.facebook.react.bridge.queue.ReactQueueConfigurationSpec;
 import com.facebook.react.cxxbridge.CatalystInstanceImpl;
@@ -36,10 +37,9 @@ public class ReactTestHelper {
   private static class DefaultReactTestFactory implements ReactTestFactory {
     private static class ReactInstanceEasyBuilderImpl implements ReactInstanceEasyBuilder {
 
-      private final NativeModuleRegistryBuilder mNativeModuleRegistryBuilder =
-        new NativeModuleRegistryBuilder(null, null, false);
       private final JavaScriptModuleRegistry.Builder mJSModuleRegistryBuilder =
         new JavaScriptModuleRegistry.Builder();
+      private NativeModuleRegistryBuilder mNativeModuleRegistryBuilder;
 
       private @Nullable Context mContext;
 
@@ -51,6 +51,12 @@ public class ReactTestHelper {
 
       @Override
       public ReactInstanceEasyBuilder addNativeModule(NativeModule nativeModule) {
+        if (mNativeModuleRegistryBuilder == null) {
+          mNativeModuleRegistryBuilder = new NativeModuleRegistryBuilder(
+            (ReactApplicationContext) mContext,
+            null,
+            false);
+        }
         mNativeModuleRegistryBuilder.addNativeModule(nativeModule);
         return this;
       }
@@ -63,6 +69,12 @@ public class ReactTestHelper {
 
       @Override
       public CatalystInstance build() {
+        if (mNativeModuleRegistryBuilder == null) {
+          mNativeModuleRegistryBuilder = new NativeModuleRegistryBuilder(
+            (ReactApplicationContext) mContext,
+            null,
+            false);
+        }
         JavaScriptExecutor executor = null;
         try {
           executor = new JSCJavaScriptExecutor.Factory(new WritableNativeMap()).create();

ReactAndroid/src/main/java/com/facebook/react/NativeModuleRegistryBuilder.java

@@ -139,6 +139,9 @@ public class NativeModuleRegistryBuilder {
       }
     }
 
-    return new NativeModuleRegistry(mModules, batchCompleteListenerModules);
+    return new NativeModuleRegistry(
+      mReactApplicationContext,
+      mModules,
+      batchCompleteListenerModules);
   }
 }

ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java

@@ -11,6 +11,8 @@ package com.facebook.react.animated;
 
 import javax.annotation.Nullable;
 
+import java.util.ArrayList;
+
 import com.facebook.infer.annotation.Assertions;
 import com.facebook.react.bridge.Arguments;
 import com.facebook.react.bridge.Callback;
@@ -27,8 +29,6 @@ import com.facebook.react.modules.core.ReactChoreographer;
 import com.facebook.react.uimanager.GuardedFrameCallback;
 import com.facebook.react.uimanager.UIManagerModule;
 
-import java.util.ArrayList;
-
 /**
  * Module that exposes interface for creating and managing animated nodes on the "native" side.
  *
@@ -90,43 +90,50 @@ public class NativeAnimatedModule extends ReactContextBaseJavaModule implements
 
   @Override
   public void initialize() {
-    // Safe to acquire choreographer here, as initialize() is invoked from UI thread.
+    getReactApplicationContext().addLifecycleEventListener(this);
-    mReactChoreographer = ReactChoreographer.getInstance();
+  }
 
-    ReactApplicationContext reactCtx = getReactApplicationContext();
+  @Override
-    UIManagerModule uiManager = reactCtx.getNativeModule(UIManagerModule.class);
+  public void onHostResume() {
+    if (mReactChoreographer == null) {
+      // Safe to acquire choreographer here, as onHostResume() is invoked from UI thread.
+      mReactChoreographer = ReactChoreographer.getInstance();
 
-    final NativeAnimatedNodesManager nodesManager = new NativeAnimatedNodesManager(uiManager);
+      ReactApplicationContext reactCtx = getReactApplicationContext();
-    mAnimatedFrameCallback = new GuardedFrameCallback(reactCtx) {
+      UIManagerModule uiManager = reactCtx.getNativeModule(UIManagerModule.class);
-      @Override
-      protected void doFrameGuarded(final long frameTimeNanos) {
 
-        ArrayList<UIThreadOperation> operations;
+      final NativeAnimatedNodesManager nodesManager = new NativeAnimatedNodesManager(uiManager);
-        synchronized (mOperationsCopyLock) {
+      mAnimatedFrameCallback = new GuardedFrameCallback(reactCtx) {
-          operations = mReadyOperations;
+        @Override
-          mReadyOperations = null;
+        protected void doFrameGuarded(final long frameTimeNanos) {
-        }
 
-        if (operations != null) {
+          ArrayList<UIThreadOperation> operations;
-          for (int i = 0, size = operations.size(); i < size; i++) {
+          synchronized (mOperationsCopyLock) {
-            operations.get(i).execute(nodesManager);
+            operations = mReadyOperations;
+            mReadyOperations = null;
           }
-        }
 
-        if (nodesManager.hasActiveAnimations()) {
+          if (operations != null) {
-          nodesManager.runUpdates(frameTimeNanos);
+            for (int i = 0, size = operations.size(); i < size; i++) {
-        }
+              operations.get(i).execute(nodesManager);
+            }
+          }
 
-        // TODO: Would be great to avoid adding this callback in case there are no active animations
+          if (nodesManager.hasActiveAnimations()) {
-        // and no outstanding tasks on the operations queue. Apparently frame callbacks can only
+            nodesManager.runUpdates(frameTimeNanos);
-        // be posted from the UI thread and therefore we cannot schedule them directly from
+          }
-        // @ReactMethod methods
+
-        Assertions.assertNotNull(mReactChoreographer).postFrameCallback(
+          // TODO: Would be great to avoid adding this callback in case there are no active animations
-          ReactChoreographer.CallbackType.NATIVE_ANIMATED_MODULE,
+          // and no outstanding tasks on the operations queue. Apparently frame callbacks can only
-          mAnimatedFrameCallback);
+          // be posted from the UI thread and therefore we cannot schedule them directly from
-      }
+          // @ReactMethod methods
-    };
+          Assertions.assertNotNull(mReactChoreographer).postFrameCallback(
-    reactCtx.addLifecycleEventListener(this);
+            ReactChoreographer.CallbackType.NATIVE_ANIMATED_MODULE,
+            mAnimatedFrameCallback);
+        }
+      };
+    }
+    enqueueFrameCallback();
   }
 
   @Override
@@ -150,11 +157,6 @@ public class NativeAnimatedModule extends ReactContextBaseJavaModule implements
     }
   }
 
-  @Override
-  public void onHostResume() {
-    enqueueFrameCallback();
-  }
-
   @Override
   public void onHostPause() {
     clearFrameCallback();

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

@@ -285,6 +285,10 @@ public class ReactContext extends ContextWrapper {
     Assertions.assertNotNull(mNativeModulesMessageQueueThread).assertIsOnThread();
   }
 
+  public void assertOnNativeModulesQueueThread(String message) {
+    Assertions.assertNotNull(mNativeModulesMessageQueueThread).assertIsOnThread(message);
+  }
+
   public boolean isOnNativeModulesQueueThread() {
     return Assertions.assertNotNull(mNativeModulesMessageQueueThread).isOnThread();
   }

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

@@ -46,6 +46,13 @@ public interface MessageQueueThread {
   @DoNotStrip
   void assertIsOnThread();
 
+  /**
+   * Asserts {@link #isOnThread()}, throwing a {@link AssertionException} (NOT an
+   * {@link AssertionError}) if the assertion fails. The given message is appended to the error.
+   */
+  @DoNotStrip
+  void assertIsOnThread(String message);
+
   /**
    * Quits this MessageQueueThread. If called from this MessageQueueThread, this will be the last
    * thing the thread runs. If called from a separate thread, this will block until the thread can

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

@@ -99,6 +99,18 @@ public class MessageQueueThreadImpl implements MessageQueueThread {
     SoftAssertions.assertCondition(isOnThread(), mAssertionErrorMessage);
   }
 
+  /**
+   * Asserts {@link #isOnThread()}, throwing a {@link AssertionException} (NOT an
+   * {@link AssertionError}) if the assertion fails.
+   */
+  @DoNotStrip
+  @Override
+  public void assertIsOnThread(String message) {
+    SoftAssertions.assertCondition(
+      isOnThread(),
+      new StringBuilder().append(mAssertionErrorMessage).append(" ").append(message).toString());
+  }
+
   /**
    * Quits this queue's Looper. If that Looper was running on a different Thread than the current
    * Thread, also waits for the last message being processed to finish and the Thread to die.

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

@@ -20,7 +20,9 @@ import java.util.concurrent.atomic.AtomicInteger;
 import android.content.res.AssetManager;
 
 import com.facebook.common.logging.FLog;
+import com.facebook.infer.annotation.Assertions;
 import com.facebook.jni.HybridData;
+import com.facebook.proguard.annotations.DoNotStrip;
 import com.facebook.react.bridge.CatalystInstance;
 import com.facebook.react.bridge.ExecutorToken;
 import com.facebook.react.bridge.JavaScriptModule;
@@ -30,15 +32,13 @@ import com.facebook.react.bridge.NativeArray;
 import com.facebook.react.bridge.NativeModule;
 import com.facebook.react.bridge.NativeModuleCallExceptionHandler;
 import com.facebook.react.bridge.NotThreadSafeBridgeIdleDebugListener;
-import com.facebook.react.bridge.queue.ReactQueueConfiguration;
 import com.facebook.react.bridge.queue.MessageQueueThread;
 import com.facebook.react.bridge.queue.QueueThreadExceptionHandler;
+import com.facebook.react.bridge.queue.ReactQueueConfiguration;
 import com.facebook.react.bridge.queue.ReactQueueConfigurationImpl;
 import com.facebook.react.bridge.queue.ReactQueueConfigurationSpec;
-import com.facebook.proguard.annotations.DoNotStrip;
 import com.facebook.react.common.ReactConstants;
 import com.facebook.react.common.annotations.VisibleForTesting;
-import com.facebook.infer.annotation.Assertions;
 import com.facebook.soloader.SoLoader;
 import com.facebook.systrace.Systrace;
 import com.facebook.systrace.TraceListener;
@@ -299,7 +299,12 @@ public class CatalystInstanceImpl implements CatalystInstance {
     // TODO: tell all APIs to shut down
     mDestroyed = true;
     mHybridData.resetNative();
-    mJavaRegistry.notifyCatalystInstanceDestroy();
+    mReactQueueConfiguration.getNativeModulesQueueThread().runOnQueue(new Runnable() {
+      @Override
+      public void run() {
+        mJavaRegistry.notifyCatalystInstanceDestroy();
+      }
+    });
     boolean wasIdle = (mPendingJSCalls.getAndSet(0) == 0);
     if (!wasIdle && !mBridgeIdleListeners.isEmpty()) {
       for (NotThreadSafeBridgeIdleDebugListener listener : mBridgeIdleListeners) {
@@ -333,7 +338,12 @@ public class CatalystInstanceImpl implements CatalystInstance {
         mAcceptCalls,
         "RunJSBundle hasn't completed.");
     mInitialized = true;
-    mJavaRegistry.notifyCatalystInstanceInitialized();
+    mReactQueueConfiguration.getNativeModulesQueueThread().runOnQueue(new Runnable() {
+      @Override
+      public void run() {
+        mJavaRegistry.notifyCatalystInstanceInitialized();
+      }
+    });
   }
 
   @Override

ReactAndroid/src/main/java/com/facebook/react/cxxbridge/ModuleHolder.java

@@ -5,12 +5,9 @@ package com.facebook.react.cxxbridge;
 import javax.annotation.Nullable;
 import javax.inject.Provider;
 
-import java.util.concurrent.ExecutionException;
-
 import com.facebook.react.bridge.NativeModule;
 import com.facebook.react.bridge.ReactMarker;
 import com.facebook.react.bridge.ReactMarkerConstants;
-import com.facebook.react.common.futures.SimpleSettableFuture;
 import com.facebook.systrace.Systrace;
 import com.facebook.systrace.SystraceMessage;
 
@@ -124,35 +121,8 @@ public class ModuleHolder {
     }
     section.flush();
     ReactMarker.logMarker(ReactMarkerConstants.INITIALIZE_MODULE_START, mName);
-    callInitializeOnUiThread(module);
+    module.initialize();
     ReactMarker.logMarker(ReactMarkerConstants.INITIALIZE_MODULE_END);
     Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE);
   }
-
-  // TODO(t11394264): Use the native module thread here after the old bridge is gone
-  private static void callInitializeOnUiThread(final NativeModule module) {
-    if (UiThreadUtil.isOnUiThread()) {
-      module.initialize();
-      return;
-    }
-    final SimpleSettableFuture<Void> future = new SimpleSettableFuture<>();
-    UiThreadUtil.runOnUiThread(new Runnable() {
-      @Override
-      public void run() {
-        Systrace.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "initializeOnUiThread");
-        try {
-          module.initialize();
-          future.set(null);
-        } catch (Exception e) {
-          future.setException(e);
-        }
-        Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE);
-      }
-    });
-    try {
-      future.get();
-    } catch (InterruptedException | ExecutionException e) {
-      throw new RuntimeException(e);
-    }
-  }
 }

ReactAndroid/src/main/java/com/facebook/react/cxxbridge/NativeModuleRegistry.java

@@ -17,6 +17,7 @@ import java.util.Map;
 import com.facebook.infer.annotation.Assertions;
 import com.facebook.react.bridge.NativeModule;
 import com.facebook.react.bridge.OnBatchCompleteListener;
+import com.facebook.react.bridge.ReactApplicationContext;
 import com.facebook.react.bridge.ReactMarker;
 import com.facebook.react.bridge.ReactMarkerConstants;
 import com.facebook.systrace.Systrace;
@@ -26,12 +27,15 @@ import com.facebook.systrace.Systrace;
   */
 public class NativeModuleRegistry {
 
+  private final ReactApplicationContext mReactApplicationContext;
   private final Map<Class<? extends NativeModule>, ModuleHolder> mModules;
   private final ArrayList<ModuleHolder> mBatchCompleteListenerModules;
 
   public NativeModuleRegistry(
+    ReactApplicationContext reactApplicationContext,
     Map<Class<? extends NativeModule>, ModuleHolder> modules,
     ArrayList<ModuleHolder> batchCompleteListenerModules) {
+    mReactApplicationContext = reactApplicationContext;
     mModules = modules;
     mBatchCompleteListenerModules = batchCompleteListenerModules;
   }
@@ -60,7 +64,7 @@ public class NativeModuleRegistry {
   }
 
   /* package */ void notifyCatalystInstanceDestroy() {
-    UiThreadUtil.assertOnUiThread();
+    mReactApplicationContext.assertOnNativeModulesQueueThread();
     Systrace.beginSection(
         Systrace.TRACE_TAG_REACT_JAVA_BRIDGE,
         "NativeModuleRegistry_notifyCatalystInstanceDestroy");
@@ -74,8 +78,10 @@ public class NativeModuleRegistry {
   }
 
   /* package */ void notifyCatalystInstanceInitialized() {
-    UiThreadUtil.assertOnUiThread();
+    mReactApplicationContext.assertOnNativeModulesQueueThread("From version React Native v0.44, " +
-
+      "native modules are explicitly not initialized on the UI thread. See " +
+      "https://github.com/facebook/react-native/wiki/Breaking-Changes#d4611211-reactnativeandroidbreaking-move-nativemodule-initialization-off-ui-thread---aaachiuuu " +
+      " for more details.");
     ReactMarker.logMarker(ReactMarkerConstants.NATIVE_MODULE_INITIALIZE_START);
     Systrace.beginSection(
         Systrace.TRACE_TAG_REACT_JAVA_BRIDGE,

ReactAndroid/src/main/java/com/facebook/react/modules/core/Timing.java

@@ -35,8 +35,8 @@ import com.facebook.react.bridge.UiThreadUtil;
 import com.facebook.react.bridge.WritableArray;
 import com.facebook.react.common.SystemClock;
 import com.facebook.react.devsupport.interfaces.DevSupportManager;
-import com.facebook.react.jstasks.HeadlessJsTaskEventListener;
 import com.facebook.react.jstasks.HeadlessJsTaskContext;
+import com.facebook.react.jstasks.HeadlessJsTaskEventListener;
 import com.facebook.react.module.annotations.ReactModule;
 
 /**
@@ -236,8 +236,6 @@ public final class Timing extends ReactContextBaseJavaModule implements Lifecycl
 
   @Override
   public void initialize() {
-    // Safe to acquire choreographer here, as initialize() is invoked from UI thread.
-    mReactChoreographer = ReactChoreographer.getInstance();
     getReactApplicationContext().addLifecycleEventListener(this);
     HeadlessJsTaskContext headlessJsTaskContext =
       HeadlessJsTaskContext.getInstance(getReactApplicationContext());
@@ -259,6 +257,11 @@ public final class Timing extends ReactContextBaseJavaModule implements Lifecycl
 
   @Override
   public void onHostResume() {
+    if (mReactChoreographer == null) {
+      // Safe to acquire choreographer here, as onHostResume() is invoked from UI thread.
+      mReactChoreographer = ReactChoreographer.getInstance();
+    }
+
     isPaused.set(false);
     // TODO(5195192) Investigate possible problems related to restarting all tasks at the same
     // moment
@@ -268,6 +271,11 @@ public final class Timing extends ReactContextBaseJavaModule implements Lifecycl
 
   @Override
   public void onHeadlessJsTaskStart(int taskId) {
+    if (mReactChoreographer == null) {
+      // Safe to acquire choreographer here, as onHeadlessJsTaskStart() is invoked from UI thread.
+      mReactChoreographer = ReactChoreographer.getInstance();
+    }
+
     if (!isRunningTasks.getAndSet(true)) {
       setChoreographerCallback();
       maybeSetChoreographerIdleCallback();