Skip to content

Commit 88f3a9b

Browse files
GijsWeteringsfacebook-github-bot
authored andcommitted
fix nullsafe FIXMEs for SurfaceMountingManager.java and mark nullsafe (#50369)
Summary: Pull Request resolved: #50369 Gone trough all the FIXMEs added in the previous diff by the nullsafe tool, marked the class as nullsafe and ensured no remaining violations. Changelog: [Android][Fixed] Made SurfaceMountingManager.java nullsafe Reviewed By: javache Differential Revision: D71979577
1 parent 7bdb867 commit 88f3a9b

File tree

4 files changed

+53
-48
lines changed

4 files changed

+53
-48
lines changed

Diff for: packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -729,7 +729,7 @@ private void preallocateView(
729729
int rootTag,
730730
int reactTag,
731731
final String componentName,
732-
@Nullable Object props,
732+
ReadableMap props,
733733
@Nullable Object stateWrapper,
734734
boolean isLayoutable) {
735735
mMountItemDispatcher.addPreAllocateMountItem(

Diff for: packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java

+50-45
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@
1414
import android.view.ViewGroup;
1515
import android.view.ViewParent;
1616
import androidx.annotation.AnyThread;
17-
import androidx.annotation.NonNull;
1817
import androidx.annotation.Nullable;
1918
import androidx.annotation.UiThread;
2019
import androidx.collection.SparseArrayCompat;
2120
import com.facebook.common.logging.FLog;
2221
import com.facebook.infer.annotation.Assertions;
22+
import com.facebook.infer.annotation.Nullsafe;
2323
import com.facebook.infer.annotation.ThreadConfined;
2424
import com.facebook.react.bridge.ReactNoCrashSoftException;
2525
import com.facebook.react.bridge.ReactSoftExceptionLogger;
@@ -56,6 +56,7 @@
5656
import java.util.Set;
5757
import java.util.concurrent.ConcurrentHashMap;
5858

59+
@Nullsafe(Nullsafe.Mode.LOCAL)
5960
public class SurfaceMountingManager {
6061
public static final String TAG = SurfaceMountingManager.class.getSimpleName();
6162

@@ -66,14 +67,15 @@ public class SurfaceMountingManager {
6667

6768
@Nullable private ThemedReactContext mThemedReactContext;
6869

69-
// These are all non-null, until StopSurface is called
70-
private ConcurrentHashMap<Integer, ViewState> mTagToViewState =
70+
private final ConcurrentHashMap<Integer, ViewState> mTagToViewState =
7171
new ConcurrentHashMap<>(); // any thread
72-
private Queue<MountItem> mOnViewAttachMountItems = new ArrayDeque<>();
72+
private final Queue<MountItem> mOnViewAttachMountItems = new ArrayDeque<>();
7373
private JSResponderHandler mJSResponderHandler;
7474
private ViewManagerRegistry mViewManagerRegistry;
75-
private RootViewManager mRootViewManager;
76-
private MountItemExecutor mMountItemExecutor;
75+
76+
// These are non-null, until StopSurface is called
77+
private @Nullable RootViewManager mRootViewManager;
78+
private @Nullable MountItemExecutor mMountItemExecutor;
7779

7880
@ThreadConfined(UI)
7981
private final Set<Integer> mErroneouslyReaddedReactTags = new HashSet<>();
@@ -90,18 +92,17 @@ public class SurfaceMountingManager {
9092
private final Set<Integer> mViewsToDeleteAfterTouchFinishes = new HashSet<>();
9193

9294
// This is null *until* StopSurface is called.
93-
// NULLSAFE_FIXME[Field Not Initialized]
94-
private SparseArrayCompat<Object> mTagSetForStoppedSurface;
95+
private @Nullable SparseArrayCompat<Object> mTagSetForStoppedSurface;
9596

9697
private final int mSurfaceId;
9798

9899
public SurfaceMountingManager(
99100
int surfaceId,
100-
@NonNull JSResponderHandler jsResponderHandler,
101-
@NonNull ViewManagerRegistry viewManagerRegistry,
102-
@NonNull RootViewManager rootViewManager,
103-
@NonNull MountItemExecutor mountItemExecutor,
104-
@NonNull ThemedReactContext reactContext) {
101+
JSResponderHandler jsResponderHandler,
102+
ViewManagerRegistry viewManagerRegistry,
103+
RootViewManager rootViewManager,
104+
MountItemExecutor mountItemExecutor,
105+
ThemedReactContext reactContext) {
105106
mSurfaceId = surfaceId;
106107
mJSResponderHandler = jsResponderHandler;
107108
mViewManagerRegistry = viewManagerRegistry;
@@ -182,11 +183,13 @@ public void scheduleMountItemOnViewAttach(MountItem item) {
182183
}
183184

184185
@AnyThread
185-
private void addRootView(@NonNull final View rootView) {
186+
private void addRootView(final View rootView) {
186187
if (isStopped()) {
187188
return;
188189
}
189-
190+
// mRootViewManager is non-null until StopSurface is called, which we know hasn't happened yet
191+
// due to the isStopped() check above
192+
Assertions.assertNotNull(mRootViewManager);
190193
mTagToViewState.put(mSurfaceId, new ViewState(mSurfaceId, rootView, mRootViewManager, true));
191194

192195
Runnable runnable =
@@ -253,6 +256,10 @@ private void addRootView(@NonNull final View rootView) {
253256
@UiThread
254257
@ThreadConfined(UI)
255258
private void executeMountItemsOnViewAttach() {
259+
Assertions.assertNotNull(
260+
mMountItemExecutor,
261+
"executeMountItemsOnViewAttach was called after StopSurface, and this wasn't checked by the"
262+
+ " caller with isStopped().");
256263
mMountItemExecutor.executeItems(mOnViewAttachMountItems);
257264
}
258265

@@ -315,15 +322,12 @@ public void stopSurface() {
315322
}
316323

317324
// Evict all views from cache and memory
318-
// TODO: clear instead of nulling out to simplify null-safety in this class
319-
// NULLSAFE_FIXME[Field Not Nullable]
320-
mTagToViewState = null;
321-
// NULLSAFE_FIXME[Field Not Nullable]
322-
mJSResponderHandler = null;
323-
// NULLSAFE_FIXME[Field Not Nullable]
325+
mTagToViewState.clear();
326+
mJSResponderHandler.clearJSResponder();
327+
324328
mRootViewManager = null;
325-
// NULLSAFE_FIXME[Field Not Nullable]
326329
mMountItemExecutor = null;
330+
327331
mThemedReactContext = null;
328332
mOnViewAttachMountItems.clear();
329333

@@ -616,9 +620,9 @@ public void run() {
616620

617621
@UiThread
618622
public void createView(
619-
@NonNull String componentName,
623+
String componentName,
620624
int reactTag,
621-
@Nullable ReadableMap props,
625+
ReadableMap props,
622626
@Nullable StateWrapper stateWrapper,
623627
@Nullable EventEmitterWrapper eventEmitterWrapper,
624628
boolean isLayoutable) {
@@ -654,17 +658,16 @@ public void createView(
654658
*/
655659
@UiThread
656660
public void createViewUnsafe(
657-
@NonNull String componentName,
661+
String componentName,
658662
int reactTag,
659-
@Nullable ReadableMap props,
663+
ReadableMap props,
660664
@Nullable StateWrapper stateWrapper,
661665
@Nullable EventEmitterWrapper eventEmitterWrapper,
662666
boolean isLayoutable) {
663667
Systrace.beginSection(
664668
Systrace.TRACE_TAG_REACT_JAVA_BRIDGE,
665669
"SurfaceMountingManager::createViewUnsafe(" + componentName + ")");
666670
try {
667-
// NULLSAFE_FIXME[Parameter Not Nullable]
668671
ReactStylesDiffMap propMap = new ReactStylesDiffMap(props);
669672

670673
ViewState viewState = new ViewState(reactTag);
@@ -675,10 +678,16 @@ public void createViewUnsafe(
675678

676679
if (isLayoutable) {
677680
ViewManager viewManager = mViewManagerRegistry.get(componentName);
681+
if (mThemedReactContext == null) {
682+
throw new IllegalStateException(
683+
"Unable to create view for tag ["
684+
+ reactTag
685+
+ "], mThemedReactContext is null. This can happen if you are creating a view"
686+
+ " after the surface has been stopped.");
687+
}
678688
// View Managers are responsible for dealing with inital state and props.
679689
viewState.mView =
680690
viewManager.createView(
681-
// NULLSAFE_FIXME[Parameter Not Nullable]
682691
reactTag, mThemedReactContext, propMap, stateWrapper, mJSResponderHandler);
683692
viewState.mViewManager = viewManager;
684693
}
@@ -733,8 +742,7 @@ public void receiveCommand(int reactTag, int commandId, @Nullable ReadableArray
733742
viewState.mViewManager.receiveCommand(viewState.mView, commandId, commandArgs);
734743
}
735744

736-
public void receiveCommand(
737-
int reactTag, @NonNull String commandId, @Nullable ReadableArray commandArgs) {
745+
public void receiveCommand(int reactTag, String commandId, @Nullable ReadableArray commandArgs) {
738746
if (isStopped()) {
739747
return;
740748
}
@@ -936,12 +944,12 @@ public void updateState(final int reactTag, @Nullable StateWrapper stateWrapper)
936944
if (viewManager == null) {
937945
throw new IllegalStateException("Unable to find ViewManager for tag: " + reactTag);
938946
}
939-
Object extraData =
940-
// NULLSAFE_FIXME[Parameter Not Nullable]
941-
viewManager.updateState(viewState.mView, viewState.mCurrentProps, stateWrapper);
942-
if (extraData != null) {
943-
// NULLSAFE_FIXME[Parameter Not Nullable]
944-
viewManager.updateExtraData(viewState.mView, extraData);
947+
if (viewState.mView != null && viewState.mCurrentProps != null && stateWrapper != null) {
948+
Object extraData =
949+
viewManager.updateState(viewState.mView, viewState.mCurrentProps, stateWrapper);
950+
if (extraData != null) {
951+
viewManager.updateExtraData(viewState.mView, extraData);
952+
}
945953
}
946954

947955
// Immediately clear native side of previous state wrapper. This causes the State object in C++
@@ -953,7 +961,7 @@ public void updateState(final int reactTag, @Nullable StateWrapper stateWrapper)
953961

954962
/** We update the event emitter from the main thread when the view is mounted. */
955963
@UiThread
956-
public void updateEventEmitter(int reactTag, @NonNull EventEmitterWrapper eventEmitter) {
964+
public void updateEventEmitter(int reactTag, EventEmitterWrapper eventEmitter) {
957965
UiThreadUtil.assertOnUiThread();
958966
if (isStopped()) {
959967
return;
@@ -1034,8 +1042,7 @@ private void onViewStateDeleted(ViewState viewState) {
10341042

10351043
// For non-root views we notify viewmanager with {@link ViewManager#onDropInstance}
10361044
ViewManager viewManager = viewState.mViewManager;
1037-
if (!viewState.mIsRoot && viewManager != null) {
1038-
// NULLSAFE_FIXME[Parameter Not Nullable]
1045+
if (!viewState.mIsRoot && viewManager != null && viewState.mView != null) {
10391046
viewManager.onDropViewInstance(viewState.mView);
10401047
}
10411048
}
@@ -1076,9 +1083,9 @@ public void deleteView(int reactTag) {
10761083

10771084
@UiThread
10781085
public void preallocateView(
1079-
@NonNull String componentName,
1086+
String componentName,
10801087
int reactTag,
1081-
@Nullable ReadableMap props,
1088+
ReadableMap props,
10821089
@Nullable StateWrapper stateWrapper,
10831090
boolean isLayoutable) {
10841091
UiThreadUtil.assertOnUiThread();
@@ -1113,7 +1120,7 @@ public View getView(int reactTag) {
11131120
return view;
11141121
}
11151122

1116-
private @NonNull ViewState getViewState(int tag) {
1123+
private ViewState getViewState(int tag) {
11171124
ViewState viewState = mTagToViewState.get(tag);
11181125
if (viewState == null) {
11191126
throw new RetryableMountingLayerException(
@@ -1131,8 +1138,7 @@ public View getView(int reactTag) {
11311138
}
11321139

11331140
@SuppressWarnings("unchecked") // prevents unchecked conversion warn of the <ViewGroup> type
1134-
private static @NonNull IViewGroupManager<ViewGroup> getViewGroupManager(
1135-
@NonNull ViewState viewState) {
1141+
private static IViewGroupManager<ViewGroup> getViewGroupManager(ViewState viewState) {
11361142
if (viewState.mViewManager == null) {
11371143
throw new IllegalStateException("Unable to find ViewManager for view: " + viewState);
11381144
}
@@ -1237,7 +1243,6 @@ private ViewState(
12371243
mViewManager = viewManager;
12381244
}
12391245

1240-
@NonNull
12411246
@Override
12421247
public String toString() {
12431248
boolean isLayoutOnly = mViewManager == null;

Diff for: packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/MountItemFactory.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ internal object MountItemFactory {
4747
surfaceId: Int,
4848
reactTag: Int,
4949
component: String,
50-
props: ReadableMap?,
50+
props: ReadableMap,
5151
stateWrapper: StateWrapper?,
5252
isLayoutable: Boolean
5353
): MountItem =

Diff for: packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/PreAllocateViewMountItem.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ internal class PreAllocateViewMountItem(
1919
private val surfaceId: Int,
2020
private val reactTag: Int,
2121
component: String,
22-
private val props: ReadableMap?,
22+
private val props: ReadableMap,
2323
private val stateWrapper: StateWrapper?,
2424
private val isLayoutable: Boolean
2525
) : MountItem {

0 commit comments

Comments
 (0)