Fabric: Marking all JS function lambdas noexcept in UIManagerBinding

Summary: Exceptions in C++ work quite differently from exceptions in other languages. To make exceptions actually work **correctly** all the code needs to be written with "exceptions in mind" (e.g., see https://www.stroustrup.com/except.pdf). In short, if the code is not "exceptions ready", throwing an exception causes memory leaks, dangling pointers, and invariant violations all over the place, which will probably cause another crashes down the road (which will be especially hard to investigate and attribute to the original issue). Fabric Core (Layout, Props parsing, ShadowNodes management, and so on) does not use exceptions because in most (all?) the cases the exception is now recoverable. So, if a program detects some internal state invariant violation or missing some resource, *logically* it's fatal. We also don't want to pay code-size and performance tax for exception support, so that's why we don't use them. It's just not the right fit for Fabric Core. This does not mean that exceptions don't happen though. C++ standard library can throw them... sometimes. And if our library is compiled with exceptions enabled (still the case, unfortunately), an exception can bubble to JavaScript code and losing all context down the road. And it's hard to investigate such crashes. To isolate those occasional exceptions inside C++ core we are marking all C++/JS boundaries with `noexcept` that stops the bubbling. I hope that will give us much more informative crash reports. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D23787492 fbshipit-source-id: 0822dbf36fc680c15b02b5cd0f2d87328296b642
facebook · Sep 19, 2020 · 57dd48b · 57dd48b
1 parent 1ddc654
commit 57dd48b
Show file tree

Hide file tree

Showing 9 changed files with 93 additions and 93 deletions.
diff --git a/React/Fabric/RCTSurfacePresenterBridgeAdapter.mm b/React/Fabric/RCTSurfacePresenterBridgeAdapter.mm
@@ -48,7 +48,7 @@ static RuntimeExecutor RCTRuntimeExecutorFromBridge(RCTBridge *bridge)
   auto bridgeWeakWrapper = wrapManagedObjectWeakly([bridge batchedBridge] ?: bridge);
 
   RuntimeExecutor runtimeExecutor = [bridgeWeakWrapper](
-                                        std::function<void(facebook::jsi::Runtime & runtime)> &&callback) {
+                                        std::function<void(facebook::jsi::Runtime &runtime)> &&callback) {
     RCTBridge *bridge = unwrapManagedObjectWeakly(bridgeWeakWrapper);
 
     RCTAssert(bridge, @"RCTRuntimeExecutorFromBridge: Bridge must not be nil at the moment of scheduling a call.");

diff --git a/ReactCommon/react/renderer/core/EventTarget.cpp b/ReactCommon/react/renderer/core/EventTarget.cpp
@@ -14,7 +14,7 @@ using Tag = EventTarget::Tag;
 
 EventTarget::EventTarget(
     jsi::Runtime &runtime,
-    const jsi::Value &instanceHandle,
+    jsi::Value const &instanceHandle,
     Tag tag)
     : weakInstanceHandle_(
           jsi::WeakObject(runtime, instanceHandle.asObject(runtime))),

diff --git a/ReactCommon/react/renderer/core/EventTarget.h b/ReactCommon/react/renderer/core/EventTarget.h
@@ -35,7 +35,7 @@ class EventTarget {
   /*
    * Constructs an EventTarget from a weak instance handler and a tag.
    */
-  EventTarget(jsi::Runtime &runtime, const jsi::Value &instanceHandle, Tag tag);
+  EventTarget(jsi::Runtime &runtime, jsi::Value const &instanceHandle, Tag tag);
 
   /*
    * Sets the `enabled` flag that allows creating a strong instance handle from

diff --git a/ReactCommon/react/renderer/uimanager/UIManager.cpp b/ReactCommon/react/renderer/uimanager/UIManager.cpp
@@ -311,8 +311,8 @@ void UIManager::dispatchCommand(
 void UIManager::configureNextLayoutAnimation(
     jsi::Runtime &runtime,
     RawValue const &config,
-    const jsi::Value &successCallback,
-    const jsi::Value &failureCallback) const {
+    jsi::Value const &successCallback,
+    jsi::Value const &failureCallback) const {
   if (animationDelegate_) {
     animationDelegate_->uiManagerDidConfigureNextLayoutAnimation(
         runtime,

diff --git a/ReactCommon/react/renderer/uimanager/UIManager.h b/ReactCommon/react/renderer/uimanager/UIManager.h
@@ -147,8 +147,8 @@ class UIManager final : public ShadowTreeDelegate {
   void configureNextLayoutAnimation(
       jsi::Runtime &runtime,
       RawValue const &config,
-      const jsi::Value &successCallback,
-      const jsi::Value &failureCallback) const;
+      jsi::Value const &successCallback,
+      jsi::Value const &failureCallback) const;
 
   ShadowTreeRegistry const &getShadowTreeRegistry() const;
 

diff --git a/ReactCommon/react/renderer/uimanager/UIManagerAnimationDelegate.h b/ReactCommon/react/renderer/uimanager/UIManagerAnimationDelegate.h
@@ -26,8 +26,8 @@ class UIManagerAnimationDelegate {
   virtual void uiManagerDidConfigureNextLayoutAnimation(
       jsi::Runtime &runtime,
       RawValue const &config,
-      const jsi::Value &successCallback,
-      const jsi::Value &failureCallback) const = 0;
+      jsi::Value const &successCallback,
+      jsi::Value const &failureCallback) const = 0;
 
   /**
    * Set ComponentDescriptor registry.

diff --git a/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp b/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp
@@ -17,7 +17,7 @@ namespace react {
 
 static jsi::Object getModule(
     jsi::Runtime &runtime,
-    const std::string &moduleName) {
+    std::string const &moduleName) {
   auto batchedBridge =
       runtime.global().getPropertyAsObject(runtime, "__fbBatchedBridge");
   auto getCallableModule =
@@ -82,8 +82,8 @@ void UIManagerBinding::attach(std::shared_ptr<UIManager> const &uiManager) {
 void UIManagerBinding::startSurface(
     jsi::Runtime &runtime,
     SurfaceId surfaceId,
-    const std::string &moduleName,
-    const folly::dynamic &initalProps) const {
+    std::string const &moduleName,
+    folly::dynamic const &initalProps) const {
   folly::dynamic parameters = folly::dynamic::object();
   parameters["rootTag"] = surfaceId;
   parameters["initialProps"] = initalProps;
@@ -128,9 +128,9 @@ void UIManagerBinding::stopSurface(jsi::Runtime &runtime, SurfaceId surfaceId)
 
 void UIManagerBinding::dispatchEvent(
     jsi::Runtime &runtime,
-    const EventTarget *eventTarget,
-    const std::string &type,
-    const ValueFactory &payloadFactory) const {
+    EventTarget const *eventTarget,
+    std::string const &type,
+    ValueFactory const &payloadFactory) const {
   SystraceSection s("UIManagerBinding::dispatchEvent");
 
   auto payload = payloadFactory(runtime);
@@ -158,7 +158,7 @@ void UIManagerBinding::dispatchEvent(
     : jsi::Value::null();
 
   auto &eventHandlerWrapper =
-      static_cast<const EventHandlerWrapper &>(*eventHandler_);
+      static_cast<EventHandlerWrapper const &>(*eventHandler_);
 
   eventHandlerWrapper.callback.call(
       runtime,
@@ -173,7 +173,7 @@ void UIManagerBinding::invalidate() const {
 
 jsi::Value UIManagerBinding::get(
     jsi::Runtime &runtime,
-    const jsi::PropNameID &name) {
+    jsi::PropNameID const &name) {
   auto methodName = name.utf8(runtime);
 
   // Convert shared_ptr<UIManager> to a raw ptr
@@ -213,9 +213,9 @@ jsi::Value UIManagerBinding::get(
         5,
         [uiManager](
             jsi::Runtime &runtime,
-            const jsi::Value &thisValue,
-            const jsi::Value *arguments,
-            size_t count) -> jsi::Value {
+            jsi::Value const &thisValue,
+            jsi::Value const *arguments,
+            size_t count) noexcept -> jsi::Value {
           return valueFromShadowNode(
               runtime,
               uiManager->createNode(
@@ -235,9 +235,9 @@ jsi::Value UIManagerBinding::get(
         1,
         [uiManager](
             jsi::Runtime &runtime,
-            const jsi::Value &thisValue,
-            const jsi::Value *arguments,
-            size_t count) -> jsi::Value {
+            jsi::Value const &thisValue,
+            jsi::Value const *arguments,
+            size_t count) noexcept -> jsi::Value {
           return valueFromShadowNode(
               runtime,
               uiManager->cloneNode(shadowNodeFromValue(runtime, arguments[0])));
@@ -251,9 +251,9 @@ jsi::Value UIManagerBinding::get(
         2,
         [uiManager](
             jsi::Runtime &runtime,
-            const jsi::Value &thisValue,
-            const jsi::Value *arguments,
-            size_t count) -> jsi::Value {
+            jsi::Value const &thisValue,
+            jsi::Value const *arguments,
+            size_t count) noexcept -> jsi::Value {
           uiManager->setJSResponder(
               shadowNodeFromValue(runtime, arguments[0]),
               arguments[1].getBool());
@@ -269,9 +269,9 @@ jsi::Value UIManagerBinding::get(
         2,
         [uiManager](
             jsi::Runtime &runtime,
-            const jsi::Value &thisValue,
-            const jsi::Value *arguments,
-            size_t count) -> jsi::Value {
+            jsi::Value const &thisValue,
+            jsi::Value const *arguments,
+            size_t count) noexcept -> jsi::Value {
           auto node = shadowNodeFromValue(runtime, arguments[0]);
           auto locationX = (Float)arguments[1].getNumber();
           auto locationY = (Float)arguments[2].getNumber();
@@ -299,9 +299,9 @@ jsi::Value UIManagerBinding::get(
         0,
         [uiManager](
             jsi::Runtime &runtime,
-            const jsi::Value &thisValue,
-            const jsi::Value *arguments,
-            size_t count) -> jsi::Value {
+            jsi::Value const &thisValue,
+            jsi::Value const *arguments,
+            size_t count) noexcept -> jsi::Value {
           uiManager->clearJSResponder();
 
           return jsi::Value::undefined();
@@ -316,9 +316,9 @@ jsi::Value UIManagerBinding::get(
         1,
         [uiManager](
             jsi::Runtime &runtime,
-            const jsi::Value &thisValue,
-            const jsi::Value *arguments,
-            size_t count) -> jsi::Value {
+            jsi::Value const &thisValue,
+            jsi::Value const *arguments,
+            size_t count) noexcept -> jsi::Value {
           return valueFromShadowNode(
               runtime,
               uiManager->cloneNode(
@@ -335,10 +335,10 @@ jsi::Value UIManagerBinding::get(
         2,
         [uiManager](
             jsi::Runtime &runtime,
-            const jsi::Value &thisValue,
-            const jsi::Value *arguments,
-            size_t count) -> jsi::Value {
-          const auto &rawProps = RawProps(runtime, arguments[1]);
+            jsi::Value const &thisValue,
+            jsi::Value const *arguments,
+            size_t count) noexcept -> jsi::Value {
+          auto const &rawProps = RawProps(runtime, arguments[1]);
           return valueFromShadowNode(
               runtime,
               uiManager->cloneNode(
@@ -356,10 +356,10 @@ jsi::Value UIManagerBinding::get(
         2,
         [uiManager](
             jsi::Runtime &runtime,
-            const jsi::Value &thisValue,
-            const jsi::Value *arguments,
-            size_t count) -> jsi::Value {
-          const auto &rawProps = RawProps(runtime, arguments[1]);
+            jsi::Value const &thisValue,
+            jsi::Value const *arguments,
+            size_t count) noexcept -> jsi::Value {
+          auto const &rawProps = RawProps(runtime, arguments[1]);
           return valueFromShadowNode(
               runtime,
               uiManager->cloneNode(
@@ -376,9 +376,9 @@ jsi::Value UIManagerBinding::get(
         2,
         [uiManager](
             jsi::Runtime &runtime,
-            const jsi::Value &thisValue,
-            const jsi::Value *arguments,
-            size_t count) -> jsi::Value {
+            jsi::Value const &thisValue,
+            jsi::Value const *arguments,
+            size_t count) noexcept -> jsi::Value {
           uiManager->appendChild(
               shadowNodeFromValue(runtime, arguments[0]),
               shadowNodeFromValue(runtime, arguments[1]));
@@ -392,9 +392,9 @@ jsi::Value UIManagerBinding::get(
         name,
         1,
         [](jsi::Runtime &runtime,
-           const jsi::Value &thisValue,
-           const jsi::Value *arguments,
-           size_t count) -> jsi::Value {
+           jsi::Value const &thisValue,
+           jsi::Value const *arguments,
+           size_t count) noexcept -> jsi::Value {
           auto shadowNodeList =
               std::make_shared<SharedShadowNodeList>(SharedShadowNodeList({}));
           return valueFromShadowNodeList(runtime, shadowNodeList);
@@ -407,9 +407,9 @@ jsi::Value UIManagerBinding::get(
         name,
         2,
         [](jsi::Runtime &runtime,
-           const jsi::Value &thisValue,
-           const jsi::Value *arguments,
-           size_t count) -> jsi::Value {
+           jsi::Value const &thisValue,
+           jsi::Value const *arguments,
+           size_t count) noexcept -> jsi::Value {
           auto shadowNodeList = shadowNodeListFromValue(runtime, arguments[0]);
           auto shadowNode = shadowNodeFromValue(runtime, arguments[1]);
           shadowNodeList->push_back(shadowNode);
@@ -429,7 +429,7 @@ jsi::Value UIManagerBinding::get(
               jsi::Runtime &runtime,
               jsi::Value const &thisValue,
               jsi::Value const *arguments,
-              size_t count) -> jsi::Value {
+              size_t count) noexcept -> jsi::Value {
             auto surfaceId = surfaceIdFromValue(runtime, arguments[0]);
             auto shadowNodeList =
                 shadowNodeListFromValue(runtime, arguments[1]);
@@ -456,7 +456,7 @@ jsi::Value UIManagerBinding::get(
               jsi::Runtime &runtime,
               jsi::Value const &thisValue,
               jsi::Value const *arguments,
-              size_t count) -> jsi::Value {
+              size_t count) noexcept -> jsi::Value {
             uiManager->completeSurface(
                 surfaceIdFromValue(runtime, arguments[0]),
                 shadowNodeListFromValue(runtime, arguments[1]));
@@ -473,9 +473,9 @@ jsi::Value UIManagerBinding::get(
         1,
         [this](
             jsi::Runtime &runtime,
-            const jsi::Value &thisValue,
-            const jsi::Value *arguments,
-            size_t count) -> jsi::Value {
+            jsi::Value const &thisValue,
+            jsi::Value const *arguments,
+            size_t count) noexcept -> jsi::Value {
           auto eventHandler =
               arguments[0].getObject(runtime).getFunction(runtime);
           eventHandler_ =
@@ -491,9 +491,9 @@ jsi::Value UIManagerBinding::get(
         2,
         [uiManager](
             jsi::Runtime &runtime,
-            const jsi::Value &thisValue,
-            const jsi::Value *arguments,
-            size_t count) -> jsi::Value {
+            jsi::Value const &thisValue,
+            jsi::Value const *arguments,
+            size_t count) noexcept -> jsi::Value {
           auto layoutMetrics = uiManager->getRelativeLayoutMetrics(
               *shadowNodeFromValue(runtime, arguments[0]),
               shadowNodeFromValue(runtime, arguments[1]).get(),
@@ -515,9 +515,9 @@ jsi::Value UIManagerBinding::get(
         3,
         [uiManager](
             jsi::Runtime &runtime,
-            const jsi::Value &thisValue,
-            const jsi::Value *arguments,
-            size_t count) -> jsi::Value {
+            jsi::Value const &thisValue,
+            jsi::Value const *arguments,
+            size_t count) noexcept -> jsi::Value {
           uiManager->dispatchCommand(
               shadowNodeFromValue(runtime, arguments[0]),
               stringFromValue(runtime, arguments[1]),
@@ -535,9 +535,9 @@ jsi::Value UIManagerBinding::get(
         4,
         [uiManager](
             jsi::Runtime &runtime,
-            const jsi::Value &thisValue,
-            const jsi::Value *arguments,
-            size_t count) -> jsi::Value {
+            jsi::Value const &thisValue,
+            jsi::Value const *arguments,
+            size_t count) noexcept -> jsi::Value {
           auto layoutMetrics = uiManager->getRelativeLayoutMetrics(
               *shadowNodeFromValue(runtime, arguments[0]),
               shadowNodeFromValue(runtime, arguments[1]).get(),
@@ -571,9 +571,9 @@ jsi::Value UIManagerBinding::get(
         2,
         [uiManager](
             jsi::Runtime &runtime,
-            const jsi::Value &thisValue,
-            const jsi::Value *arguments,
-            size_t count) -> jsi::Value {
+            jsi::Value const &thisValue,
+            jsi::Value const *arguments,
+            size_t count) noexcept -> jsi::Value {
           auto layoutMetrics = uiManager->getRelativeLayoutMetrics(
               *shadowNodeFromValue(runtime, arguments[0]),
               nullptr,
@@ -606,9 +606,9 @@ jsi::Value UIManagerBinding::get(
         2,
         [uiManager](
             jsi::Runtime &runtime,
-            const jsi::Value &thisValue,
-            const jsi::Value *arguments,
-            size_t count) -> jsi::Value {
+            jsi::Value const &thisValue,
+            jsi::Value const *arguments,
+            size_t count) noexcept -> jsi::Value {
           auto layoutMetrics = uiManager->getRelativeLayoutMetrics(
               *shadowNodeFromValue(runtime, arguments[0]),
               nullptr,
@@ -641,9 +641,9 @@ jsi::Value UIManagerBinding::get(
         2,
         [uiManager](
             jsi::Runtime &runtime,
-            const jsi::Value &thisValue,
-            const jsi::Value *arguments,
-            size_t count) -> jsi::Value {
+            jsi::Value const &thisValue,
+            jsi::Value const *arguments,
+            size_t count) noexcept -> jsi::Value {
           uiManager->setNativeProps(
               *shadowNodeFromValue(runtime, arguments[0]),
               RawProps(runtime, arguments[1]));
@@ -659,9 +659,9 @@ jsi::Value UIManagerBinding::get(
         3,
         [uiManager](
             jsi::Runtime &runtime,
-            const jsi::Value &thisValue,
-            const jsi::Value *arguments,
-            size_t count) -> jsi::Value {
+            jsi::Value const &thisValue,
+            jsi::Value const *arguments,
+            size_t count) noexcept -> jsi::Value {
           uiManager->configureNextLayoutAnimation(
               runtime,
               // TODO: pass in JSI value instead of folly::dynamic to RawValue