diff --git a/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactTestHelper.java b/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactTestHelper.java index a4d770e94..8054dff33 100644 --- a/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactTestHelper.java +++ b/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(); diff --git a/ReactAndroid/src/main/java/com/facebook/react/NativeModuleRegistryBuilder.java b/ReactAndroid/src/main/java/com/facebook/react/NativeModuleRegistryBuilder.java index 4977d60d9..1a5f06c37 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/NativeModuleRegistryBuilder.java +++ b/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); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java index c5e2aa04f..8256e7b56 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java +++ b/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. - mReactChoreographer = ReactChoreographer.getInstance(); + getReactApplicationContext().addLifecycleEventListener(this); + } - ReactApplicationContext reactCtx = getReactApplicationContext(); - UIManagerModule uiManager = reactCtx.getNativeModule(UIManagerModule.class); + @Override + 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); - mAnimatedFrameCallback = new GuardedFrameCallback(reactCtx) { - @Override - protected void doFrameGuarded(final long frameTimeNanos) { + ReactApplicationContext reactCtx = getReactApplicationContext(); + UIManagerModule uiManager = reactCtx.getNativeModule(UIManagerModule.class); - ArrayList operations; - synchronized (mOperationsCopyLock) { - operations = mReadyOperations; - mReadyOperations = null; - } + final NativeAnimatedNodesManager nodesManager = new NativeAnimatedNodesManager(uiManager); + mAnimatedFrameCallback = new GuardedFrameCallback(reactCtx) { + @Override + protected void doFrameGuarded(final long frameTimeNanos) { - if (operations != null) { - for (int i = 0, size = operations.size(); i < size; i++) { - operations.get(i).execute(nodesManager); + ArrayList operations; + synchronized (mOperationsCopyLock) { + operations = mReadyOperations; + mReadyOperations = null; } - } - if (nodesManager.hasActiveAnimations()) { - nodesManager.runUpdates(frameTimeNanos); - } + if (operations != null) { + 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 - // and no outstanding tasks on the operations queue. Apparently frame callbacks can only - // be posted from the UI thread and therefore we cannot schedule them directly from - // @ReactMethod methods - Assertions.assertNotNull(mReactChoreographer).postFrameCallback( - ReactChoreographer.CallbackType.NATIVE_ANIMATED_MODULE, - mAnimatedFrameCallback); - } - }; - reactCtx.addLifecycleEventListener(this); + if (nodesManager.hasActiveAnimations()) { + nodesManager.runUpdates(frameTimeNanos); + } + + // TODO: Would be great to avoid adding this callback in case there are no active animations + // and no outstanding tasks on the operations queue. Apparently frame callbacks can only + // be posted from the UI thread and therefore we cannot schedule them directly from + // @ReactMethod methods + Assertions.assertNotNull(mReactChoreographer).postFrameCallback( + 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(); diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java index d416b7d5f..b403b2c9f 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java +++ b/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(); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/queue/MessageQueueThread.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/queue/MessageQueueThread.java index 84cc9accb..fca4ddc18 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/queue/MessageQueueThread.java +++ b/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 diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/queue/MessageQueueThreadImpl.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/queue/MessageQueueThreadImpl.java index 4687fc140..f78988ca8 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/queue/MessageQueueThreadImpl.java +++ b/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. diff --git a/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/CatalystInstanceImpl.java b/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/CatalystInstanceImpl.java index 14e5b774b..0b564b42a 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/CatalystInstanceImpl.java +++ b/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 diff --git a/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/ModuleHolder.java b/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/ModuleHolder.java index 87aba77b9..52ac2059e 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/ModuleHolder.java +++ b/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 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); - } - } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/NativeModuleRegistry.java b/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/NativeModuleRegistry.java index f20d0d32c..93d9e276d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/NativeModuleRegistry.java +++ b/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, ModuleHolder> mModules; private final ArrayList mBatchCompleteListenerModules; public NativeModuleRegistry( + ReactApplicationContext reactApplicationContext, Map, ModuleHolder> modules, ArrayList 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, diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/core/Timing.java b/ReactAndroid/src/main/java/com/facebook/react/modules/core/Timing.java index c6da41a84..0e504d5d8 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/core/Timing.java +++ b/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();