From ea29ae1ceb3845e8116e2e9cf9399ab38523b777 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Fri, 17 Jun 2022 04:34:59 -0700 Subject: [PATCH] SurfaceRegistryBinding: Display RedBox when RN$SurfaceRegistry isn't available Summary: SurfaceRegistryBinding::startSurface [checks whether global.RN$SurfaceRegistry binding is installed](https://www.internalfb.com/code/fbsource/[7040bef7d4fe43298c5f9dc1fef10f47ec396e79]/xplat/js/react-native-github/ReactCommon/react/renderer/uimanager/SurfaceRegistryBinding.cpp?lines=28-29). If not, control flow goes directly into the bridge path: [callFunctionOnModule](https://www.internalfb.com/code/fbsource/[7040bef7d4fe43298c5f9dc1fef10f47ec396e79]/xplat/js/react-native-github/ReactCommon/react/renderer/uimanager/SurfaceRegistryBinding.cpp?lines=40-46). callMethodOfModule [react_native_asserts, if the requested callable JS module isn't available](https://www.internalfb.com/code/fbsource/[7040bef7d4fe43298c5f9dc1fef10f47ec396e79]/xplat/js/react-native-github/ReactCommon/react/renderer/uimanager/bindingUtils.cpp?lines=29) on the batched bridge. This crashes the app. We could make this error experience better: 1. We shouldn't crash the app. 2. We should fail fast, and produce an error message that is more descriptive of the actual cause of the error. 3. We can leverage the RedBox infra to display this error on the screen. ## Fixes This diff modifies the gating inside SurfaceRegistryBinding. Now, in bridgeless mode, if global.RN$SurfaceRegistry isn't instaled, we'll display a RedBox instead, that shows what the error is. Changelog: [Internal] Reviewed By: sshic Differential Revision: D37223640 fbshipit-source-id: 8fbf57f5d9cf359046dc94f0a5f7d66624caee4e --- .../uimanager/SurfaceRegistryBinding.cpp | 36 ++++++++++++++----- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/ReactCommon/react/renderer/uimanager/SurfaceRegistryBinding.cpp b/ReactCommon/react/renderer/uimanager/SurfaceRegistryBinding.cpp index d193de25c87..93c4c4885a2 100644 --- a/ReactCommon/react/renderer/uimanager/SurfaceRegistryBinding.cpp +++ b/ReactCommon/react/renderer/uimanager/SurfaceRegistryBinding.cpp @@ -25,11 +25,19 @@ void SurfaceRegistryBinding::startSurface( parameters["initialProps"] = initalProps; parameters["fabric"] = true; - if (runtime.global().hasProperty(runtime, "RN$SurfaceRegistry")) { - auto registry = - runtime.global().getPropertyAsObject(runtime, "RN$SurfaceRegistry"); - auto method = registry.getPropertyAsFunction(runtime, "renderSurface"); + auto global = runtime.global(); + auto isBridgeless = global.hasProperty(runtime, "RN$Bridgeless") && + global.getProperty(runtime, "RN$Bridgeless").asBool(); + if (isBridgeless) { + if (!global.hasProperty(runtime, "RN$SurfaceRegistry")) { + throw std::runtime_error( + "SurfaceRegistryBinding::startSurface: Failed to start Surface \"" + + moduleName + "\". global.RN$SurfaceRegistry was not installed."); + } + + auto registry = global.getPropertyAsObject(runtime, "RN$SurfaceRegistry"); + auto method = registry.getPropertyAsFunction(runtime, "renderSurface"); method.call( runtime, {jsi::String::createFromUtf8(runtime, moduleName), @@ -58,9 +66,18 @@ void SurfaceRegistryBinding::setSurfaceProps( parameters["initialProps"] = initalProps; parameters["fabric"] = true; - if (runtime.global().hasProperty(runtime, "RN$SurfaceRegistry")) { - auto registry = - runtime.global().getPropertyAsObject(runtime, "RN$SurfaceRegistry"); + auto global = runtime.global(); + auto isBridgeless = global.hasProperty(runtime, "RN$Bridgeless") && + global.getProperty(runtime, "RN$Bridgeless").asBool(); + + if (isBridgeless) { + if (!global.hasProperty(runtime, "RN$SurfaceRegistry")) { + throw std::runtime_error( + "SurfaceRegistryBinding::setSurfaceProps: Failed to set Surface props for \"" + + moduleName + "\". global.RN$SurfaceRegistry was not installed."); + } + + auto registry = global.getPropertyAsObject(runtime, "RN$SurfaceRegistry"); auto method = registry.getPropertyAsFunction(runtime, "setSurfaceProps"); method.call( @@ -83,7 +100,10 @@ void SurfaceRegistryBinding::stopSurface( jsi::Runtime &runtime, SurfaceId surfaceId) { auto global = runtime.global(); - if (global.hasProperty(runtime, "RN$Bridgeless")) { + auto isBridgeless = global.hasProperty(runtime, "RN$Bridgeless") && + global.getProperty(runtime, "RN$Bridgeless").asBool(); + + if (isBridgeless) { if (!global.hasProperty(runtime, "RN$stopSurface")) { // ReactFabric module has not been loaded yet; there's no surface to stop. return;