From 546821e25838969996da0947424ee75585c59e85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Thu, 30 May 2024 08:32:06 -0700 Subject: [PATCH] Mark root view as attached after executing all pending operations (#44726) Summary: 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`. Differential Revision: D57968937 --- .../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 72b640e0bbb4b0..63b79a4d2228aa 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 @@ private void addRootView(@NonNull final View rootView) { 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()) {