From cc6b430c3a2bdca9553c30f63e3f1b5f5639d5ac Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Fri, 15 Nov 2019 16:57:17 -0800 Subject: [PATCH] Annotate UIManager methods to document thread semantics Summary: Document which methods can be called on UI thread or ANY thread. In the future we should see if we can use only `ThreadConfined` or the AndroidX annotations instead of using both / choosing between them at each site. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D18532542 fbshipit-source-id: 3b5406ea5035615a0ebf83484bf8ec0747a6b6f7 --- .../com/facebook/react/bridge/UIManager.java | 20 ++++++++ .../react/fabric/FabricUIManager.java | 48 +++++++++++++++++++ .../fabric/mounting/MountingManager.java | 13 +++++ 3 files changed, 81 insertions(+) diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/UIManager.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/UIManager.java index 6f3c442634e..73e815306cf 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/UIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/UIManager.java @@ -7,12 +7,18 @@ package com.facebook.react.bridge; +import static com.facebook.infer.annotation.ThreadConfined.UI; + import android.view.View; import androidx.annotation.Nullable; +import androidx.annotation.UiThread; +import com.facebook.infer.annotation.ThreadConfined; public interface UIManager extends JSIModule, PerformanceCounter { /** Registers a new root view. */ + @UiThread + @ThreadConfined(UI) int addRootView( final T rootView, WritableMap initialProps, @Nullable String initialUITemplate); @@ -20,6 +26,8 @@ public interface UIManager extends JSIModule, PerformanceCounter { * Updates the layout specs of the RootShadowNode based on the Measure specs received by * parameters. */ + @UiThread + @ThreadConfined(UI) void updateRootLayoutSpecs(int rootTag, int widthMeasureSpec, int heightMeasureSpec); /** @@ -28,6 +36,8 @@ public interface UIManager extends JSIModule, PerformanceCounter { * *

Receiving commands as ints is deprecated and will be removed in a future release. * + *

Pre-Fabric, this is only called on the Native Module Thread. + * * @param reactTag {@link int} that identifies the view that will receive this command * @param commandId {@link int} command id * @param commandArgs {@link ReadableArray} parameters associated with the command @@ -38,6 +48,8 @@ public interface UIManager extends JSIModule, PerformanceCounter { * Dispatches the commandId received by parameter to the view associated with the reactTag. The * command will be processed in the UIThread. * + *

Pre-Fabric, this is only called on the Native Module Thread. + * * @param reactTag {@link int} that identifies the view that will receive this command * @param commandId {@link String} command id * @param commandArgs {@link ReadableArray} parameters associated with the command @@ -53,11 +65,15 @@ public interface UIManager extends JSIModule, PerformanceCounter { * @param tag {@link int} that identifies the view that will be updated * @param props {@link ReadableMap} props that should be immediately updated in view */ + @UiThread + @ThreadConfined(UI) void synchronouslyUpdateViewOnUIThread(int reactTag, ReadableMap props); /** * Dispatch an accessibility event to a view asynchronously. * + *

Pre-Fabric, this is only called on the Native Module Thread. + * * @param reactTag * @param eventType */ @@ -67,7 +83,11 @@ public interface UIManager extends JSIModule, PerformanceCounter { * When mounting instructions are scheduled on the UI thread, should they be executed immediately? * For Fabric. Should noop in pre-Fabric. * + *

This should only be called on the UI thread. + * * @param flag */ + @UiThread + @ThreadConfined(UI) void setAllowImmediateUIOperationExecution(boolean flag); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java index 9f4f9426e11..09f1bc08511 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -167,6 +167,8 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { // TODO (T47819352): Rename this to startSurface for consistency with xplat/iOS @Override + @UiThread + @ThreadConfined(UI) public int addRootView( final T rootView, final WritableMap initialProps, final @Nullable String initialUITemplate) { final int rootTag = ReactRootViewTagGenerator.getNextRootViewTag(); @@ -186,6 +188,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { return rootTag; } + @AnyThread @ThreadConfined(ANY) public int startSurface( final T rootView, @@ -219,6 +222,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { mEventDispatcher.dispatchAllEvents(); } + @AnyThread @ThreadConfined(ANY) public void stopSurface(int surfaceID) { mBinding.stopSurface(surfaceID); @@ -232,6 +236,8 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { // This is called on the JS thread (see CatalystInstanceImpl). @Override + @AnyThread + @ThreadConfined(ANY) public void onCatalystInstanceDestroy() { FLog.i(TAG, "FabricUIManager.onCatalystInstanceDestroy"); @@ -270,6 +276,8 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { @DoNotStrip @SuppressWarnings("unused") + @AnyThread + @ThreadConfined(ANY) private void preallocateView( int rootTag, int reactTag, @@ -294,6 +302,8 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { @DoNotStrip @SuppressWarnings("unused") + @AnyThread + @ThreadConfined(ANY) private MountItem createMountItem( String componentName, @Nullable ReadableMap props, @@ -318,30 +328,40 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { @DoNotStrip @SuppressWarnings("unused") + @AnyThread + @ThreadConfined(ANY) private MountItem removeMountItem(int reactTag, int parentReactTag, int index) { return new RemoveMountItem(reactTag, parentReactTag, index); } @DoNotStrip @SuppressWarnings("unused") + @AnyThread + @ThreadConfined(ANY) private MountItem insertMountItem(int reactTag, int parentReactTag, int index) { return new InsertMountItem(reactTag, parentReactTag, index); } @DoNotStrip @SuppressWarnings("unused") + @AnyThread + @ThreadConfined(ANY) private MountItem deleteMountItem(int reactTag) { return new DeleteMountItem(reactTag); } @DoNotStrip @SuppressWarnings("unused") + @AnyThread + @ThreadConfined(ANY) private MountItem removeDeleteMultiMountItem(int[] metadata) { return new RemoveDeleteMultiMountItem(metadata); } @DoNotStrip @SuppressWarnings("unused") + @AnyThread + @ThreadConfined(ANY) private MountItem updateLayoutMountItem( int reactTag, int x, int y, int width, int height, int layoutDirection) { return new UpdateLayoutMountItem(reactTag, x, y, width, height, layoutDirection); @@ -349,36 +369,48 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { @DoNotStrip @SuppressWarnings("unused") + @AnyThread + @ThreadConfined(ANY) private MountItem updatePaddingMountItem(int reactTag, int left, int top, int right, int bottom) { return new UpdatePaddingMountItem(reactTag, left, top, right, bottom); } @DoNotStrip @SuppressWarnings("unused") + @AnyThread + @ThreadConfined(ANY) private MountItem updatePropsMountItem(int reactTag, ReadableMap map) { return new UpdatePropsMountItem(reactTag, map); } @DoNotStrip @SuppressWarnings("unused") + @AnyThread + @ThreadConfined(ANY) private MountItem updateLocalDataMountItem(int reactTag, ReadableMap newLocalData) { return new UpdateLocalDataMountItem(reactTag, newLocalData); } @DoNotStrip @SuppressWarnings("unused") + @AnyThread + @ThreadConfined(ANY) private MountItem updateStateMountItem(int reactTag, @Nullable Object stateWrapper) { return new UpdateStateMountItem(reactTag, (StateWrapper) stateWrapper); } @DoNotStrip @SuppressWarnings("unused") + @AnyThread + @ThreadConfined(ANY) private MountItem updateEventEmitterMountItem(int reactTag, Object eventEmitter) { return new UpdateEventEmitterMountItem(reactTag, (EventEmitterWrapper) eventEmitter); } @DoNotStrip @SuppressWarnings("unused") + @AnyThread + @ThreadConfined(ANY) private MountItem createBatchMountItem(MountItem[] items, int size, int commitNumber) { return new BatchMountItem(items, size, commitNumber); } @@ -431,6 +463,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { } @Override + @UiThread @ThreadConfined(UI) public void synchronouslyUpdateViewOnUIThread(int reactTag, @NonNull ReadableMap props) { UiThreadUtil.assertOnUiThread(); @@ -458,6 +491,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { * * */ @Override + @UiThread @ThreadConfined(UI) public void setAllowImmediateUIOperationExecution(boolean flag) { mImmediatelyExecutedMountItemsOnUI = flag; @@ -469,6 +503,8 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { */ @DoNotStrip @SuppressWarnings("unused") + @AnyThread + @ThreadConfined(ANY) private void scheduleMountItem( @NonNull final MountItem mountItem, int commitNumber, @@ -528,6 +564,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { } @UiThread + @ThreadConfined(UI) private void dispatchMountItems() { mRunStartTime = SystemClock.uptimeMillis(); @@ -582,6 +619,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { } @UiThread + @ThreadConfined(UI) private void dispatchPreMountItems(long frameTimeNanos) { Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "FabricUIManager::premountViews"); @@ -612,6 +650,8 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { * Updates the layout metrics of the root view based on the Measure specs received by parameters. */ @Override + @UiThread + @ThreadConfined(UI) public void updateRootLayoutSpecs( final int rootTag, final int widthMeasureSpec, final int heightMeasureSpec) { @@ -655,6 +695,8 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { @Deprecated @Override + @AnyThread + @ThreadConfined(ANY) public void dispatchCommand( final int reactTag, final int commandId, @Nullable final ReadableArray commandArgs) { synchronized (mMountItemsLock) { @@ -663,6 +705,8 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { } @Override + @AnyThread + @ThreadConfined(ANY) public void dispatchCommand( final int reactTag, final String commandId, @Nullable final ReadableArray commandArgs) { synchronized (mMountItemsLock) { @@ -671,6 +715,8 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { } @Override + @AnyThread + @ThreadConfined(ANY) public void sendAccessibilityEvent(int reactTag, int eventType) { synchronized (mMountItemsLock) { mMountItems.add(new SendAccessibilityEvent(reactTag, eventType)); @@ -747,6 +793,8 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { } @Override + @UiThread + @ThreadConfined(UI) public void doFrameGuarded(long frameTimeNanos) { if (!mIsMountingEnabled || mDestroyed) { FLog.w( diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java index 381b58a81f0..30a74cfa55a 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java @@ -7,6 +7,9 @@ package com.facebook.react.fabric.mounting; +import static com.facebook.infer.annotation.ThreadConfined.ANY; +import static com.facebook.infer.annotation.ThreadConfined.UI; + import android.content.Context; import android.view.View; import android.view.ViewGroup; @@ -16,6 +19,7 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.UiThread; import com.facebook.infer.annotation.Assertions; +import com.facebook.infer.annotation.ThreadConfined; import com.facebook.react.bridge.ReactSoftException; import com.facebook.react.bridge.ReadableArray; import com.facebook.react.bridge.ReadableMap; @@ -55,6 +59,14 @@ public class MountingManager { mViewManagerRegistry = viewManagerRegistry; } + /** + * This mutates the rootView, which is an Android View, so this should only be called on the UI + * thread. + * + * @param reactRootTag + * @param rootView + */ + @ThreadConfined(UI) public void addRootView(int reactRootTag, @NonNull View rootView) { if (rootView.getId() != View.NO_ID) { throw new IllegalViewOperationException( @@ -490,6 +502,7 @@ public class MountingManager { } @AnyThread + @ThreadConfined(ANY) public @Nullable EventEmitterWrapper getEventEmitter(int reactTag) { ViewState viewState = getNullableViewState(reactTag); return viewState == null ? null : viewState.mEventEmitter;