clean up enable_sync_void_methods_android (#45683)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/45683

Changelog: [Internal]

this did not make any meaningful progress, let's clean it up and revisit it later.

Reviewed By: fkgozali

Differential Revision: D60219828

fbshipit-source-id: 89a283d7c572dfcd6ef16472e81f3dce1c2cd284
This commit is contained in:
Phillip Pan
2024-07-26 13:31:47 -07:00
committed by Facebook GitHub Bot
parent 44d38c9d4d
commit a694e4b325
9 changed files with 26 additions and 77 deletions
@@ -361,7 +361,6 @@ public abstract class com/facebook/react/ReactPackageTurboModuleManagerDelegate
public fun getEagerInitModuleNames ()Ljava/util/List;
public fun getLegacyModule (Ljava/lang/String;)Lcom/facebook/react/bridge/NativeModule;
public fun getModule (Ljava/lang/String;)Lcom/facebook/react/turbomodule/core/interfaces/TurboModule;
public fun unstable_enableSyncVoidMethods ()Z
public fun unstable_isLegacyModuleRegistered (Ljava/lang/String;)Z
public fun unstable_isModuleRegistered (Ljava/lang/String;)Z
public fun unstable_shouldEnableLegacyModuleInterop ()Z
@@ -1970,7 +1969,6 @@ public class com/facebook/react/config/ReactFeatureFlags {
public static field enableFabricRenderer Z
public static field enableViewRecycling Z
public static field traceTurboModulePromiseRejections Z
public static field unstable_enableTurboModuleSyncVoidMethods Z
public static field unstable_useFabricInterop Z
public static field unstable_useTurboModuleInterop Z
public static field useTurboModules Z
@@ -39,9 +39,6 @@ public abstract class ReactPackageTurboModuleManagerDelegate extends TurboModule
ReactFeatureFlags.enableBridgelessArchitecture
&& ReactFeatureFlags.unstable_useTurboModuleInterop;
private final boolean mEnableTurboModuleSyncVoidMethods =
ReactFeatureFlags.unstable_enableTurboModuleSyncVoidMethods;
// Lazy Props
private List<ReactPackage> mPackages;
private ReactApplicationContext mReactContext;
@@ -143,10 +140,6 @@ public abstract class ReactPackageTurboModuleManagerDelegate extends TurboModule
return mShouldEnableLegacyModuleInterop;
}
public boolean unstable_enableSyncVoidMethods() {
return mEnableTurboModuleSyncVoidMethods;
}
@Nullable
@Override
public TurboModule getModule(String moduleName) {
@@ -32,12 +32,6 @@ public class ReactFeatureFlags {
/** In Bridgeless mode, should legacy NativeModules use the TurboModule system? */
public static volatile boolean unstable_useTurboModuleInterop = false;
/**
* By default, native module methods that return void run asynchronously. This flag will make
* execution of void methods in TurboModules stay on the JS thread.
*/
public static volatile boolean unstable_enableTurboModuleSyncVoidMethods = false;
/**
* Should this application use the new (Fabric) Renderer? If yes, all rendering in this app will
* use Fabric instead of the legacy renderer.
@@ -73,7 +73,7 @@ public class TurboModuleManager implements TurboModuleRegistry {
(CallInvokerHolderImpl) jsCallInvokerHolder,
(NativeMethodCallInvokerHolderImpl) nativeMethodCallInvokerHolder,
delegate);
installJSIBindings(shouldEnableLegacyModuleInterop(), enableSyncVoidMethods());
installJSIBindings(shouldEnableLegacyModuleInterop());
mEagerInitModuleNames =
delegate == null ? Collections.emptyList() : delegate.getEagerInitModuleNames();
@@ -113,10 +113,6 @@ public class TurboModuleManager implements TurboModuleRegistry {
return mDelegate != null && mDelegate.unstable_shouldEnableLegacyModuleInterop();
}
private boolean enableSyncVoidMethods() {
return mDelegate != null && mDelegate.unstable_enableSyncVoidMethods();
}
@Override
@NonNull
public List<String> getEagerInitModuleNames() {
@@ -389,8 +385,7 @@ public class TurboModuleManager implements TurboModuleRegistry {
NativeMethodCallInvokerHolderImpl nativeMethodCallInvoker,
TurboModuleManagerDelegate tmmDelegate);
private native void installJSIBindings(
boolean shouldCreateLegacyModules, boolean enableSyncVoidMethods);
private native void installJSIBindings(boolean shouldCreateLegacyModules);
@Override
public void invalidate() {
@@ -70,11 +70,6 @@ public abstract class TurboModuleManagerDelegate {
return false;
}
/* Can TurboModule methods that return void execute on the JS thread? */
public boolean unstable_enableSyncVoidMethods() {
return false;
}
// TODO(T171231381): Consider removing this method: could we just use the static initializer
// of derived classes instead?
protected synchronized void maybeLoadOtherSoLibraries() {}
@@ -128,31 +128,27 @@ void TurboModuleManager::registerNatives() {
TurboModuleProviderFunctionType TurboModuleManager::createTurboModuleProvider(
jni::alias_ref<jhybridobject> javaPart,
jsi::Runtime* runtime,
bool enableSyncVoidMethods) {
return
[runtime, weakJavaPart = jni::make_weak(javaPart), enableSyncVoidMethods](
const std::string& name) -> std::shared_ptr<TurboModule> {
auto javaPart = weakJavaPart.lockLocal();
if (!javaPart) {
return nullptr;
}
jsi::Runtime* runtime) {
return [runtime, weakJavaPart = jni::make_weak(javaPart)](
const std::string& name) -> std::shared_ptr<TurboModule> {
auto javaPart = weakJavaPart.lockLocal();
if (!javaPart) {
return nullptr;
}
auto cxxPart = javaPart->cthis();
if (cxxPart == nullptr) {
return nullptr;
}
auto cxxPart = javaPart->cthis();
if (cxxPart == nullptr) {
return nullptr;
}
return cxxPart->getTurboModule(
javaPart, name, *runtime, enableSyncVoidMethods);
};
return cxxPart->getTurboModule(javaPart, name, *runtime);
};
}
std::shared_ptr<TurboModule> TurboModuleManager::getTurboModule(
jni::alias_ref<jhybridobject> javaPart,
const std::string& name,
jsi::Runtime& runtime,
bool enableSyncVoidMethods) {
jsi::Runtime& runtime) {
const char* moduleName = name.c_str();
TurboModulePerfLogger::moduleJSRequireBeginningStart(moduleName);
@@ -192,8 +188,7 @@ std::shared_ptr<TurboModule> TurboModuleManager::getTurboModule(
.moduleName = name,
.instance = moduleInstance,
.jsInvoker = jsCallInvoker_,
.nativeMethodCallInvoker = nativeMethodCallInvoker_,
.shouldVoidMethodsExecuteSync = enableSyncVoidMethods};
.nativeMethodCallInvoker = nativeMethodCallInvoker_};
auto turboModule = cxxDelegate->getTurboModule(name, params);
if (moduleInstance->isInstanceOf(
@@ -294,8 +289,7 @@ std::shared_ptr<TurboModule> TurboModuleManager::getLegacyModule(
.moduleName = name,
.instance = moduleInstance,
.jsInvoker = jsCallInvoker_,
.nativeMethodCallInvoker = nativeMethodCallInvoker_,
.shouldVoidMethodsExecuteSync = false};
.nativeMethodCallInvoker = nativeMethodCallInvoker_};
static auto getMethodDescriptorsFromModule =
javaPart->getClass()
@@ -326,8 +320,7 @@ std::shared_ptr<TurboModule> TurboModuleManager::getLegacyModule(
void TurboModuleManager::installJSIBindings(
jni::alias_ref<jhybridobject> javaPart,
bool shouldCreateLegacyModules,
bool enableSyncVoidMethods) {
bool shouldCreateLegacyModules) {
auto cxxPart = javaPart->cthis();
if (cxxPart == nullptr || !cxxPart->jsCallInvoker_) {
return; // Runtime doesn't exist when attached to Chrome debugger.
@@ -335,12 +328,10 @@ void TurboModuleManager::installJSIBindings(
cxxPart->runtimeExecutor_([cxxPart,
javaPart = jni::make_global(javaPart),
shouldCreateLegacyModules,
enableSyncVoidMethods](jsi::Runtime& runtime) {
shouldCreateLegacyModules](jsi::Runtime& runtime) {
TurboModuleBinding::install(
runtime,
cxxPart->createTurboModuleProvider(
javaPart, &runtime, enableSyncVoidMethods),
cxxPart->createTurboModuleProvider(javaPart, &runtime),
shouldCreateLegacyModules
? cxxPart->createLegacyModuleProvider(javaPart)
: nullptr);
@@ -62,18 +62,15 @@ class TurboModuleManager : public jni::HybridClass<TurboModuleManager> {
static void installJSIBindings(
jni::alias_ref<jhybridobject> javaPart,
bool shouldCreateLegacyModules,
bool enableSyncVoidMethods);
bool shouldCreateLegacyModules);
static TurboModuleProviderFunctionType createTurboModuleProvider(
jni::alias_ref<jhybridobject> javaPart,
jsi::Runtime* runtime,
bool enableSyncVoidMethods);
jsi::Runtime* runtime);
std::shared_ptr<TurboModule> getTurboModule(
jni::alias_ref<jhybridobject> javaPart,
const std::string& name,
jsi::Runtime& runtime,
bool enableSyncVoidMethods);
jsi::Runtime& runtime);
static TurboModuleProviderFunctionType createLegacyModuleProvider(
jni::alias_ref<jhybridobject> javaPart);
@@ -33,8 +33,7 @@ namespace TMPL = TurboModulePerfLogger;
JavaTurboModule::JavaTurboModule(const InitParams& params)
: TurboModule(params.moduleName, params.jsInvoker),
instance_(jni::make_global(params.instance)),
nativeMethodCallInvoker_(params.nativeMethodCallInvoker),
shouldVoidMethodsExecuteSync_(params.shouldVoidMethodsExecuteSync) {}
nativeMethodCallInvoker_(params.nativeMethodCallInvoker) {}
JavaTurboModule::~JavaTurboModule() {
/**
@@ -531,9 +530,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod(
const char* methodName = methodNameStr.c_str();
const char* moduleName = name_.c_str();
bool isMethodSync =
(valueKind == VoidKind && shouldVoidMethodsExecuteSync_) ||
!(valueKind == VoidKind || valueKind == PromiseKind);
bool isMethodSync = !(valueKind == VoidKind || valueKind == PromiseKind);
if (isMethodSync) {
TMPL::syncMethodCallStart(moduleName, methodName);
@@ -813,15 +810,6 @@ jsi::Value JavaTurboModule::invokeJavaMethod(
return returnValue;
}
case VoidKind: {
if (shouldVoidMethodsExecuteSync_) {
env->CallVoidMethodA(instance, methodID, jargs.data());
checkJNIErrorForMethodCall();
TMPL::syncMethodCallExecutionEnd(moduleName, methodName);
TMPL::syncMethodCallEnd(moduleName, methodName);
return jsi::Value::undefined();
}
TMPL::asyncMethodCallArgConversionEnd(moduleName, methodName);
TMPL::asyncMethodCallDispatch(moduleName, methodName);
@@ -36,7 +36,6 @@ class JSI_EXPORT JavaTurboModule : public TurboModule {
jni::alias_ref<jobject> instance;
std::shared_ptr<CallInvoker> jsInvoker;
std::shared_ptr<NativeMethodCallInvoker> nativeMethodCallInvoker;
bool shouldVoidMethodsExecuteSync;
};
JavaTurboModule(const InitParams& params);
@@ -57,7 +56,6 @@ class JSI_EXPORT JavaTurboModule : public TurboModule {
// instance_ can be of type JTurboModule, or JNativeModule
jni::global_ref<jobject> instance_;
std::shared_ptr<NativeMethodCallInvoker> nativeMethodCallInvoker_;
bool shouldVoidMethodsExecuteSync_;
};
} // namespace facebook::react