fix nullsafe FIXMEs for FabicUIManager.java and mark nullsafe (#50365)

Summary:
Pull Request resolved: #50365

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 FabricUIManager.java nullsafe

Reviewed By: GijsWeterings

Differential Revision: D71979601

fbshipit-source-id: 599d87f0783a566c837aa70975a62f5f908a2294

Author: javache

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/UIManager.kt

@@ -20,7 +20,7 @@ public interface UIManager : PerformanceCounter {
   @UiThread
   @ThreadConfined(ThreadConfined.UI)
   @Deprecated("")
-  public fun <T : View?> addRootView(rootView: T, initialProps: WritableMap?): Int
+  public fun <T : View> addRootView(rootView: T, initialProps: WritableMap?): Int
 
   /** Registers a new root view with width and height. */
   @AnyThread
@@ -92,7 +92,7 @@ public interface UIManager : PerformanceCounter {
    */
   @UiThread
   @ThreadConfined(ThreadConfined.UI)
-  public fun synchronouslyUpdateViewOnUIThread(reactTag: Int, props: ReadableMap?)
+  public fun synchronouslyUpdateViewOnUIThread(reactTag: Int, props: ReadableMap)
 
   /**
    * Dispatch an accessibility event to a view asynchronously.
@@ -109,14 +109,14 @@ public interface UIManager : PerformanceCounter {
    *
    * @param listener
    */
-  public fun addUIManagerEventListener(listener: UIManagerListener?)
+  public fun addUIManagerEventListener(listener: UIManagerListener)
 
   /**
    * Unregister a [UIManagerListener] from this UIManager to stop receiving lifecycle callbacks.
    *
    * @param listener
    */
-  public fun removeUIManagerEventListener(listener: UIManagerListener?)
+  public fun removeUIManagerEventListener(listener: UIManagerListener)
 
   /**
    * Resolves a view based on its reactTag. Do not mutate properties on this view that are already

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

@@ -25,10 +25,11 @@
 import android.view.View;
 import android.view.accessibility.AccessibilityEvent;
 import androidx.annotation.AnyThread;
-import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.annotation.UiThread;
 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.proguard.annotations.DoNotStripAny;
 import com.facebook.react.bridge.ColorPropConverter;
@@ -98,6 +99,7 @@
  * We instruct ProGuard not to strip out any fields or methods, because many of these methods are
  * only called through the JNI from Cxx so it appears that most of this class is "unused".
  */
+@Nullsafe(Nullsafe.Mode.LOCAL)
 @SuppressLint("MissingNativeLoadLibrary")
 @DoNotStripAny
 public class FabricUIManager
@@ -107,8 +109,7 @@ public class FabricUIManager
   // The IS_DEVELOPMENT_ENVIRONMENT variable is used to log extra data when running fabric in a
   // development environment. DO NOT ENABLE THIS ON PRODUCTION OR YOU WILL BE FIRED!
   public static final boolean IS_DEVELOPMENT_ENVIRONMENT = false && ReactBuildConfig.DEBUG;
-  // NULLSAFE_FIXME[Field Not Initialized]
-  public DevToolsReactPerfLogger mDevToolsReactPerfLogger;
+  public @Nullable DevToolsReactPerfLogger mDevToolsReactPerfLogger;
 
   private static final DevToolsReactPerfLogger.DevToolsReactPerfLoggerListener FABRIC_PERF_LOGGER =
       commitPoint -> {
@@ -161,27 +162,24 @@ public class FabricUIManager
   }
 
   @Nullable private FabricUIManagerBinding mBinding;
-  @NonNull private final ReactApplicationContext mReactApplicationContext;
-  @NonNull private final MountingManager mMountingManager;
-  @NonNull private final FabricEventDispatcher mEventDispatcher;
-  @NonNull private final MountItemDispatcher mMountItemDispatcher;
-  @NonNull private final ViewManagerRegistry mViewManagerRegistry;
+  private final ReactApplicationContext mReactApplicationContext;
+  private final MountingManager mMountingManager;
+  private final FabricEventDispatcher mEventDispatcher;
+  private final MountItemDispatcher mMountItemDispatcher;
+  private final ViewManagerRegistry mViewManagerRegistry;
 
-  @NonNull private final BatchEventDispatchedListener mBatchEventDispatchedListener;
+  private final BatchEventDispatchedListener mBatchEventDispatchedListener;
 
-  @NonNull
   private final CopyOnWriteArrayList<UIManagerListener> mListeners = new CopyOnWriteArrayList<>();
 
   private boolean mMountNotificationScheduled = false;
   private List<Integer> mSurfaceIdsWithPendingMountNotification = new ArrayList<>();
 
   @ThreadConfined(UI)
-  @NonNull
   private final DispatchUIFrameCallback mDispatchUIFrameCallback;
 
   /** Set of events sent synchronously during the current frame render. Cleared after each frame. */
   @ThreadConfined(UI)
-  @NonNull
   private final Set<SynchronousEvent> mSynchronousEvents = new HashSet<>();
 
   /**
@@ -219,9 +217,9 @@ public void executeItems(Queue<MountItem> items) {
   @Nullable private InteropUIBlockListener mInteropUIBlockListener;
 
   public FabricUIManager(
-      @NonNull ReactApplicationContext reactContext,
-      @NonNull ViewManagerRegistry viewManagerRegistry,
-      @NonNull BatchEventDispatchedListener batchEventDispatchedListener) {
+      ReactApplicationContext reactContext,
+      ViewManagerRegistry viewManagerRegistry,
+      BatchEventDispatchedListener batchEventDispatchedListener) {
     mDispatchUIFrameCallback = new DispatchUIFrameCallback(reactContext);
     mReactApplicationContext = reactContext;
     mMountingManager = new MountingManager(viewManagerRegistry, mMountItemExecutor);
@@ -239,8 +237,8 @@ public FabricUIManager(
   @UiThread
   @ThreadConfined(UI)
   @Deprecated
-  // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation]
-  public <T extends View> int addRootView(final T rootView, final WritableMap initialProps) {
+  public <T extends View> int addRootView(
+      final T rootView, final @Nullable WritableMap initialProps) {
     ReactSoftExceptionLogger.logSoftException(
         TAG,
         new IllegalViewOperationException(
@@ -257,19 +255,18 @@ public <T extends View> int addRootView(final T rootView, final WritableMap init
     if (ReactNativeFeatureFlags.enableFabricLogs()) {
       FLog.d(TAG, "Starting surface for module: %s and reactTag: %d", moduleName, rootTag);
     }
-    // NULLSAFE_FIXME[Nullable Dereference]
+    Assertions.assertNotNull(mBinding, "Binding in FabricUIManager is null");
     mBinding.startSurface(rootTag, moduleName, (NativeMap) initialProps);
     return rootTag;
   }
 
   @Override
   @AnyThread
   @ThreadConfined(ANY)
-  // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation]
   public <T extends View> int startSurface(
       final T rootView,
       final String moduleName,
-      final WritableMap initialProps,
+      final @Nullable WritableMap initialProps,
       int widthMeasureSpec,
       int heightMeasureSpec) {
     final int rootTag = ((ReactRoot) rootView).getRootViewTag();
@@ -288,7 +285,7 @@ public <T extends View> int startSurface(
     Point viewportOffset =
         UiThreadUtil.isOnUiThread() ? RootViewUtil.getViewportOffset(rootView) : new Point(0, 0);
 
-    // NULLSAFE_FIXME[Nullable Dereference]
+    Assertions.assertNotNull(mBinding, "Binding in FabricUIManager is null");
     mBinding.startSurfaceWithConstraints(
         rootTag,
         moduleName,
@@ -314,7 +311,7 @@ public void startSurface(
         new ThemedReactContext(
             mReactApplicationContext, context, surfaceHandler.getModuleName(), rootTag);
     mMountingManager.startSurface(rootTag, reactContext, rootView);
-    // NULLSAFE_FIXME[Nullable Dereference]
+    Assertions.assertNotNull(mBinding, "Binding in FabricUIManager is null");
     mBinding.startSurfaceWithSurfaceHandler(rootTag, surfaceHandler, rootView != null);
   }
 
@@ -339,7 +336,7 @@ public void stopSurface(final SurfaceHandlerBinding surfaceHandler) {
     }
 
     mMountingManager.stopSurface(surfaceHandler.getSurfaceId());
-    // NULLSAFE_FIXME[Nullable Dereference]
+    Assertions.assertNotNull(mBinding, "Binding in FabricUIManager is null");
     mBinding.stopSurfaceWithSurfaceHandler(surfaceHandler);
   }
 
@@ -359,7 +356,7 @@ public void stopSurface(final int surfaceID) {
     // Communicate stopSurface to Cxx - causes an empty ShadowTree to be committed,
     // but all mounting instructions will be ignored because stopSurface was called
     // on the MountingManager
-    // NULLSAFE_FIXME[Nullable Dereference]
+    Assertions.assertNotNull(mBinding, "Binding in FabricUIManager is null");
     mBinding.stopSurface(surfaceID);
   }
 
@@ -410,8 +407,9 @@ public void invalidate() {
     mReactApplicationContext.removeLifecycleEventListener(this);
     onHostPause();
 
-    // NULLSAFE_FIXME[Nullable Dereference]
-    mBinding.unregister();
+    if (mBinding != null) {
+      mBinding.unregister();
+    }
     mBinding = null;
 
     ViewManagerPropertyUpdater.clear();
@@ -455,7 +453,6 @@ public void prependUIBlock(UIBlock block) {
     }
   }
 
-  @NonNull
   private InteropUIBlockListener getInteropUIBlockListener() {
     if (mInteropUIBlockListener == null) {
       mInteropUIBlockListener = new InteropUIBlockListener();
@@ -542,12 +539,13 @@ private long measure(
         return 0;
       }
       context = surfaceMountingManager.getContext();
+      Assertions.assertNotNull(
+          context, "Context in SurfaceMountingManager is null. surfaceId: " + surfaceId);
     } else {
       context = mReactApplicationContext;
     }
 
     return mMountingManager.measure(
-        // NULLSAFE_FIXME[Parameter Not Nullable]
         context,
         componentName,
         localData,
@@ -581,13 +579,14 @@ private long measureMapBuffer(
         return 0;
       }
       context = surfaceMountingManager.getContext();
+      Assertions.assertNotNull(
+          context, "Context in SurfaceMountingManager is null. surfaceId: " + surfaceId);
     } else {
       context = mReactApplicationContext;
     }
 
     // TODO: replace ReadableNativeMap -> ReadableMapBuffer
     return mMountingManager.measureMapBuffer(
-        // NULLSAFE_FIXME[Parameter Not Nullable]
         context,
         componentName,
         localData,
@@ -624,13 +623,11 @@ public boolean getThemeData(int surfaceId, float[] defaultTextInputPadding) {
   }
 
   @Override
-  // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation]
   public void addUIManagerEventListener(UIManagerListener listener) {
     mListeners.add(listener);
   }
 
   @Override
-  // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation]
   public void removeUIManagerEventListener(UIManagerListener listener) {
     mListeners.remove(listener);
   }
@@ -645,9 +642,7 @@ public void synchronouslyUpdateViewOnUIThread(
   @Override
   @UiThread
   @ThreadConfined(UI)
-  // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation]
-  public void synchronouslyUpdateViewOnUIThread(
-      final int reactTag, @NonNull final ReadableMap props) {
+  public void synchronouslyUpdateViewOnUIThread(final int reactTag, final ReadableMap props) {
     UiThreadUtil.assertOnUiThread();
 
     int commitNumber = mCurrentSynchronousCommitNumber++;
@@ -678,7 +673,7 @@ public void synchronouslyUpdateViewOnUIThread(
     MountItem synchronousMountItem =
         new MountItem() {
           @Override
-          public void execute(@NonNull MountingManager mountingManager) {
+          public void execute(MountingManager mountingManager) {
             try {
               mountingManager.updateProps(reactTag, props);
             } catch (Exception ex) {
@@ -696,7 +691,6 @@ public int getSurfaceId() {
             return View.NO_ID;
           }
 
-          @NonNull
           @Override
           public String toString() {
             String propsString =
@@ -801,11 +795,14 @@ private void scheduleMountItem(
     // calls scheduleMountItems with a BatchMountItem.
     long scheduleMountItemStartTime = SystemClock.uptimeMillis();
     boolean isBatchMountItem = mountItem instanceof BatchMountItem;
-    boolean shouldSchedule =
-        // NULLSAFE_FIXME[Nullable Dereference]
-        (isBatchMountItem && !((BatchMountItem) mountItem).isBatchEmpty())
-            || (!isBatchMountItem && mountItem != null);
-
+    boolean shouldSchedule = false;
+    if (isBatchMountItem) {
+      BatchMountItem batchMountItem = (BatchMountItem) mountItem;
+      Assertions.assertNotNull(batchMountItem, "BatchMountItem is null");
+      shouldSchedule = !batchMountItem.isBatchEmpty();
+    } else {
+      shouldSchedule = mountItem != null;
+    }
     // In case of sync rendering, this could be called on the UI thread. Otherwise,
     // it should ~always be called on the JS thread.
     for (UIManagerListener listener : mListeners) {
@@ -821,7 +818,7 @@ private void scheduleMountItem(
     }
 
     if (shouldSchedule) {
-      // NULLSAFE_FIXME[Parameter Not Nullable]
+      Assertions.assertNotNull(mountItem, "MountItem is null");
       mMountItemDispatcher.addMountItem(mountItem);
       Runnable runnable =
           new GuardedRunnable(mReactApplicationContext) {
@@ -918,7 +915,7 @@ public void updateRootLayoutSpecs(
       doLeftAndRightSwapInRTL = I18nUtil.getInstance().doLeftAndRightSwapInRTL(context);
     }
 
-    // NULLSAFE_FIXME[Nullable Dereference]
+    Assertions.assertNotNull(mBinding, "Binding in FabricUIManager is null");
     mBinding.setConstraints(
         surfaceId,
         getMinSize(widthMeasureSpec),
@@ -932,11 +929,10 @@ public void updateRootLayoutSpecs(
   }
 
   @Override
-  public View resolveView(int reactTag) {
+  public @Nullable View resolveView(int reactTag) {
     UiThreadUtil.assertOnUiThread();
 
     SurfaceMountingManager surfaceManager = mMountingManager.getSurfaceManagerForView(reactTag);
-    // NULLSAFE_FIXME[Return Not Nullable]
     return surfaceManager == null ? null : surfaceManager.getView(reactTag);
   }
 
@@ -979,7 +975,7 @@ public void receiveEvent(
   public void receiveEvent(
       int surfaceId,
       int reactTag,
-      @NonNull String eventName,
+      String eventName,
       boolean canCoalesceEvent,
       @Nullable WritableMap params,
       @EventCategoryDef int eventCategory,
@@ -1032,7 +1028,6 @@ public void onHostResume() {
   }
 
   @Override
-  @NonNull
   public EventDispatcher getEventDispatcher() {
     return mEventDispatcher;
   }
@@ -1145,7 +1140,7 @@ public void setJSResponder(
     mMountItemDispatcher.addMountItem(
         new MountItem() {
           @Override
-          public void execute(@NonNull MountingManager mountingManager) {
+          public void execute(MountingManager mountingManager) {
             SurfaceMountingManager surfaceMountingManager =
                 mountingManager.getSurfaceManager(surfaceId);
             if (surfaceMountingManager != null) {
@@ -1162,7 +1157,6 @@ public int getSurfaceId() {
             return surfaceId;
           }
 
-          @NonNull
           @SuppressLint("DefaultLocale")
           @Override
           public String toString() {
@@ -1179,7 +1173,7 @@ public void clearJSResponder() {
     mMountItemDispatcher.addMountItem(
         new MountItem() {
           @Override
-          public void execute(@NonNull MountingManager mountingManager) {
+          public void execute(MountingManager mountingManager) {
             mountingManager.clearJSResponder();
           }
 
@@ -1188,7 +1182,6 @@ public int getSurfaceId() {
             return View.NO_ID;
           }
 
-          @NonNull
           @Override
           public String toString() {
             return "CLEAR_JS_RESPONDER";
@@ -1337,7 +1330,7 @@ private class DispatchUIFrameCallback extends GuardedFrameCallback {
     @ThreadConfined(UI)
     private boolean mIsScheduled = false;
 
-    private DispatchUIFrameCallback(@NonNull ReactContext reactContext) {
+    private DispatchUIFrameCallback(ReactContext reactContext) {
       super(reactContext);
     }