From 322d9e1537a3687ae3925cdcf0ea8f571b9fa24d Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Thu, 30 Nov 2023 02:52:42 -0800 Subject: [PATCH] Clarify threading of OnViewAttach mount items (#41704) Summary: `mOnViewAttachItems` was set to be be concurrent, but this would be unexpected, as all mount item operations occur solely on the main thread. Simplify this to be just a LinkedList and annotate the methods as being UI thread only. Changelog: [Internal] Reviewed By: NickGerleman Differential Revision: D51662154 --- .../fabric/mounting/MountItemDispatcher.java | 2 +- .../mounting/SurfaceMountingManager.java | 19 ++++++++++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.java index 7a083c9870b322..3261a598b041f3 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.java @@ -362,7 +362,7 @@ private void executeOrEnqueue(MountItem item) { } SurfaceMountingManager surfaceMountingManager = mMountingManager.getSurfaceManager(item.getSurfaceId()); - surfaceMountingManager.executeOnViewAttach(item); + surfaceMountingManager.scheduleMountItemOnViewAttach(item); } else { item.execute(mMountingManager); } 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 87b15e454f3127..90267da55b369a 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 @@ -52,6 +52,7 @@ import com.facebook.react.uimanager.events.EventCategoryDef; import com.facebook.react.views.view.ReactMapBufferViewManager; import com.facebook.react.views.view.ReactViewManagerWrapper; +import java.util.ArrayDeque; import java.util.HashSet; import java.util.LinkedList; import java.util.Map; @@ -59,7 +60,6 @@ import java.util.Set; import java.util.Stack; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentLinkedQueue; public class SurfaceMountingManager { public static final String TAG = SurfaceMountingManager.class.getSimpleName(); @@ -74,7 +74,7 @@ public class SurfaceMountingManager { // These are all non-null, until StopSurface is called private ConcurrentHashMap mTagToViewState = new ConcurrentHashMap<>(); // any thread - private ConcurrentLinkedQueue mOnViewAttachItems = new ConcurrentLinkedQueue<>(); + private Queue mOnViewAttachMountItems = new ArrayDeque<>(); private JSResponderHandler mJSResponderHandler; private ViewManagerRegistry mViewManagerRegistry; private RootViewManager mRootViewManager; @@ -181,9 +181,10 @@ public boolean getViewExists(int tag) { return mTagToViewState.containsKey(tag); } - @AnyThread - public void executeOnViewAttach(MountItem item) { - mOnViewAttachItems.add(item); + @UiThread + @ThreadConfined(UI) + public void scheduleMountItemOnViewAttach(MountItem item) { + mOnViewAttachMountItems.add(item); } @AnyThread @@ -233,7 +234,7 @@ private void addRootView(@NonNull final View rootView) { } mRootViewAttached = true; - executeViewAttachMountItems(); + executeMountItemsOnViewAttach(); }; if (UiThreadUtil.isOnUiThread()) { @@ -245,8 +246,8 @@ private void addRootView(@NonNull final View rootView) { @UiThread @ThreadConfined(UI) - private void executeViewAttachMountItems() { - mMountItemExecutor.executeItems(mOnViewAttachItems); + private void executeMountItemsOnViewAttach() { + mMountItemExecutor.executeItems(mOnViewAttachMountItems); } /** @@ -319,7 +320,7 @@ public void stopSurface() { mRootViewManager = null; mMountItemExecutor = null; mThemedReactContext = null; - mOnViewAttachItems.clear(); + mOnViewAttachMountItems.clear(); if (ReactFeatureFlags.enableViewRecycling) { mViewManagerRegistry.onSurfaceStopped(mSurfaceId);