Skip to content

Commit

Permalink
Clarify threading of OnViewAttach mount items (facebook#41704)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
javache authored and facebook-github-bot committed Nov 30, 2023
1 parent 05ed007 commit 322d9e1
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,14 @@
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;
import java.util.Queue;
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();
Expand All @@ -74,7 +74,7 @@ public class SurfaceMountingManager {
// These are all non-null, until StopSurface is called
private ConcurrentHashMap<Integer, ViewState> mTagToViewState =
new ConcurrentHashMap<>(); // any thread
private ConcurrentLinkedQueue<MountItem> mOnViewAttachItems = new ConcurrentLinkedQueue<>();
private Queue<MountItem> mOnViewAttachMountItems = new ArrayDeque<>();
private JSResponderHandler mJSResponderHandler;
private ViewManagerRegistry mViewManagerRegistry;
private RootViewManager mRootViewManager;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -233,7 +234,7 @@ private void addRootView(@NonNull final View rootView) {
}
mRootViewAttached = true;

executeViewAttachMountItems();
executeMountItemsOnViewAttach();
};

if (UiThreadUtil.isOnUiThread()) {
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -319,7 +320,7 @@ public void stopSurface() {
mRootViewManager = null;
mMountItemExecutor = null;
mThemedReactContext = null;
mOnViewAttachItems.clear();
mOnViewAttachMountItems.clear();

if (ReactFeatureFlags.enableViewRecycling) {
mViewManagerRegistry.onSurfaceStopped(mSurfaceId);
Expand Down

0 comments on commit 322d9e1

Please sign in to comment.