From 8420ecc2a23debecb33747f2707812be82ce3c6d Mon Sep 17 00:00:00 2001 From: David Vacca Date: Mon, 23 Mar 2026 16:25:01 -0700 Subject: [PATCH] Remove JavaMethodWrapper and decouple JavaModuleWrapper from legacy invocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: JavaMethodWrapper is a legacy architecture class that wraps ReactMethod-annotated Java methods via reflection for the bridge-based NativeModule invocation path. When ReactNativeFeatureFlags.useTurboModuleInterop is enabled (the new architecture), this class is completely bypassed — the TurboModule interop layer uses JavaInteropTurboModule (C++) with direct JNI invocation via JavaTurboModule::invokeJavaMethod(), which converts JSI values to JNI arguments directly in C++ without any Java-side reflection. This diff: - Deletes JavaMethodWrapper.kt (only instantiated from JavaModuleWrapper.findMethods()) - Deletes BaseJavaModuleTest.kt (test that exercised JavaMethodWrapper) - Removes the NativeMethod interface from JavaModuleWrapper (only implemented by JavaMethodWrapper) - Rewrites JavaModuleWrapper.findMethods() to compute method type inline without JavaMethodWrapper - Changes JavaModuleWrapper.invoke() to throw UnsupportedOperationException - Sets md.signature to empty string for sync methods to prevent C++ null deref The JavaModuleWrapper class shell is kept because it is still referenced from C++ JNI (JavaModuleWrapper.cpp) and NativeModuleRegistry. Full removal is planned as follow-up. Reviewed By: cortinico, javache Differential Revision: D97387121 --- .../react/bridge/JavaMethodWrapper.kt | 442 ------------------ .../react/bridge/JavaModuleWrapper.kt | 38 +- .../react/bridge/BaseJavaModuleTest.kt | 103 ---- 3 files changed, 15 insertions(+), 568 deletions(-) delete mode 100644 packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaMethodWrapper.kt delete mode 100644 packages/react-native/ReactAndroid/src/test/java/com/facebook/react/bridge/BaseJavaModuleTest.kt diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaMethodWrapper.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaMethodWrapper.kt deleted file mode 100644 index 139c93acbfcd..000000000000 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaMethodWrapper.kt +++ /dev/null @@ -1,442 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -package com.facebook.react.bridge - -import com.facebook.debug.holder.PrinterHolder -import com.facebook.debug.tags.ReactDebugOverlayTags -import com.facebook.react.common.annotations.internal.LegacyArchitecture -import com.facebook.react.common.annotations.internal.LegacyArchitectureLogLevel -import com.facebook.react.common.annotations.internal.LegacyArchitectureLogger -import com.facebook.systrace.Systrace.TRACE_TAG_REACT -import com.facebook.systrace.SystraceMessage -import java.lang.reflect.InvocationTargetException -import java.lang.reflect.Method - -@Deprecated( - message = "This class is part of Legacy Architecture and will be removed in a future release", - level = DeprecationLevel.WARNING, -) -@LegacyArchitecture(logLevel = LegacyArchitectureLogLevel.ERROR) -internal class JavaMethodWrapper( - private val moduleWrapper: JavaModuleWrapper, - val method: Method, - isSync: Boolean, -) : JavaModuleWrapper.NativeMethod { - private abstract class ArgumentExtractor { - open fun getJSArgumentsNeeded(): Int = 1 - - @Suppress("DEPRECATION") - abstract fun extractArgument( - jsInstance: JSInstance, - jsArguments: ReadableArray, - atIndex: Int, - ): T? - } - - private val parameterTypes: Array> - private val paramLength: Int - - /** - * Determines how the method is exported in JavaScript: METHOD_TYPE_ASYNC for regular methods - * METHOD_TYPE_PROMISE for methods that return a promise object to the caller. METHOD_TYPE_SYNC - * for sync methods - */ - override var type: String = BaseJavaModule.METHOD_TYPE_ASYNC - private var argumentsProcessed = false - private var argumentExtractors: Array>? = null - private var internalSignature: String? = null - private var arguments: Array? = null - private var jsArgumentsNeeded = 0 - - val signature: String? - get() { - if (!argumentsProcessed) { - processArguments() - } - return checkNotNull(internalSignature) - } - - init { - method.isAccessible = true - parameterTypes = method.parameterTypes - paramLength = parameterTypes.size - - if (isSync) { - type = BaseJavaModule.METHOD_TYPE_SYNC - } else if (paramLength > 0 && (parameterTypes[paramLength - 1] == Promise::class.java)) { - type = BaseJavaModule.METHOD_TYPE_PROMISE - } - } - - private fun processArguments() { - if (argumentsProcessed) { - return - } - SystraceMessage.beginSection(TRACE_TAG_REACT, "processArguments") - .arg("method", moduleWrapper.name + "." + method.name) - .flush() - try { - argumentsProcessed = true - argumentExtractors = buildArgumentExtractors(parameterTypes) - internalSignature = - buildSignature(method, parameterTypes, (type == BaseJavaModule.METHOD_TYPE_SYNC)) - // Since native methods are invoked from a message queue executed on a single thread, it is - // safe to allocate only one arguments object per method that can be reused across calls - arguments = arrayOfNulls(parameterTypes.size) - jsArgumentsNeeded = calculateJSArgumentsNeeded() - } finally { - SystraceMessage.endSection(TRACE_TAG_REACT).flush() - } - } - - private fun buildSignature(method: Method, paramTypes: Array>, isSync: Boolean): String = - buildString(paramTypes.size + 2) { - if (isSync) { - append(returnTypeToChar(method.returnType)) - append('.') - } else { - append("v.") - } - - for (i in paramTypes.indices) { - val paramClass = paramTypes[i] - if (paramClass == Promise::class.java) { - check(i == paramTypes.size - 1) { "Promise must be used as last parameter only" } - } - append(paramTypeToChar(paramClass)) - } - } - - private fun buildArgumentExtractors(paramTypes: Array>): Array> { - val argumentExtractors = arrayOfNulls>(paramTypes.size) - var i = 0 - while (i < paramTypes.size) { - val argumentClass = paramTypes[i] - val extractor: ArgumentExtractor<*> = - when (argumentClass) { - Boolean::class.javaObjectType, - Boolean::class.javaPrimitiveType -> ARGUMENT_EXTRACTOR_BOOLEAN - Int::class.javaObjectType, - Int::class.javaPrimitiveType -> ARGUMENT_EXTRACTOR_INTEGER - Double::class.javaObjectType, - Double::class.javaPrimitiveType -> ARGUMENT_EXTRACTOR_DOUBLE - Float::class.javaObjectType, - Float::class.javaPrimitiveType -> ARGUMENT_EXTRACTOR_FLOAT - String::class.java -> ARGUMENT_EXTRACTOR_STRING - Callback::class.java -> ARGUMENT_EXTRACTOR_CALLBACK - Promise::class.java -> { - check(i == paramTypes.size - 1) { "Promise must be used as last parameter only" } - ARGUMENT_EXTRACTOR_PROMISE - } - ReadableMap::class.java -> ARGUMENT_EXTRACTOR_MAP - ReadableArray::class.java -> ARGUMENT_EXTRACTOR_ARRAY - Dynamic::class.java -> ARGUMENT_EXTRACTOR_DYNAMIC - else -> - throw RuntimeException("Got unknown argument class: ${argumentClass.simpleName}") - } - - argumentExtractors[i] = extractor - i += extractor.getJSArgumentsNeeded() - } - - return argumentExtractors.requireNoNulls() - } - - private fun calculateJSArgumentsNeeded(): Int { - var n = 0 - for (extractor in checkNotNull(argumentExtractors)) { - n += extractor.getJSArgumentsNeeded() - } - return n - } - - private fun getAffectedRange(startIndex: Int, jsArgumentsNeeded: Int): String = - if (jsArgumentsNeeded > 1) { - "$startIndex-${startIndex + jsArgumentsNeeded - 1}" - } else { - "$startIndex" - } - - @Suppress("DEPRECATION") - override fun invoke(jsInstance: JSInstance, parameters: ReadableArray) { - val traceName = moduleWrapper.name + "." + method.name - SystraceMessage.beginSection(TRACE_TAG_REACT, "callJavaModuleMethod") - .arg("method", traceName) - .flush() - if (DEBUG) { - PrinterHolder.printer.logMessage( - ReactDebugOverlayTags.BRIDGE_CALLS, - "JS->Java: %s.%s()", - moduleWrapper.name, - method.name, - ) - } - try { - if (!argumentsProcessed) { - processArguments() - } - - val validatedArguments = - requireNotNull(arguments) { "processArguments failed: 'arguments' is null." } - val validatedArgumentExtractors = - requireNotNull(argumentExtractors) { - "processArguments failed: 'argumentExtractors' is null." - } - - if (jsArgumentsNeeded != parameters.size()) { - throw JSApplicationCausedNativeException( - "$traceName got ${parameters.size()} arguments, expected $jsArgumentsNeeded" - ) - } - - var i = 0 - var jsArgumentsConsumed = 0 - try { - while (i < validatedArgumentExtractors.size) { - validatedArguments[i] = - validatedArgumentExtractors[i].extractArgument( - jsInstance, - parameters, - jsArgumentsConsumed, - ) - jsArgumentsConsumed += validatedArgumentExtractors[i].getJSArgumentsNeeded() - i++ - } - } catch (e: UnexpectedNativeTypeException) { - throw JSApplicationCausedNativeException( - "${e.message} (constructing arguments for $traceName at argument index ${ - getAffectedRange( - jsArgumentsConsumed, - validatedArgumentExtractors[i].getJSArgumentsNeeded(), - ) - })", - e, - ) - } catch (e: NullPointerException) { - throw JSApplicationCausedNativeException( - "${e.message} (constructing arguments for $traceName at argument index ${ - getAffectedRange( - jsArgumentsConsumed, - validatedArgumentExtractors[i].getJSArgumentsNeeded(), - ) - })", - e, - ) - } - - try { - method.invoke(moduleWrapper.module, *validatedArguments) - } catch (e: IllegalArgumentException) { - throw RuntimeException(createInvokeExceptionMessage(traceName), e) - } catch (e: IllegalAccessException) { - throw RuntimeException(createInvokeExceptionMessage(traceName), e) - } catch (e: InvocationTargetException) { - // Exceptions thrown from native module calls end up wrapped in InvocationTargetException - // which just make traces harder to read and bump out useful information - if (e.cause is RuntimeException) { - throw (e.cause as RuntimeException) - } - throw RuntimeException(createInvokeExceptionMessage(traceName), e) - } - } finally { - SystraceMessage.endSection(TRACE_TAG_REACT).flush() - } - } - - companion object { - private val ARGUMENT_EXTRACTOR_BOOLEAN: ArgumentExtractor = - object : ArgumentExtractor() { - @Suppress("DEPRECATION") - override fun extractArgument( - jsInstance: JSInstance, - jsArguments: ReadableArray, - atIndex: Int, - ): Boolean = jsArguments.getBoolean(atIndex) - } - - private val ARGUMENT_EXTRACTOR_DOUBLE: ArgumentExtractor = - object : ArgumentExtractor() { - @Suppress("DEPRECATION") - override fun extractArgument( - jsInstance: JSInstance, - jsArguments: ReadableArray, - atIndex: Int, - ): Double = jsArguments.getDouble(atIndex) - } - - private val ARGUMENT_EXTRACTOR_FLOAT: ArgumentExtractor = - object : ArgumentExtractor() { - @Suppress("DEPRECATION") - override fun extractArgument( - jsInstance: JSInstance, - jsArguments: ReadableArray, - atIndex: Int, - ): Float = jsArguments.getDouble(atIndex).toFloat() - } - - private val ARGUMENT_EXTRACTOR_INTEGER: ArgumentExtractor = - object : ArgumentExtractor() { - @Suppress("DEPRECATION") - override fun extractArgument( - jsInstance: JSInstance, - jsArguments: ReadableArray, - atIndex: Int, - ): Int = jsArguments.getDouble(atIndex).toInt() - } - - private val ARGUMENT_EXTRACTOR_STRING: ArgumentExtractor = - object : ArgumentExtractor() { - @Suppress("DEPRECATION") - override fun extractArgument( - jsInstance: JSInstance, - jsArguments: ReadableArray, - atIndex: Int, - ): String? = jsArguments.getString(atIndex) - } - - private val ARGUMENT_EXTRACTOR_ARRAY: ArgumentExtractor = - object : ArgumentExtractor() { - @Suppress("DEPRECATION") - override fun extractArgument( - jsInstance: JSInstance, - jsArguments: ReadableArray, - atIndex: Int, - ): ReadableArray? = jsArguments.getArray(atIndex) - } - - private val ARGUMENT_EXTRACTOR_DYNAMIC: ArgumentExtractor = - object : ArgumentExtractor() { - @Suppress("DEPRECATION") - override fun extractArgument( - jsInstance: JSInstance, - jsArguments: ReadableArray, - atIndex: Int, - ): Dynamic = DynamicFromArray.create(jsArguments, atIndex) - } - - private val ARGUMENT_EXTRACTOR_MAP: ArgumentExtractor = - object : ArgumentExtractor() { - @Suppress("DEPRECATION") - override fun extractArgument( - jsInstance: JSInstance, - jsArguments: ReadableArray, - atIndex: Int, - ): ReadableMap? = jsArguments.getMap(atIndex) - } - - private val ARGUMENT_EXTRACTOR_CALLBACK: ArgumentExtractor = - object : ArgumentExtractor() { - @Suppress("DEPRECATION") - override fun extractArgument( - jsInstance: JSInstance, - jsArguments: ReadableArray, - atIndex: Int, - ): Callback? = - if (jsArguments.isNull(atIndex)) { - null - } else { - object : Callback { - private var invoked = false - - override fun invoke(vararg args: Any?) { - if (invoked) { - error( - "Illegal callback invocation from native module. This callback type only permits a single invocation from native code." - ) - } - @Suppress("UNCHECKED_CAST") - jsInstance.invokeCallback( - callbackID = jsArguments.getDouble(atIndex).toInt(), - arguments = Arguments.fromJavaArgs(args as Array), - ) - invoked = true - } - } - } - } - - private val ARGUMENT_EXTRACTOR_PROMISE: ArgumentExtractor = - object : ArgumentExtractor() { - override fun getJSArgumentsNeeded(): Int = 2 - - @Suppress("DEPRECATION") - override fun extractArgument( - jsInstance: JSInstance, - jsArguments: ReadableArray, - atIndex: Int, - ): Promise { - val resolve = - ARGUMENT_EXTRACTOR_CALLBACK.extractArgument(jsInstance, jsArguments, atIndex) - val reject = - ARGUMENT_EXTRACTOR_CALLBACK.extractArgument(jsInstance, jsArguments, atIndex + 1) - return PromiseImpl(resolve, reject) - } - } - - private val DEBUG = - PrinterHolder.printer.shouldDisplayLogMessage(ReactDebugOverlayTags.BRIDGE_CALLS) - - init { - LegacyArchitectureLogger.assertLegacyArchitecture( - "JavaMethodWrapper", - LegacyArchitectureLogLevel.ERROR, - ) - } - - private fun paramTypeToChar(paramClass: Class<*>): Char { - val tryCommon = commonTypeToChar(paramClass) - if (tryCommon != '\u0000') { - return tryCommon - } - return when (paramClass) { - Callback::class.java -> 'X' - Promise::class.java -> 'P' - ReadableMap::class.java -> 'M' - ReadableArray::class.java -> 'A' - Dynamic::class.java -> 'Y' - else -> throw RuntimeException("Got unknown param class: ${paramClass.simpleName}") - } - } - - private fun returnTypeToChar(returnClass: Class<*>): Char { - // Keep this in sync with MethodInvoker - val tryCommon = commonTypeToChar(returnClass) - if (tryCommon != '\u0000') { - return tryCommon - } - return when (returnClass) { - Void.TYPE -> 'v' - WritableMap::class.java -> 'M' - WritableArray::class.java -> 'A' - else -> throw RuntimeException("Got unknown return class: ${returnClass.simpleName}") - } - } - - private fun commonTypeToChar(typeClass: Class<*>): Char { - return when (typeClass) { - Boolean::class.javaPrimitiveType -> 'z' - Boolean::class.javaObjectType -> 'Z' - Int::class.javaPrimitiveType -> 'i' - Int::class.javaObjectType -> 'I' - Double::class.javaPrimitiveType -> 'd' - Double::class.javaObjectType -> 'D' - Float::class.javaPrimitiveType -> 'f' - Float::class.javaObjectType -> 'F' - String::class.java -> 'S' - else -> '\u0000' - } - } - - /** - * Makes it easier to determine the cause of an error invoking a native method from Javascript - * code by adding the function name. - */ - private fun createInvokeExceptionMessage(traceName: String): String = - "Could not invoke $traceName" - } -} diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaModuleWrapper.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaModuleWrapper.kt index d24e6e9a4efd..5314d04b1277 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaModuleWrapper.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaModuleWrapper.kt @@ -25,15 +25,9 @@ import java.lang.reflect.Method @DoNotStrip @InteropLegacyArchitecture internal class JavaModuleWrapper( - @Suppress("DEPRECATION") private val jsInstance: JSInstance, + @Suppress("DEPRECATION", "unused") private val jsInstance: JSInstance, private val moduleHolder: ModuleHolder, ) { - interface NativeMethod { - @Suppress("DEPRECATION") fun invoke(jsInstance: JSInstance, parameters: ReadableArray) - - val type: String - } - @DoNotStrip class MethodDescriptor { @DoNotStrip var method: Method? = null @@ -45,7 +39,6 @@ internal class JavaModuleWrapper( @DoNotStrip var type: String? = null } - private val methods = ArrayList() private val descs = ArrayList() @get:DoNotStrip @@ -63,26 +56,26 @@ internal class JavaModuleWrapper( var classForMethods: Class<*> = moduleHolder.module.javaClass val superClass = classForMethods.superclass if (superClass != null && TurboModule::class.java.isAssignableFrom(superClass)) { - // For java module that is based on generated flow-type spec, inspect the - // spec abstract class instead, which is the super class of the given Java - // module. classForMethods = superClass } val targetMethods = classForMethods.declaredMethods for (targetMethod in targetMethods) { targetMethod.getAnnotation(ReactMethod::class.java)?.let { annotation -> - val methodName = targetMethod.name val md = MethodDescriptor() - @Suppress("DEPRECATION") - val method = JavaMethodWrapper(this, targetMethod, annotation.isBlockingSynchronousMethod) - md.name = methodName - md.type = method.type + md.name = targetMethod.name + md.type = + when { + annotation.isBlockingSynchronousMethod -> BaseJavaModule.METHOD_TYPE_SYNC + targetMethod.parameterTypes.let { params -> + params.isNotEmpty() && params.last() == Promise::class.java + } -> BaseJavaModule.METHOD_TYPE_PROMISE + else -> BaseJavaModule.METHOD_TYPE_ASYNC + } if (BaseJavaModule.METHOD_TYPE_SYNC == md.type) { - md.signature = method.signature + md.signature = "" md.method = targetMethod } - methods.add(method) descs.add(md) } } @@ -128,11 +121,10 @@ internal class JavaModuleWrapper( @DoNotStrip fun invoke(methodId: Int, parameters: ReadableNativeArray) { - if (methodId >= methods.size) { - return - } - - methods[methodId].invoke(jsInstance, parameters) + throw UnsupportedOperationException( + "JavaModuleWrapper.invoke() is no longer supported. " + + "Use TurboModule interop instead (ReactNativeFeatureFlags.useTurboModuleInterop)." + ) } private companion object { diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/bridge/BaseJavaModuleTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/bridge/BaseJavaModuleTest.kt deleted file mode 100644 index 28b682bda5d0..000000000000 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/bridge/BaseJavaModuleTest.kt +++ /dev/null @@ -1,103 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -package com.facebook.react.bridge - -import com.facebook.react.turbomodule.core.interfaces.TurboModule -import com.facebook.testutils.shadows.ShadowNativeLoader -import com.facebook.testutils.shadows.ShadowSoLoader -import org.junit.Before -import org.junit.Test -import org.junit.runner.RunWith -import org.mockito.kotlin.mock -import org.mockito.kotlin.whenever -import org.robolectric.RobolectricTestRunner -import org.robolectric.annotation.Config - -/** Tests for [BaseJavaModule] and [JavaModuleWrapper] */ -@Suppress("DEPRECATION") -@Config(shadows = [ShadowSoLoader::class, ShadowNativeLoader::class]) -@RunWith(RobolectricTestRunner::class) -class BaseJavaModuleTest { - private lateinit var methods: List - private lateinit var moduleWrapper: JavaModuleWrapper - private lateinit var generatedMethods: List - private lateinit var generatedModuleWrapper: JavaModuleWrapper - private lateinit var arguments: ReadableNativeArray - - @Before - fun setup() { - val jsInstance = mock() - val moduleHolder = ModuleHolder(MethodsModule()) - moduleWrapper = JavaModuleWrapper(jsInstance, moduleHolder) - methods = moduleWrapper.methodDescriptors - val generatedModuleHolder = ModuleHolder(GeneratedMethodsModule()) - generatedModuleWrapper = JavaModuleWrapper(jsInstance, generatedModuleHolder) - generatedMethods = generatedModuleWrapper.methodDescriptors - arguments = mock() - } - - private fun findMethod(mname: String, methods: List): Int = - methods.indexOfFirst({ it.name === mname }) - - @Test(expected = JSApplicationCausedNativeException::class) - fun testCallMethodWithoutEnoughArgs() { - val methodId = findMethod("regularMethod", methods) - whenever(arguments.size()).thenReturn(1) - moduleWrapper.invoke(methodId, arguments) - } - - @Test - fun testCallMethodWithEnoughArgs() { - val methodId = findMethod("regularMethod", methods) - whenever(arguments.size()).thenReturn(2) - moduleWrapper.invoke(methodId, arguments) - } - - @Test - fun testCallAsyncMethodWithEnoughArgs() { - // Promise block evaluates to 2 args needing to be passed from JS - val methodId = findMethod("asyncMethod", methods) - whenever(arguments.size()).thenReturn(3) - moduleWrapper.invoke(methodId, arguments) - } - - @Test - fun testCallSyncMethod() { - val methodId = findMethod("syncMethod", methods) - whenever(arguments.size()).thenReturn(2) - moduleWrapper.invoke(methodId, arguments) - } - - @Test - fun testCallGeneratedMethod() { - val methodId = findMethod("generatedMethod", generatedMethods) - whenever(arguments.size()).thenReturn(2) - generatedModuleWrapper.invoke(methodId, arguments) - } - - @Suppress("UNUSED_PARAMETER") - private class MethodsModule : BaseJavaModule() { - override fun getName(): String = "Methods" - - @ReactMethod fun regularMethod(a: String?, b: Int?) = Unit - - @ReactMethod fun asyncMethod(a: Int, p: Promise) = Unit - - @ReactMethod(isBlockingSynchronousMethod = true) fun syncMethod(a: Int, b: Int): Int = a + b - } - - private abstract inner class NativeTestGeneratedModuleSpec : BaseJavaModule(), TurboModule { - @ReactMethod abstract fun generatedMethod(a: String?, b: Int?) - } - - private inner class GeneratedMethodsModule : NativeTestGeneratedModuleSpec() { - override fun getName(): String = "GeneratedMethods" - - override fun generatedMethod(a: String?, b: Int?) = Unit - } -}