From 4f94d7da8197c98db5927d9b5ee350db7a6a2eeb Mon Sep 17 00:00:00 2001
From: Samuel Susla <samuelsusla@fb.com>
Date: Sun, 16 Aug 2020 10:31:29 -0700
Subject: [PATCH] Back out "Use ShadowTree::commit if failureCallback is
 nullptr"

Summary:
Changelog: [internal]

Original commit changeset: 5cb78a3a75a7

In D23151218 (https://github.com/facebook/react-native/commit/dffec8bc7bee4d05b72d19b4efe4aeab980f4cd5) we switched from `ShadowTree::tryCommit` to `ShadowTree::commit` inside `UIManager::updateState`.
It fixes update state being dropped but can cause an infinite loop inside `ShadowTree::commit` because the shadow node that triggered `UIManager::updateState` can been removed before the `updateState` call is dispatched..

Reviewed By: JoshuaGross

Differential Revision: D23155228

fbshipit-source-id: f3339a4e4798880972366d6f894c14a58be1b9b2
---
 .../react/renderer/uimanager/UIManager.cpp    | 23 ++++++-------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/ReactCommon/react/renderer/uimanager/UIManager.cpp b/ReactCommon/react/renderer/uimanager/UIManager.cpp
index e9e48ea856f..91827e3fb94 100644
--- a/ReactCommon/react/renderer/uimanager/UIManager.cpp
+++ b/ReactCommon/react/renderer/uimanager/UIManager.cpp
@@ -238,10 +238,10 @@ void UIManager::updateState(StateUpdate const &stateUpdate) const {
 
   shadowTreeRegistry_.visit(
       family->getSurfaceId(), [&](ShadowTree const &shadowTree) {
-        auto transaction = [&](RootShadowNode::Shared const &oldRootShadowNode)
-            -> RootShadowNode::Unshared {
-          return std::static_pointer_cast<RootShadowNode>(
-              oldRootShadowNode->cloneTree(
+        bool updateSucceeded = shadowTree.tryCommit(
+            [&](RootShadowNode::Shared const &oldRootShadowNode) {
+              return std::static_pointer_cast<
+                  RootShadowNode>(oldRootShadowNode->cloneTree(
                   *family, [&](ShadowNode const &oldShadowNode) {
                     auto newData =
                         callback(oldShadowNode.getState()->getDataPointer());
@@ -255,18 +255,9 @@ void UIManager::updateState(StateUpdate const &stateUpdate) const {
                         /* .state = */ newState,
                     });
                   }));
-        };
-        if (stateUpdate.failureCallback) {
-          // If caller passes failure callback, we don't retry but let the
-          // caller handle failure.
-          bool updateSucceeded = shadowTree.tryCommit(transaction);
-          if (!updateSucceeded && stateUpdate.failureCallback) {
-            stateUpdate.failureCallback();
-          }
-        } else {
-          // If caller does't provide failure block, commit is attempted until
-          // it succeeds.
-          shadowTree.commit(transaction);
+            });
+        if (!updateSucceeded && stateUpdate.failureCallback) {
+          stateUpdate.failureCallback();
         }
       });
 }