From 8a8c96b016cacc383c0c27f79d1e6f518dc57bc2 Mon Sep 17 00:00:00 2001 From: MazurDorian Date: Sun, 25 Oct 2026 22:20:04 +0100 Subject: [PATCH] Reject mounting revisions older than the current base --- .../renderer/mounting/MountingCoordinator.cpp | 7 ++ .../renderer/mounting/MountingCoordinator.h | 1 + .../tests/MountingCoordinatorTest.cpp | 94 +++++++++++++++++++ 3 files changed, 102 insertions(+) create mode 100644 packages/react-native/ReactCommon/react/renderer/mounting/tests/MountingCoordinatorTest.cpp diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp b/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp index c25515b6d014..51a30b1f497d 100644 --- a/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp +++ b/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp @@ -40,6 +40,13 @@ void MountingCoordinator::push(ShadowTreeRevision revision) const { { std::scoped_lock lock(mutex_); + // ShadowTree releases its commit lock before pushing. Another thread may + // already have diffed a newer revision, even when lastRevision_ is empty. + // Never recreate removed views by diffing backwards from that newer base. + if (revision.number < baseRevision_.number) { + return; + } + react_native_assert( !lastRevision_.has_value() || revision.number != lastRevision_->number); diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.h b/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.h index c520508cb06b..0a4f7063011f 100644 --- a/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.h +++ b/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.h @@ -110,6 +110,7 @@ class MountingCoordinator final { */ private: friend class ShadowTree; + friend class MountingCoordinatorTest; void push(ShadowTreeRevision revision) const; diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/tests/MountingCoordinatorTest.cpp b/packages/react-native/ReactCommon/react/renderer/mounting/tests/MountingCoordinatorTest.cpp new file mode 100644 index 000000000000..c523d12a49a4 --- /dev/null +++ b/packages/react-native/ReactCommon/react/renderer/mounting/tests/MountingCoordinatorTest.cpp @@ -0,0 +1,94 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include +#include +#include + +#include + +#include +#include +#include + +namespace facebook::react { + +class MountingCoordinatorTest : public ::testing::Test { + protected: + ComponentBuilder builder_{simpleComponentBuilder()}; + RootShadowNode::Unshared withChild_; + RootShadowNode::Unshared withoutChild_; + + void SetUp() override { + auto props = std::make_shared(); + props->collapsable = false; + withChild_ = builder_.build( + Element().tag(1).surfaceId(1).children( + {Element().tag(2).surfaceId(1).props(props)})); + withChild_->layoutIfNeeded(); + withoutChild_ = + std::static_pointer_cast(withChild_->ShadowNode::clone( + {.children = ShadowNode::emptySharedShadowNodeSharedList()})); + withoutChild_->layoutIfNeeded(); + } + + static void push( + const MountingCoordinator& coordinator, + ShadowTreeRevision revision) { + coordinator.push(std::move(revision)); + } + + static bool hasMutation( + const MountingTransaction& transaction, + ShadowViewMutation::Type type) { + const auto& mutations = transaction.getMutations(); + return std::any_of( + mutations.begin(), mutations.end(), [&](const auto& mutation) { + return mutation.type == type && + (mutation.oldChildShadowView.tag == 2 || + mutation.newChildShadowView.tag == 2); + }); + } +}; + +TEST_F(MountingCoordinatorTest, lateRevisionDoesNotRecreateRemovedView) { + MountingCoordinator coordinator{{withChild_, 1, {}}}; + + // Revision 3 removes the view while revision 2 is delayed between commit + // and push. Pulling clears lastRevision_, but revision 3 remains the base. + push(coordinator, {withoutChild_, 3, {}}); + auto removal = coordinator.pullTransaction(); + ASSERT_TRUE(removal.has_value()); + ASSERT_TRUE(hasMutation(*removal, ShadowViewMutation::Type::Delete)); + + push(coordinator, {withChild_, 2, {}}); + EXPECT_FALSE(coordinator.pullTransaction().has_value()); + EXPECT_EQ(coordinator.getBaseRevision().number, 3); + EXPECT_EQ(coordinator.getBaseRevision().rootShadowNode, withoutChild_); + + // A subsequent navigation may legitimately create the view again. + push(coordinator, {withChild_, 4, {}}); + auto insertion = coordinator.pullTransaction(); + ASSERT_TRUE(insertion.has_value()); + EXPECT_TRUE(hasMutation(*insertion, ShadowViewMutation::Type::Create)); + EXPECT_EQ(coordinator.getBaseRevision().number, 4); +} + +TEST_F(MountingCoordinatorTest, lateRevisionPreservesNewerPendingRevision) { + MountingCoordinator coordinator{{withChild_, 1, {}}}; + push(coordinator, {withoutChild_, 3, {}}); + push(coordinator, {withChild_, 2, {}}); + + auto removal = coordinator.pullTransaction(); + ASSERT_TRUE(removal.has_value()); + EXPECT_TRUE(hasMutation(*removal, ShadowViewMutation::Type::Delete)); + EXPECT_EQ(coordinator.getBaseRevision().number, 3); + EXPECT_EQ(coordinator.getBaseRevision().rootShadowNode, withoutChild_); + EXPECT_FALSE(coordinator.pullTransaction().has_value()); +} + +} // namespace facebook::react