From 490b05d0d89bc3de05bfde8baf25f7f64df75fbe Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Thu, 25 Sep 2025 10:04:48 -0700 Subject: [PATCH] Extract SynchronousMountItem (#53936) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/53936 Align with other mount items and make it easier to identify in memory traces. Changelog: [Internal] Reviewed By: lenaic Differential Revision: D83241794 fbshipit-source-id: 9df1523e265e560c15f93bad8cd91a651bc5a4e2 --- .../react/fabric/FabricUIManager.java | 60 +++++-------------- .../react/fabric/mounting/MountingManager.kt | 2 +- .../DestroyUnmountedViewMountItem.kt | 6 +- .../mountitems/IntBufferBatchMountItem.kt | 6 +- .../SendAccessibilityEventMountItem.kt | 12 ++-- .../mountitems/SynchronousMountItem.kt | 38 ++++++++++++ 6 files changed, 65 insertions(+), 59 deletions(-) create mode 100644 packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/SynchronousMountItem.kt diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java index 4ab96289469..3f765f81415 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -66,6 +66,7 @@ import com.facebook.react.fabric.mounting.mountitems.DispatchCommandMountItem; import com.facebook.react.fabric.mounting.mountitems.MountItem; import com.facebook.react.fabric.mounting.mountitems.MountItemFactory; import com.facebook.react.fabric.mounting.mountitems.PrefetchResourcesMountItem; +import com.facebook.react.fabric.mounting.mountitems.SynchronousMountItem; import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags; import com.facebook.react.internal.featureflags.ReactNativeNewArchitectureFeatureFlags; import com.facebook.react.internal.interop.InteropEventEmitter; @@ -178,7 +179,7 @@ public class FabricUIManager private final BatchEventDispatchedListener mBatchEventDispatchedListener; - private final CopyOnWriteArrayList mListeners = new CopyOnWriteArrayList<>(); + private final List mListeners = new CopyOnWriteArrayList<>(); private boolean mMountNotificationScheduled = false; private List mSurfaceIdsWithPendingMountNotification = new ArrayList<>(); @@ -791,35 +792,7 @@ public class FabricUIManager // android.view.View.updateDisplayListIfDirty(View.java:20466) // 3. A view is deleted while its parent is being drawn, causing a crash. - MountItem synchronousMountItem = - new MountItem() { - @Override - public void execute(MountingManager mountingManager) { - try { - mountingManager.storeSynchronousMountPropsOverride(reactTag, props); - mountingManager.updatePropsSynchronously(reactTag, props); - } catch (Exception ex) { - // TODO T42943890: Fix animations in Fabric and remove this try/catch? - // There might always be race conditions between surface teardown and - // animations/other operations, so it may not be feasible to remove this. - // Practically 100% of reported errors from this point are because the - // surface has stopped by this point, but the MountItem was queued before - // the surface was stopped. It's likely not feasible to prevent all such races. - } - } - - @Override - public int getSurfaceId() { - return View.NO_ID; - } - - @Override - public String toString() { - String propsString = - IS_DEVELOPMENT_ENVIRONMENT ? props.toHashMap().toString() : ""; - return String.format("SYNC UPDATE PROPS [%d]: %s", reactTag, propsString); - } - }; + MountItem synchronousMountItem = new SynchronousMountItem(reactTag, props); // If the reactTag exists, we assume that it might at the end of the next // batch of MountItems. Otherwise, we try to execute immediately. @@ -1399,24 +1372,21 @@ public class FabricUIManager // delay paint. UiThreadUtil.getUiThreadHandler() .postAtFrontOfQueue( - new Runnable() { - @Override - public void run() { - mMountNotificationScheduled = false; + () -> { + mMountNotificationScheduled = false; - // Create a copy in case mount hooks trigger more mutations - final List surfaceIdsToReportMount = - mSurfaceIdsWithPendingMountNotification; - mSurfaceIdsWithPendingMountNotification = new ArrayList<>(); + // Create a copy in case mount hooks trigger more mutations + final List surfaceIdsToReportMount = + mSurfaceIdsWithPendingMountNotification; + mSurfaceIdsWithPendingMountNotification = new ArrayList<>(); - final @Nullable FabricUIManagerBinding binding = mBinding; - if (binding == null || mDestroyed) { - return; - } + final @Nullable FabricUIManagerBinding binding = mBinding; + if (binding == null || mDestroyed) { + return; + } - for (int surfaceId : surfaceIdsToReportMount) { - binding.reportMount(surfaceId); - } + for (int surfaceId : surfaceIdsToReportMount) { + binding.reportMount(surfaceId); } }); } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.kt index be94133ddd5..c40beec2172 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.kt @@ -51,7 +51,7 @@ internal class MountingManager( private val rootViewManager = RootViewManager() internal fun interface MountItemExecutor { - @UiThread @ThreadConfined(ThreadConfined.UI) fun executeItems(items: Queue?) + @UiThread @ThreadConfined(ThreadConfined.UI) fun executeItems(items: Queue) } /** diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/DestroyUnmountedViewMountItem.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/DestroyUnmountedViewMountItem.kt index eba0bebf249..8d83fa39a30 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/DestroyUnmountedViewMountItem.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/DestroyUnmountedViewMountItem.kt @@ -14,14 +14,14 @@ import com.facebook.react.fabric.mounting.MountingManager * for views that were preallcated but never mounted on the screen. */ internal class DestroyUnmountedViewMountItem( - private val _surfaceId: Int, + private val surfaceId: Int, private val reactTag: Int, ) : MountItem { override fun execute(mountingManager: MountingManager) { - val surfaceMountingManager = mountingManager.getSurfaceManager(_surfaceId) ?: return + val surfaceMountingManager = mountingManager.getSurfaceManager(surfaceId) ?: return surfaceMountingManager.deleteView(reactTag) } - override fun getSurfaceId(): Int = _surfaceId + override fun getSurfaceId(): Int = surfaceId } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/IntBufferBatchMountItem.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/IntBufferBatchMountItem.kt index 4a87e388dfe..221171fb470 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/IntBufferBatchMountItem.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/IntBufferBatchMountItem.kt @@ -8,7 +8,6 @@ package com.facebook.react.fabric.mounting.mountitems import com.facebook.common.logging.FLog -import com.facebook.proguard.annotations.DoNotStripAny import com.facebook.react.bridge.ReactMarker import com.facebook.react.bridge.ReactMarkerConstants import com.facebook.react.bridge.ReadableMap @@ -31,7 +30,8 @@ import java.util.Locale * The purpose of encapsulating the array of MountItems this way, is to reduce the amount of * allocations in C++ and JNI round-trips. */ -@DoNotStripAny +private const val TAG = "IntBufferBatchMountItem" + internal class IntBufferBatchMountItem( private val surfaceId: Int, private val intBuffer: IntArray, @@ -341,8 +341,6 @@ internal class IntBufferBatchMountItem( } companion object { - val TAG: String = IntBufferBatchMountItem::class.java.simpleName - const val INSTRUCTION_FLAG_MULTIPLE: Int = 1 const val INSTRUCTION_CREATE: Int = 2 diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/SendAccessibilityEventMountItem.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/SendAccessibilityEventMountItem.kt index 6478199bb0f..34d7b15f912 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/SendAccessibilityEventMountItem.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/SendAccessibilityEventMountItem.kt @@ -12,17 +12,17 @@ import com.facebook.react.bridge.ReactSoftExceptionLogger import com.facebook.react.bridge.RetryableMountingLayerException import com.facebook.react.fabric.mounting.MountingManager +private const val TAG = "SendAccessibilityEventMountItem" + internal class SendAccessibilityEventMountItem( - private val _surfaceId: Int, + private val surfaceId: Int, private val reactTag: Int, private val eventType: Int, ) : MountItem { - private val TAG = "Fabric.SendAccessibilityEvent" - override fun execute(mountingManager: MountingManager) { try { - mountingManager.sendAccessibilityEvent(_surfaceId, reactTag, eventType) + mountingManager.sendAccessibilityEvent(surfaceId, reactTag, eventType) } catch (e: RetryableMountingLayerException) { // Accessibility events are similar to commands in that they're imperative // calls from JS, disconnected from the commit lifecycle, and therefore @@ -35,7 +35,7 @@ internal class SendAccessibilityEventMountItem( } } - override fun getSurfaceId(): Int = _surfaceId + override fun getSurfaceId(): Int = surfaceId - override fun toString(): String = "SendAccessibilityEventMountItem [$reactTag] $eventType" + override fun toString(): String = "$TAG [$reactTag] $eventType" } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/SynchronousMountItem.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/SynchronousMountItem.kt new file mode 100644 index 00000000000..33a5b3a600a --- /dev/null +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/SynchronousMountItem.kt @@ -0,0 +1,38 @@ +/* + * 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. + */ + +package com.facebook.react.fabric.mounting.mountitems + +import android.view.View +import com.facebook.react.bridge.ReadableMap +import com.facebook.react.fabric.FabricUIManager.IS_DEVELOPMENT_ENVIRONMENT +import com.facebook.react.fabric.mounting.MountingManager + +internal class SynchronousMountItem(private val reactTag: Int, private val props: ReadableMap) : + MountItem { + + override fun execute(mountingManager: MountingManager) { + try { + mountingManager.storeSynchronousMountPropsOverride(reactTag, props) + mountingManager.updatePropsSynchronously(reactTag, props) + } catch (ex: Exception) { + // TODO T42943890: Fix animations in Fabric and remove this try/catch? + // There might always be race conditions between surface teardown and + // animations/other operations, so it may not be feasible to remove this. + // Practically 100% of reported errors from this point are because the + // surface has stopped by this point, but the MountItem was queued before + // the surface was stopped. It's likely not feasible to prevent all such races. + } + } + + override fun toString(): String { + val propsString = if (IS_DEVELOPMENT_ENVIRONMENT) props.toHashMap().toString() else "" + return "SYNC UPDATE PROPS [$reactTag]: $propsString" + } + + override fun getSurfaceId(): Int = View.NO_ID +}