Skip to content

fix nullsafe FIXMEs for SurfaceMountingManager.java and mark nullsafe #50369

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -728,7 +728,7 @@ private void preallocateView(
int rootTag,
int reactTag,
final String componentName,
@Nullable Object props,
ReadableMap props,
@Nullable Object stateWrapper,
boolean isLayoutable) {
mMountItemDispatcher.addPreAllocateMountItem(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@
import android.view.ViewGroup;
import android.view.ViewParent;
import androidx.annotation.AnyThread;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.UiThread;
import androidx.collection.SparseArrayCompat;
import com.facebook.common.logging.FLog;
import com.facebook.infer.annotation.Assertions;
import com.facebook.infer.annotation.Nullsafe;
import com.facebook.infer.annotation.ThreadConfined;
import com.facebook.react.bridge.ReactNoCrashSoftException;
import com.facebook.react.bridge.ReactSoftExceptionLogger;
Expand Down Expand Up @@ -56,6 +56,7 @@
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;

@Nullsafe(Nullsafe.Mode.LOCAL)
public class SurfaceMountingManager {
public static final String TAG = SurfaceMountingManager.class.getSimpleName();

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

@Nullable private ThemedReactContext mThemedReactContext;

// These are all non-null, until StopSurface is called
private ConcurrentHashMap<Integer, ViewState> mTagToViewState =
private final ConcurrentHashMap<Integer, ViewState> mTagToViewState =
new ConcurrentHashMap<>(); // any thread
private Queue<MountItem> mOnViewAttachMountItems = new ArrayDeque<>();
private final Queue<MountItem> mOnViewAttachMountItems = new ArrayDeque<>();
private JSResponderHandler mJSResponderHandler;
private ViewManagerRegistry mViewManagerRegistry;
private RootViewManager mRootViewManager;
private MountItemExecutor mMountItemExecutor;

// These are non-null, until StopSurface is called
private @Nullable RootViewManager mRootViewManager;
private @Nullable MountItemExecutor mMountItemExecutor;

@ThreadConfined(UI)
private final Set<Integer> mErroneouslyReaddedReactTags = new HashSet<>();
Expand All @@ -90,17 +92,17 @@ public class SurfaceMountingManager {
private final Set<Integer> mViewsToDeleteAfterTouchFinishes = new HashSet<>();

// This is null *until* StopSurface is called.
private SparseArrayCompat<Object> mTagSetForStoppedSurface;
private @Nullable SparseArrayCompat<Object> mTagSetForStoppedSurface;

private final int mSurfaceId;

public SurfaceMountingManager(
int surfaceId,
@NonNull JSResponderHandler jsResponderHandler,
@NonNull ViewManagerRegistry viewManagerRegistry,
@NonNull RootViewManager rootViewManager,
@NonNull MountItemExecutor mountItemExecutor,
@NonNull ThemedReactContext reactContext) {
JSResponderHandler jsResponderHandler,
ViewManagerRegistry viewManagerRegistry,
RootViewManager rootViewManager,
MountItemExecutor mountItemExecutor,
ThemedReactContext reactContext) {
mSurfaceId = surfaceId;
mJSResponderHandler = jsResponderHandler;
mViewManagerRegistry = viewManagerRegistry;
Expand Down Expand Up @@ -181,11 +183,13 @@ public void scheduleMountItemOnViewAttach(MountItem item) {
}

@AnyThread
private void addRootView(@NonNull final View rootView) {
private void addRootView(final View rootView) {
if (isStopped()) {
return;
}

// mRootViewManager is non-null until StopSurface is called, which we know hasn't happened yet
// due to the isStopped() check above
Assertions.assertNotNull(mRootViewManager);
mTagToViewState.put(mSurfaceId, new ViewState(mSurfaceId, rootView, mRootViewManager, true));

Runnable runnable =
Expand Down Expand Up @@ -252,6 +256,10 @@ private void addRootView(@NonNull final View rootView) {
@UiThread
@ThreadConfined(UI)
private void executeMountItemsOnViewAttach() {
Assertions.assertNotNull(
mMountItemExecutor,
"executeMountItemsOnViewAttach was called after StopSurface, and this wasn't checked by the"
+ " caller with isStopped().");
mMountItemExecutor.executeItems(mOnViewAttachMountItems);
}

Expand Down Expand Up @@ -314,11 +322,12 @@ public void stopSurface() {
}

// Evict all views from cache and memory
// TODO: clear instead of nulling out to simplify null-safety in this class
mTagToViewState = null;
mJSResponderHandler = null;
mTagToViewState.clear();
mJSResponderHandler.clearJSResponder();

mRootViewManager = null;
mMountItemExecutor = null;

mThemedReactContext = null;
mOnViewAttachMountItems.clear();

Expand Down Expand Up @@ -611,9 +620,9 @@ public void run() {

@UiThread
public void createView(
@NonNull String componentName,
String componentName,
int reactTag,
@Nullable ReadableMap props,
ReadableMap props,
@Nullable StateWrapper stateWrapper,
@Nullable EventEmitterWrapper eventEmitterWrapper,
boolean isLayoutable) {
Expand Down Expand Up @@ -649,9 +658,9 @@ public void createView(
*/
@UiThread
public void createViewUnsafe(
@NonNull String componentName,
String componentName,
int reactTag,
@Nullable ReadableMap props,
ReadableMap props,
@Nullable StateWrapper stateWrapper,
@Nullable EventEmitterWrapper eventEmitterWrapper,
boolean isLayoutable) {
Expand All @@ -669,6 +678,13 @@ public void createViewUnsafe(

if (isLayoutable) {
ViewManager viewManager = mViewManagerRegistry.get(componentName);
if (mThemedReactContext == null) {
throw new IllegalStateException(
"Unable to create view for tag ["
+ reactTag
+ "], mThemedReactContext is null. This can happen if you are creating a view"
+ " after the surface has been stopped.");
}
// View Managers are responsible for dealing with inital state and props.
viewState.mView =
viewManager.createView(
Expand Down Expand Up @@ -726,8 +742,7 @@ public void receiveCommand(int reactTag, int commandId, @Nullable ReadableArray
viewState.mViewManager.receiveCommand(viewState.mView, commandId, commandArgs);
}

public void receiveCommand(
int reactTag, @NonNull String commandId, @Nullable ReadableArray commandArgs) {
public void receiveCommand(int reactTag, String commandId, @Nullable ReadableArray commandArgs) {
if (isStopped()) {
return;
}
Expand Down Expand Up @@ -929,10 +944,12 @@ public void updateState(final int reactTag, @Nullable StateWrapper stateWrapper)
if (viewManager == null) {
throw new IllegalStateException("Unable to find ViewManager for tag: " + reactTag);
}
Object extraData =
viewManager.updateState(viewState.mView, viewState.mCurrentProps, stateWrapper);
if (extraData != null) {
viewManager.updateExtraData(viewState.mView, extraData);
if (viewState.mView != null && viewState.mCurrentProps != null && stateWrapper != null) {
Object extraData =
viewManager.updateState(viewState.mView, viewState.mCurrentProps, stateWrapper);
if (extraData != null) {
viewManager.updateExtraData(viewState.mView, extraData);
}
}

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

/** We update the event emitter from the main thread when the view is mounted. */
@UiThread
public void updateEventEmitter(int reactTag, @NonNull EventEmitterWrapper eventEmitter) {
public void updateEventEmitter(int reactTag, EventEmitterWrapper eventEmitter) {
UiThreadUtil.assertOnUiThread();
if (isStopped()) {
return;
Expand Down Expand Up @@ -1025,7 +1042,7 @@ private void onViewStateDeleted(ViewState viewState) {

// For non-root views we notify viewmanager with {@link ViewManager#onDropInstance}
ViewManager viewManager = viewState.mViewManager;
if (!viewState.mIsRoot && viewManager != null) {
if (!viewState.mIsRoot && viewManager != null && viewState.mView != null) {
viewManager.onDropViewInstance(viewState.mView);
}
}
Expand Down Expand Up @@ -1066,9 +1083,9 @@ public void deleteView(int reactTag) {

@UiThread
public void preallocateView(
@NonNull String componentName,
String componentName,
int reactTag,
@Nullable ReadableMap props,
ReadableMap props,
@Nullable StateWrapper stateWrapper,
boolean isLayoutable) {
UiThreadUtil.assertOnUiThread();
Expand Down Expand Up @@ -1103,7 +1120,7 @@ public View getView(int reactTag) {
return view;
}

private @NonNull ViewState getViewState(int tag) {
private ViewState getViewState(int tag) {
ViewState viewState = mTagToViewState.get(tag);
if (viewState == null) {
throw new RetryableMountingLayerException(
Expand All @@ -1121,8 +1138,7 @@ public View getView(int reactTag) {
}

@SuppressWarnings("unchecked") // prevents unchecked conversion warn of the <ViewGroup> type
private static @NonNull IViewGroupManager<ViewGroup> getViewGroupManager(
@NonNull ViewState viewState) {
private static IViewGroupManager<ViewGroup> getViewGroupManager(ViewState viewState) {
if (viewState.mViewManager == null) {
throw new IllegalStateException("Unable to find ViewManager for view: " + viewState);
}
Expand Down Expand Up @@ -1227,7 +1243,6 @@ private ViewState(
mViewManager = viewManager;
}

@NonNull
@Override
public String toString() {
boolean isLayoutOnly = mViewManager == null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ internal object MountItemFactory {
surfaceId: Int,
reactTag: Int,
component: String,
props: ReadableMap?,
props: ReadableMap,
stateWrapper: StateWrapper?,
isLayoutable: Boolean
): MountItem =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ internal class PreAllocateViewMountItem(
private val surfaceId: Int,
private val reactTag: Int,
component: String,
private val props: ReadableMap?,
private val props: ReadableMap,
private val stateWrapper: StateWrapper?,
private val isLayoutable: Boolean
) : MountItem {
Expand Down
Loading