Preserve mount order when merged transactions recreate a view

packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp

@@ -10,6 +10,7 @@
 #include "AnimatedPropBufferEncoder.h"
 #include "EventEmitterWrapper.h"
 #include "MountItem.h"
+#include "MountItemReordering.h"
 #include "StateWrapperImpl.h"
 
 #include <cxxreact/TraceSection.h>
@@ -606,7 +607,8 @@ void FabricMountingManager::executeMount(
   auto& mutations = transaction.getMutations();
 
   bool maintainMutationOrder =
-      ReactNativeFeatureFlags::disableMountItemReorderingAndroid();
+      ReactNativeFeatureFlags::disableMountItemReorderingAndroid() ||
+      requiresStrictMountItemOrder(mutations);
   bool updateViewsOnDensityChange =
       ReactNativeFeatureFlags::fixTextLayoutOnPixelDensityChange();
 
@@ -1026,13 +1028,9 @@ void FabricMountingManager::executeMount(
     }
   }
 
-  // Write deletes last - so that all prop updates, etc, for the tag in the same
-  // batch don't fail. Without additional machinery, moving deletes here
-  // requires that the differ never produces "DELETE...CREATE" in that order for
-  // the same tag. It's nice to be able to batch all similar operations together
-  // for space efficiency.
-  // FIXME: this optimization is incorrect when multiple transactions are
-  // merged together
+  // Write deletes last so updates for the same tag do not fail. Batches that
+  // recreate a deleted tag preserve mutation order in cppCommonMountItems
+  // instead, keeping updates and deletes attached to the correct native view.
   if (!cppDeleteMountItems.empty()) {
     writeMountItemPreamble(
         buffer, CppMountItem::Type::Delete, cppDeleteMountItems.size());

packages/react-native/ReactAndroid/src/main/jni/react/fabric/MountItemReordering.h

@@ -0,0 +1,45 @@
+/*
+ * 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.
+ */
+
+#pragma once
+
+#include <react/renderer/mounting/ShadowViewMutation.h>
+
+#include <algorithm>
+#include <iterator>
+#include <unordered_set>
+
+namespace facebook::react {
+namespace {
+
+inline bool requiresStrictMountItemOrder(const ShadowViewMutationList &mutations)
+{
+  auto firstDelete = std::find_if(mutations.begin(), mutations.end(), [](const auto &mutation) {
+    return mutation.type == ShadowViewMutation::Delete;
+  });
+  if (firstDelete == mutations.end() || std::none_of(std::next(firstDelete), mutations.end(), [](const auto &mutation) {
+        return mutation.type == ShadowViewMutation::Create;
+      })) {
+    return false;
+  }
+
+  // Do not allocate a set for ordinary mount-only or unmount-only batches.
+  std::unordered_set<Tag> deletedTags;
+  for (auto it = firstDelete; it != mutations.end(); ++it) {
+    const auto &mutation = *it;
+    if (mutation.type == ShadowViewMutation::Delete) {
+      deletedTags.insert(mutation.oldChildShadowView.tag);
+    } else if (mutation.type == ShadowViewMutation::Create && deletedTags.contains(mutation.newChildShadowView.tag)) {
+      // Moving deletes to the end would delete this tag's new native view.
+      return true;
+    }
+  }
+  return false;
+}
+
+} // namespace
+} // namespace facebook::react

packages/react-native/ReactAndroid/src/main/jni/react/fabric/test/MountItemReorderingTest.cpp

@@ -0,0 +1,142 @@
+/*
+ * 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 <react/fabric/MountItemReordering.h>
+#include <react/renderer/mounting/MountingTransaction.h>
+
+#include <gtest/gtest.h>
+
+namespace facebook::react {
+namespace {
+
+ShadowView viewWithTag(Tag tag) {
+  ShadowView view;
+  view.tag = tag;
+  view.layoutMetrics.frame.size = {100, 100};
+  return view;
+}
+
+} // namespace
+
+TEST(MountItemReorderingTest, emptyAndUpdateOnlyBatchesCanBeReordered) {
+  EXPECT_FALSE(requiresStrictMountItemOrder({}));
+  auto view = viewWithTag(42);
+  EXPECT_FALSE(requiresStrictMountItemOrder(
+      {ShadowViewMutation::UpdateMutation(view, view, 1)}));
+}
+
+TEST(MountItemReorderingTest, distinctLifetimesCanBeReordered) {
+  EXPECT_FALSE(requiresStrictMountItemOrder(
+      {ShadowViewMutation::DeleteMutation(viewWithTag(42)),
+       ShadowViewMutation::CreateMutation(viewWithTag(43))}));
+}
+
+TEST(MountItemReorderingTest, deletionOnlyBatchesCanBeReordered) {
+  EXPECT_FALSE(requiresStrictMountItemOrder(
+      {ShadowViewMutation::DeleteMutation(viewWithTag(42)),
+       ShadowViewMutation::DeleteMutation(viewWithTag(43))}));
+}
+
+TEST(MountItemReorderingTest, creationBeforeFinalDeletionCanBeReordered) {
+  auto view = viewWithTag(42);
+  EXPECT_FALSE(requiresStrictMountItemOrder(
+      {ShadowViewMutation::CreateMutation(view),
+       ShadowViewMutation::DeleteMutation(view)}));
+}
+
+TEST(MountItemReorderingTest, reparentingWithoutDeletionCanBeReordered) {
+  auto view = viewWithTag(42);
+  EXPECT_FALSE(requiresStrictMountItemOrder(
+      {ShadowViewMutation::RemoveMutation(1, view, 0),
+       ShadowViewMutation::InsertMutation(2, view, 0)}));
+}
+
+TEST(MountItemReorderingTest, recreationRequiresOriginalOrder) {
+  auto view = viewWithTag(42);
+  EXPECT_TRUE(requiresStrictMountItemOrder(
+      {ShadowViewMutation::DeleteMutation(view),
+       ShadowViewMutation::CreateMutation(view)}));
+}
+
+TEST(MountItemReorderingTest, virtualViewRecreationRequiresOriginalOrder) {
+  auto view = viewWithTag(42);
+  view.layoutMetrics = EmptyLayoutMetrics;
+  EXPECT_TRUE(requiresStrictMountItemOrder(
+      {ShadowViewMutation::DeleteMutation(view),
+       ShadowViewMutation::CreateMutation(view)}));
+}
+
+TEST(MountItemReorderingTest, multipleLifetimesRequireOriginalOrder) {
+  auto view = viewWithTag(42);
+  EXPECT_TRUE(requiresStrictMountItemOrder(
+      {ShadowViewMutation::CreateMutation(view),
+       ShadowViewMutation::DeleteMutation(view),
+       ShadowViewMutation::CreateMutation(view),
+       ShadowViewMutation::DeleteMutation(view)}));
+}
+
+TEST(MountItemReorderingTest, interleavedTagsDoNotHideRecreation) {
+  EXPECT_TRUE(requiresStrictMountItemOrder(
+      {ShadowViewMutation::DeleteMutation(viewWithTag(42)),
+       ShadowViewMutation::DeleteMutation(viewWithTag(43)),
+       ShadowViewMutation::CreateMutation(viewWithTag(44)),
+       ShadowViewMutation::CreateMutation(viewWithTag(42))}));
+}
+
+TEST(MountItemReorderingTest, mergedTransactionsRetainLifetimeBoundaries) {
+  auto oldView = viewWithTag(42);
+  auto newView = viewWithTag(42);
+  newView.layoutMetrics.frame.size = {100, 100};
+  MountingTransaction transaction{
+      1,
+      1,
+      {ShadowViewMutation::UpdateMutation(oldView, oldView, 1),
+       ShadowViewMutation::RemoveMutation(1, oldView, 0),
+       ShadowViewMutation::DeleteMutation(oldView)},
+      TransactionTelemetry{}};
+  transaction.mergeWith(
+      MountingTransaction{
+          1,
+          2,
+          {ShadowViewMutation::CreateMutation(newView),
+           ShadowViewMutation::InsertMutation(1, newView, 0),
+           ShadowViewMutation::UpdateMutation(newView, newView, 1)},
+          TransactionTelemetry{}});
+
+  EXPECT_TRUE(requiresStrictMountItemOrder(transaction.getMutations()));
+}
+
+TEST(
+    MountItemReorderingTest,
+    selectsStrictOrderForRecreationBeforeLaterReparenting) {
+  auto child = viewWithTag(480);
+  auto sibling = viewWithTag(1712);
+  MountingTransaction transaction{
+      1,
+      1,
+      {ShadowViewMutation::RemoveMutation(10, child, 0),
+       ShadowViewMutation::DeleteMutation(child)},
+      TransactionTelemetry{}};
+  transaction.mergeWith(
+      MountingTransaction{
+          1,
+          2,
+          {ShadowViewMutation::CreateMutation(child),
+           ShadowViewMutation::InsertMutation(10, child, 0)},
+          TransactionTelemetry{}});
+
+  // A trailing delete would lose this child before a later batch moves it to
+  // index 0 and inserts its next sibling at index 1 in the destination parent.
+  EXPECT_TRUE(requiresStrictMountItemOrder(transaction.getMutations()));
+  EXPECT_FALSE(requiresStrictMountItemOrder(
+      {ShadowViewMutation::RemoveMutation(10, child, 0),
+       ShadowViewMutation::InsertMutation(482, child, 0),
+       ShadowViewMutation::CreateMutation(sibling),
+       ShadowViewMutation::InsertMutation(482, sibling, 1)}));
+}
+
+} // namespace facebook::react