From 0bea4cd0cc52cc301991425410e4d778fe673f69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Thu, 30 May 2024 12:37:17 -0700 Subject: [PATCH] Mark root view as attached after executing all pending operations (#44726) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/44726 Changelog: [internal] ## Context We ran an experiment to test synchronous state updates in Fabric and we saw some crashes on Android. Those crashes were caused by mounting operations not being applied in the correct order. There were 2 root causes for that problem: 1. State updates triggered during mount would be committed and mounted synchronously during that specific mount operation. That caused problems like trying to clip views that weren't created already (as we were processing the state update for the content offset before we actually created the child views). 2. Same problem as before, but with mount operations that were processed when the root view wasn't available yet (this is a separate queue). We tried to fix the problem in https://github.com/facebook/react-native/pull/44015, but the solution for 2) was incorrect, as we didn't account for those operations being in a different queue (it was reverted in https://github.com/facebook/react-native/pull/44724). ## Changes I think the right solution for point 2) is that, instead of marking the root view as available and then process all pending operations, we flip those operations. That was, if there are any mount operations as a side-effect of processing that queue, those will also be added to the same queue, instead of being processed immediately in `MountItemDispatcher`. Reviewed By: sammy-SC Differential Revision: D57968937 fbshipit-source-id: 93d10cdeced0c837d4301768aee8575d2c940b10 --- .../fabric/mounting/SurfaceMountingManager.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java index 72b640e0bbb..63b79a4d222 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java @@ -222,9 +222,20 @@ public class SurfaceMountingManager { if (rootView instanceof ReactRoot) { ((ReactRoot) rootView).setRootViewTag(mSurfaceId); } - mRootViewAttached = true; + + if (!ReactNativeFeatureFlags.forceBatchingMountItemsOnAndroid()) { + mRootViewAttached = true; + } executeMountItemsOnViewAttach(); + + if (ReactNativeFeatureFlags.forceBatchingMountItemsOnAndroid()) { + // By doing this after `executeMountItemsOnViewAttach`, we ensure + // that any operations scheduled while processing this queue are + // also added to the queue, instead of being processed immediately + // through the queue in `MountItemDispatcher`. + mRootViewAttached = true; + } }; if (UiThreadUtil.isOnUiThread()) {