From b9cf207db9c18bddf9b9c719dd905e802ae6a504 Mon Sep 17 00:00:00 2001 From: Dark Knight <> Date: Thu, 21 Jul 2022 12:21:48 -0700 Subject: [PATCH] Revert D37912783: Multisect successfully blamed D37912783 for test or build failures Summary: changelog: [internal] This diff is reverting D37912783 (https://github.com/facebook/react-native/commit/2b57b749fbdeee8340dee385288d194155f693c8) Depends on D38035753 D37912783 (https://github.com/facebook/react-native/commit/2b57b749fbdeee8340dee385288d194155f693c8) has been identified to be causing the following test or build failures: Tests affected: - https://www.internalfb.com/intern/test/281475006604971/ Here's the Multisect link: https://www.internalfb.com/intern/testinfra/multisect/1077515 Here are the tasks that are relevant to this breakage: T93091116: 1 test started failing for oncall messenger_kids_www_rn in the last 2 weeks We're generating a revert to back out the changes in this diff, please note the backout may land if someone accepts it. Reviewed By: sammy-SC Differential Revision: D38035761 fbshipit-source-id: 70034af3275b7b69c0b50f12a377182d4f23e669 --- .../react/bridge/CatalystInstanceImpl.java | 13 +++++-- .../react/config/ReactFeatureFlags.java | 4 +++ .../jni/react/jni/CatalystInstanceImpl.cpp | 36 +++++++++++++------ .../main/jni/react/jni/CatalystInstanceImpl.h | 12 +++++-- 4 files changed, 50 insertions(+), 15 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java index ad5fb6985f9..50aedd43421 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java @@ -108,7 +108,8 @@ public class CatalystInstanceImpl implements CatalystInstance { // C++ parts private final HybridData mHybridData; - private static native HybridData initHybrid(); + private static native HybridData initHybrid( + boolean enableRuntimeScheduler, boolean enableRuntimeSchedulerInTurboModule); public native CallInvokerHolderImpl getJSCallInvokerHolder(); @@ -123,7 +124,15 @@ public class CatalystInstanceImpl implements CatalystInstance { FLog.d(ReactConstants.TAG, "Initializing React Xplat Bridge."); Systrace.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "createCatalystInstanceImpl"); - mHybridData = initHybrid(); + if (ReactFeatureFlags.enableRuntimeSchedulerInTurboModule + && !ReactFeatureFlags.enableRuntimeScheduler) { + Assertions.assertUnreachable(); + } + + mHybridData = + initHybrid( + ReactFeatureFlags.enableRuntimeScheduler, + ReactFeatureFlags.enableRuntimeSchedulerInTurboModule); mReactQueueConfiguration = ReactQueueConfigurationImpl.create( diff --git a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java index 68728b18fed..a8a5264042b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java +++ b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java @@ -70,6 +70,10 @@ public class ReactFeatureFlags { /** This feature flag enables logs for Fabric */ public static boolean enableFabricLogs = false; + public static boolean enableRuntimeScheduler = false; + + public static boolean enableRuntimeSchedulerInTurboModule = false; + /** Feature flag to configure eager attachment of the root view/initialisation of the JS code */ public static boolean enableEagerRootViewAttachment = false; diff --git a/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp b/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp index e44913e4fd0..6a936c66f66 100644 --- a/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp +++ b/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp @@ -93,12 +93,21 @@ class JInstanceCallback : public InstanceCallback { } // namespace jni::local_ref -CatalystInstanceImpl::initHybrid(jni::alias_ref) { - return makeCxxInstance(); +CatalystInstanceImpl::initHybrid( + jni::alias_ref, + bool enableRuntimeScheduler, + bool enableRuntimeSchedulerInTurboModule) { + return makeCxxInstance( + enableRuntimeScheduler, enableRuntimeSchedulerInTurboModule); } -CatalystInstanceImpl::CatalystInstanceImpl() - : instance_(std::make_unique()) {} +CatalystInstanceImpl::CatalystInstanceImpl( + bool enableRuntimeScheduler, + bool enableRuntimeSchedulerInTurboModule) + : instance_(std::make_unique()), + enableRuntimeScheduler_(enableRuntimeScheduler), + enableRuntimeSchedulerInTurboModule_( + enableRuntimeScheduler && enableRuntimeSchedulerInTurboModule) {} void CatalystInstanceImpl::warnOnLegacyNativeModuleSystemUse() { CxxNativeModule::setShouldWarnOnUse(true); @@ -373,12 +382,17 @@ void CatalystInstanceImpl::handleMemoryPressure(int pressureLevel) { jni::alias_ref CatalystInstanceImpl::getJSCallInvokerHolder() { if (!jsCallInvokerHolder_) { - auto runtimeScheduler = getRuntimeScheduler(); - auto runtimeSchedulerCallInvoker = - std::make_shared( - runtimeScheduler->cthis()->get()); - jsCallInvokerHolder_ = jni::make_global( - CallInvokerHolder::newObjectCxxArgs(runtimeSchedulerCallInvoker)); + if (enableRuntimeSchedulerInTurboModule_) { + auto runtimeScheduler = getRuntimeScheduler(); + auto runtimeSchedulerCallInvoker = + std::make_shared( + runtimeScheduler->cthis()->get()); + jsCallInvokerHolder_ = jni::make_global( + CallInvokerHolder::newObjectCxxArgs(runtimeSchedulerCallInvoker)); + } else { + jsCallInvokerHolder_ = jni::make_global( + CallInvokerHolder::newObjectCxxArgs(instance_->getJSCallInvoker())); + } } return jsCallInvokerHolder_; } @@ -426,7 +440,7 @@ CatalystInstanceImpl::getRuntimeExecutor() { jni::alias_ref CatalystInstanceImpl::getRuntimeScheduler() { - if (!runtimeScheduler_) { + if (enableRuntimeScheduler_ && !runtimeScheduler_) { auto runtimeExecutor = instance_->getRuntimeExecutor(); auto runtimeScheduler = std::make_shared(runtimeExecutor); diff --git a/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h b/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h index cd242903fab..fe635bf4825 100644 --- a/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h +++ b/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h @@ -37,7 +37,10 @@ class CatalystInstanceImpl : public jni::HybridClass { static constexpr auto kJavaDescriptor = "Lcom/facebook/react/bridge/CatalystInstanceImpl;"; - static jni::local_ref initHybrid(jni::alias_ref); + static jni::local_ref initHybrid( + jni::alias_ref, + bool enableRuntimeScheduler, + bool enableRuntimeSchedulerInTurboModule); static void registerNatives(); @@ -48,7 +51,9 @@ class CatalystInstanceImpl : public jni::HybridClass { private: friend HybridBase; - CatalystInstanceImpl(); + CatalystInstanceImpl( + bool enableRuntimeScheduler, + bool enableRuntimeSchedulerInTurboModule); void initializeBridge( jni::alias_ref callback, @@ -115,6 +120,9 @@ class CatalystInstanceImpl : public jni::HybridClass { jni::global_ref nativeCallInvokerHolder_; jni::global_ref runtimeExecutor_; jni::global_ref runtimeScheduler_; + + bool const enableRuntimeScheduler_; + bool const enableRuntimeSchedulerInTurboModule_; }; } // namespace react