Skip to content

Migrate BlobModule to TurboModuleWithJSIBindings for safe JSI access #56272

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
javache wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Migrate BlobModule to TurboModuleWithJSIBindings for safe JSI access #56272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/react-native/ReactAndroid/api/ReactAndroid.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2447,16 +2447,16 @@ public abstract interface class com/facebook/react/modules/appregistry/AppRegist
public abstract fun unmountApplicationComponentAtRootTag (I)V
}

public final class com/facebook/react/modules/blob/BlobModule : com/facebook/fbreact/specs/NativeBlobModuleSpec {
public final class com/facebook/react/modules/blob/BlobModule : com/facebook/fbreact/specs/NativeBlobModuleSpec, com/facebook/react/turbomodule/core/interfaces/TurboModuleWithJSIBindings {
public static final field Companion Lcom/facebook/react/modules/blob/BlobModule$Companion;
public static final field NAME Ljava/lang/String;
public fun <init> (Lcom/facebook/react/bridge/ReactApplicationContext;)V
public fun addNetworkingHandler ()V
public fun addWebSocketHandler (D)V
public fun createFromParts (Lcom/facebook/react/bridge/ReadableArray;Ljava/lang/String;)V
public fun getBindingsInstaller ()Lcom/facebook/react/turbomodule/core/interfaces/BindingsInstallerHolder;
public final fun getLengthOfBlob (Ljava/lang/String;)J
public fun getTypedExportedConstants ()Ljava/util/Map;
public fun initialize ()V
public fun release (Ljava/lang/String;)V
public final fun remove (Ljava/lang/String;)V
public fun removeWebSocketHandler (D)V
Expand Down
31 changes: 0 additions & 31 deletions .../react-native/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobCollector.kt
View file
Open in desktop

This file was deleted.

15 changes: 10 additions & 5 deletions ...ges/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ import com.facebook.react.bridge.buildReadableMap
import com.facebook.react.module.annotations.ReactModule
import com.facebook.react.modules.network.NetworkingModule
import com.facebook.react.modules.websocket.WebSocketModule
import com.facebook.react.turbomodule.core.interfaces.BindingsInstallerHolder
import com.facebook.react.turbomodule.core.interfaces.TurboModuleWithJSIBindings
import com.facebook.soloader.SoLoader
import java.io.ByteArrayOutputStream
import java.io.File
import java.io.FileNotFoundException
Expand All @@ -39,7 +42,7 @@ import okio.ByteString

@ReactModule(name = NativeBlobModuleSpec.NAME)
public class BlobModule(reactContext: ReactApplicationContext) :
NativeBlobModuleSpec(reactContext) {
NativeBlobModuleSpec(reactContext), TurboModuleWithJSIBindings {

private val blobs = HashMap<String, ByteArray>()

Expand Down Expand Up @@ -128,10 +131,6 @@ public class BlobModule(reactContext: ReactApplicationContext) :
}
}

public override fun initialize() {
BlobCollector.install(reactApplicationContext, this)
}

public override fun getTypedExportedConstants(): Map<String, Any> {
val resources = reactApplicationContext.resources
val packageName = reactApplicationContext.packageName
Expand Down Expand Up @@ -331,7 +330,13 @@ public class BlobModule(reactContext: ReactApplicationContext) :
remove(blobId)
}

@DoNotStrip external override fun getBindingsInstaller(): BindingsInstallerHolder

public companion object {
init {
SoLoader.loadLibrary("reactnativeblob")
}

public const val NAME: String = NativeBlobModuleSpec.NAME
}
}
67 changes: 37 additions & 30 deletions packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/BlobCollector.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,38 +40,45 @@ size_t BlobCollector::getBlobLength() {
return static_cast<size_t>(length);
}

void BlobCollector::nativeInstall(
jni::alias_ref<jclass> /*unused*/,
jni::alias_ref<jobject> blobModule,
jlong jsContextNativePointer) {
auto& runtime = *((jsi::Runtime*)jsContextNativePointer);
auto blobModuleRef = jni::make_global(blobModule);
runtime.global().setProperty(
runtime,
"__blobCollectorProvider",
jsi::Function::createFromHostFunction(
runtime,
jsi::PropNameID::forAscii(runtime, "__blobCollectorProvider"),
1,
[blobModuleRef](
jsi::Runtime& rt,
const jsi::Value& /*thisVal*/,
const jsi::Value* args,
size_t /*count*/) {
auto blobId = args[0].asString(rt).utf8(rt);
auto blobCollector =
std::make_shared<BlobCollector>(blobModuleRef, blobId);
auto blobCollectorJsObject =
jsi::Object::createFromHostObject(rt, blobCollector);
blobCollectorJsObject.setExternalMemoryPressure(
rt, blobCollector->getBlobLength());
return blobCollectorJsObject;
}));
// static
void BlobModuleJSIBindings::registerNatives() {
javaClassLocal()->registerNatives({
makeNativeMethod(
"getBindingsInstaller", BlobModuleJSIBindings::getBindingsInstaller),
});
}

void BlobCollector::registerNatives() {
registerHybrid(
{makeNativeMethod("nativeInstall", BlobCollector::nativeInstall)});
// static
jni::local_ref<BindingsInstallerHolder::javaobject>
BlobModuleJSIBindings::getBindingsInstaller(
jni::alias_ref<BlobModuleJSIBindings> jobj) {
auto blobModuleRef = jni::make_global(jobj);
return BindingsInstallerHolder::newObjectCxxArgs(
[blobModuleRef = std::move(blobModuleRef)](
jsi::Runtime& runtime, const std::shared_ptr<CallInvoker>&) {
runtime.global().setProperty(
runtime,
"__blobCollectorProvider",
jsi::Function::createFromHostFunction(
runtime,
jsi::PropNameID::forAscii(runtime, "__blobCollectorProvider"),
1,
[blobModuleRef](
jsi::Runtime& rt,
const jsi::Value& /*thisVal*/,
const jsi::Value* args,
size_t /*count*/) {
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
auto blobId = args[0].asString(rt).utf8(rt);
auto blobCollector =
std::make_shared<BlobCollector>(blobModuleRef, blobId);
auto blobCollectorJsObject =
jsi::Object::createFromHostObject(rt, blobCollector);
blobCollectorJsObject.setExternalMemoryPressure(
rt, blobCollector->getBlobLength());
return blobCollectorJsObject;
}));
});
}

} // namespace facebook::react
19 changes: 11 additions & 8 deletions packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/BlobCollector.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,33 @@

#pragma once

#include <ReactCommon/BindingsInstallerHolder.h>
#include <fbjni/fbjni.h>
#include <jsi/jsi.h>

namespace facebook::react {

class BlobCollector : public jni::HybridClass<BlobCollector>, public jsi::HostObject {
class BlobCollector : public jsi::HostObject {
public:
BlobCollector(jni::global_ref<jobject> blobModule, std::string blobId);
~BlobCollector();

size_t getBlobLength();

static constexpr auto kJavaDescriptor = "Lcom/facebook/react/modules/blob/BlobCollector;";
private:
jni::global_ref<jobject> blobModule_;
const std::string blobId_;
};

static void
nativeInstall(jni::alias_ref<jclass> /*unused*/, jni::alias_ref<jobject> blobModule, jlong jsContextNativePointer);
class BlobModuleJSIBindings : public jni::JavaClass<BlobModuleJSIBindings> {
public:
static constexpr const char *kJavaDescriptor = "Lcom/facebook/react/modules/blob/BlobModule;";

static void registerNatives();

private:
friend HybridBase;

jni::global_ref<jobject> blobModule_;
const std::string blobId_;
static jni::local_ref<BindingsInstallerHolder::javaobject> getBindingsInstaller(
jni::alias_ref<BlobModuleJSIBindings> jobj);
};

} // namespace facebook::react
3 changes: 2 additions & 1 deletion packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ target_link_libraries(reactnativeblob
fbjni
folly_runtime
jsi
reactnativejni)
reactnativejni
turbomodulejsijni)

target_compile_reactnative_options(reactnativeblob PRIVATE)
target_compile_options(reactnativeblob PRIVATE -fvisibility=hidden)
2 changes: 1 addition & 1 deletion packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/OnLoad.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@

JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM* vm, void* /*unused*/) {
return facebook::jni::initialize(
vm, [] { facebook::react::BlobCollector::registerNatives(); });
vm, [] { facebook::react::BlobModuleJSIBindings::registerNatives(); });
}
66 changes: 0 additions & 66 deletions ...ct-native/ReactAndroid/src/test/java/com/facebook/react/modules/blob/BlobCollectorTest.kt
View file
Open in desktop

This file was deleted.

17 changes: 17 additions & 0 deletions ...react-native/ReactAndroid/src/test/java/com/facebook/react/modules/blob/BlobModuleTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,21 @@ import com.facebook.react.bridge.JavaOnlyArray
import com.facebook.react.bridge.JavaOnlyMap
import com.facebook.react.bridge.ReactApplicationContext
import com.facebook.react.bridge.ReactTestHelper
import com.facebook.react.turbomodule.core.interfaces.TurboModuleWithJSIBindings
import com.facebook.soloader.SoLoader
import com.facebook.testutils.shadows.ShadowArguments
import java.io.ByteArrayInputStream
import java.nio.ByteBuffer
import java.util.UUID
import kotlin.random.Random
import org.assertj.core.api.Assertions.assertThat
import org.junit.After
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.MockedStatic
import org.mockito.Mockito.mockStatic
import org.robolectric.RobolectricTestRunner
import org.robolectric.Shadows.shadowOf
import org.robolectric.annotation.Config
Expand All @@ -33,9 +38,15 @@ class BlobModuleTest {
private lateinit var blobId: String
private lateinit var context: ReactApplicationContext
private lateinit var blobModule: BlobModule
private lateinit var mockedStaticSoLoader: MockedStatic<SoLoader>

@Before
fun prepareModules() {
mockedStaticSoLoader = mockStatic(SoLoader::class.java)
mockedStaticSoLoader
.`when`<Boolean> { SoLoader.loadLibrary("reactnativeblob") }
.thenReturn(true)

bytes = ByteArray(120)
Random.Default.nextBytes(bytes)

Expand All @@ -47,6 +58,7 @@ class BlobModuleTest {
@After
fun cleanUp() {
blobModule.remove(blobId)
mockedStaticSoLoader.close()
}

@Test
Expand Down Expand Up @@ -214,4 +226,9 @@ class BlobModuleTest {
assertThat(blob.getInt("size")).isEqualTo(testData.size)
assertThat(blob.getString("blobId")).isNotEmpty()
}

@Test
fun testBlobModuleImplementsTurboModuleWithJSIBindings() {
assertTrue(blobModule is TurboModuleWithJSIBindings)
}
}
Loading