diff --git a/ReactCommon/fabric/mounting/ShadowTree.cpp b/ReactCommon/fabric/mounting/ShadowTree.cpp index 57221095f62..9b4f43b6024 100644 --- a/ReactCommon/fabric/mounting/ShadowTree.cpp +++ b/ReactCommon/fabric/mounting/ShadowTree.cpp @@ -103,19 +103,6 @@ ShadowTree::ShadowTree( } ShadowTree::~ShadowTree() { - commit( - [](const SharedRootShadowNode &oldRootShadowNode) { - return std::make_shared( - *oldRootShadowNode, - ShadowNodeFragment{ - /* .tag = */ ShadowNodeFragment::tagPlaceholder(), - /* .surfaceId = */ ShadowNodeFragment::surfaceIdPlaceholder(), - /* .props = */ ShadowNodeFragment::propsPlaceholder(), - /* .eventEmitter = */ - ShadowNodeFragment::eventEmitterPlaceholder(), - /* .children = */ ShadowNode::emptySharedShadowNodeSharedList(), - }); - }); mountingCoordinator_->revoke(); } @@ -206,6 +193,23 @@ bool ShadowTree::tryCommit(ShadowTreeCommitTransaction transaction) const { return true; } +void ShadowTree::commitEmptyTree() const { + commit( + [](const SharedRootShadowNode &oldRootShadowNode) + -> UnsharedRootShadowNode { + return std::make_shared( + *oldRootShadowNode, + ShadowNodeFragment{ + /* .tag = */ ShadowNodeFragment::tagPlaceholder(), + /* .surfaceId = */ ShadowNodeFragment::surfaceIdPlaceholder(), + /* .props = */ ShadowNodeFragment::propsPlaceholder(), + /* .eventEmitter = */ + ShadowNodeFragment::eventEmitterPlaceholder(), + /* .children = */ ShadowNode::emptySharedShadowNodeSharedList(), + }); + }); +} + void ShadowTree::emitLayoutEvents( std::vector &affectedLayoutableNodes) const { SystraceSection s("ShadowTree::emitLayoutEvents"); diff --git a/ReactCommon/fabric/mounting/ShadowTree.h b/ReactCommon/fabric/mounting/ShadowTree.h index 4fedcf7449f..c7a20513745 100644 --- a/ReactCommon/fabric/mounting/ShadowTree.h +++ b/ReactCommon/fabric/mounting/ShadowTree.h @@ -58,6 +58,11 @@ class ShadowTree final { */ void commit(ShadowTreeCommitTransaction transaction) const; + /* + * Commit an empty tree (a new `RootShadowNode` with no children). + */ + void commitEmptyTree() const; + #pragma mark - Delegate /* diff --git a/ReactCommon/fabric/uimanager/Scheduler.cpp b/ReactCommon/fabric/uimanager/Scheduler.cpp index 139ec13c147..1828abaae3a 100644 --- a/ReactCommon/fabric/uimanager/Scheduler.cpp +++ b/ReactCommon/fabric/uimanager/Scheduler.cpp @@ -151,29 +151,24 @@ void Scheduler::renderTemplateToSurface( void Scheduler::stopSurface(SurfaceId surfaceId) const { SystraceSection s("Scheduler::stopSurface"); + // Note, we have to do in inside `visit` function while the Shadow Tree + // is still being registered. uiManager_->getShadowTreeRegistry().visit( - surfaceId, [](const ShadowTree &shadowTree) { - // As part of stopping the Surface, we have to commit an empty tree. - return shadowTree.tryCommit([&](const SharedRootShadowNode - &oldRootShadowNode) { - return std::make_shared( - *oldRootShadowNode, - ShadowNodeFragment{ - /* .tag = */ ShadowNodeFragment::tagPlaceholder(), - /* .surfaceId = */ - ShadowNodeFragment::surfaceIdPlaceholder(), - /* .props = */ ShadowNodeFragment::propsPlaceholder(), - /* .eventEmitter = */ - ShadowNodeFragment::eventEmitterPlaceholder(), - /* .children = */ - ShadowNode::emptySharedShadowNodeSharedList(), - }); - }); + surfaceId, [](ShadowTree const &shadowTree) { + // As part of stopping a Surface, we need to properly destroy all + // mounted views, so we need to commit an empty tree to trigger all + // side-effects that will perform that. + shadowTree.commitEmptyTree(); }); - auto shadowTree = uiManager_->getShadowTreeRegistry().remove(surfaceId); - shadowTree->setDelegate(nullptr); + // Waiting for all concurrent commits to be finished and unregistering the + // `ShadowTree`. + uiManager_->getShadowTreeRegistry().remove(surfaceId); + // We execute JavaScript/React part of the process at the very end to minimize + // any visible side-effects of stopping the Surface. Any possible commits from + // the JavaScript side will not be able to reference a `ShadowTree` and will + // fail silently. auto uiManager = uiManager_; runtimeExecutor_([=](jsi::Runtime &runtime) { uiManager->visitBinding([&](UIManagerBinding const &uiManagerBinding) {