Revert D71979601: fix nullsafe FIXMEs for FabicUIManager.java and mark nullsafe

Differential Revision:
D71979601

Original commit changeset: 599d87f0783a

Original Phabricator Diff: D71979601

fbshipit-source-id: e1f88e18bb3cf0d99de00b750c399eb0a4221e13

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,11 +25,10 @@ import android.os.SystemClock;
 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;
@@ -99,7 +98,6 @@ import java.util.concurrent.CopyOnWriteArrayList;
  * 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
@@ -109,7 +107,8 @@ 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;
-  public @Nullable DevToolsReactPerfLogger mDevToolsReactPerfLogger;
+  // NULLSAFE_FIXME[Field Not Initialized]
+  public DevToolsReactPerfLogger mDevToolsReactPerfLogger;
 
   private static final DevToolsReactPerfLogger.DevToolsReactPerfLoggerListener FABRIC_PERF_LOGGER =
       commitPoint -> {
@@ -162,24 +161,27 @@ public class FabricUIManager
   }
 
   @Nullable private FabricUIManagerBinding mBinding;
-  private final ReactApplicationContext mReactApplicationContext;
-  private final MountingManager mMountingManager;
-  private final FabricEventDispatcher mEventDispatcher;
-  private final MountItemDispatcher mMountItemDispatcher;
-  private final ViewManagerRegistry mViewManagerRegistry;
+  @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 BatchEventDispatchedListener mBatchEventDispatchedListener;
+  @NonNull 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<>();
 
   /**
@@ -217,9 +219,9 @@ public class FabricUIManager
   @Nullable private InteropUIBlockListener mInteropUIBlockListener;
 
   public FabricUIManager(
-      ReactApplicationContext reactContext,
-      ViewManagerRegistry viewManagerRegistry,
-      BatchEventDispatchedListener batchEventDispatchedListener) {
+      @NonNull ReactApplicationContext reactContext,
+      @NonNull ViewManagerRegistry viewManagerRegistry,
+      @NonNull BatchEventDispatchedListener batchEventDispatchedListener) {
     mDispatchUIFrameCallback = new DispatchUIFrameCallback(reactContext);
     mReactApplicationContext = reactContext;
     mMountingManager = new MountingManager(viewManagerRegistry, mMountItemExecutor);
@@ -237,8 +239,8 @@ public class FabricUIManager
   @UiThread
   @ThreadConfined(UI)
   @Deprecated
-  public <T extends View> int addRootView(
-      final T rootView, final @Nullable WritableMap initialProps) {
+  // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation]
+  public <T extends View> int addRootView(final T rootView, final WritableMap initialProps) {
     ReactSoftExceptionLogger.logSoftException(
         TAG,
         new IllegalViewOperationException(
@@ -255,7 +257,7 @@ public class FabricUIManager
     if (ReactNativeFeatureFlags.enableFabricLogs()) {
       FLog.d(TAG, "Starting surface for module: %s and reactTag: %d", moduleName, rootTag);
     }
-    Assertions.assertNotNull(mBinding, "Binding in FabricUIManager is null");
+    // NULLSAFE_FIXME[Nullable Dereference]
     mBinding.startSurface(rootTag, moduleName, (NativeMap) initialProps);
     return rootTag;
   }
@@ -263,10 +265,11 @@ public class FabricUIManager
   @Override
   @AnyThread
   @ThreadConfined(ANY)
+  // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation]
   public <T extends View> int startSurface(
       final T rootView,
       final String moduleName,
-      final @Nullable WritableMap initialProps,
+      final WritableMap initialProps,
       int widthMeasureSpec,
       int heightMeasureSpec) {
     final int rootTag = ((ReactRoot) rootView).getRootViewTag();
@@ -285,7 +288,7 @@ public class FabricUIManager
     Point viewportOffset =
         UiThreadUtil.isOnUiThread() ? RootViewUtil.getViewportOffset(rootView) : new Point(0, 0);
 
-    Assertions.assertNotNull(mBinding, "Binding in FabricUIManager is null");
+    // NULLSAFE_FIXME[Nullable Dereference]
     mBinding.startSurfaceWithConstraints(
         rootTag,
         moduleName,
@@ -311,7 +314,7 @@ public class FabricUIManager
         new ThemedReactContext(
             mReactApplicationContext, context, surfaceHandler.getModuleName(), rootTag);
     mMountingManager.startSurface(rootTag, reactContext, rootView);
-    Assertions.assertNotNull(mBinding, "Binding in FabricUIManager is null");
+    // NULLSAFE_FIXME[Nullable Dereference]
     mBinding.startSurfaceWithSurfaceHandler(rootTag, surfaceHandler, rootView != null);
   }
 
@@ -336,7 +339,7 @@ public class FabricUIManager
     }
 
     mMountingManager.stopSurface(surfaceHandler.getSurfaceId());
-    Assertions.assertNotNull(mBinding, "Binding in FabricUIManager is null");
+    // NULLSAFE_FIXME[Nullable Dereference]
     mBinding.stopSurfaceWithSurfaceHandler(surfaceHandler);
   }
 
@@ -356,7 +359,7 @@ public class FabricUIManager
     // 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
-    Assertions.assertNotNull(mBinding, "Binding in FabricUIManager is null");
+    // NULLSAFE_FIXME[Nullable Dereference]
     mBinding.stopSurface(surfaceID);
   }
 
@@ -407,9 +410,8 @@ public class FabricUIManager
     mReactApplicationContext.removeLifecycleEventListener(this);
     onHostPause();
 
-    if (mBinding != null) {
-      mBinding.unregister();
-    }
+    // NULLSAFE_FIXME[Nullable Dereference]
+    mBinding.unregister();
     mBinding = null;
 
     ViewManagerPropertyUpdater.clear();
@@ -453,6 +455,7 @@ public class FabricUIManager
     }
   }
 
+  @NonNull
   private InteropUIBlockListener getInteropUIBlockListener() {
     if (mInteropUIBlockListener == null) {
       mInteropUIBlockListener = new InteropUIBlockListener();
@@ -539,13 +542,12 @@ public class FabricUIManager
         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,
@@ -579,14 +581,13 @@ public class FabricUIManager
         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,
@@ -623,11 +624,13 @@ public class FabricUIManager
   }
 
   @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);
   }
@@ -642,7 +645,9 @@ public class FabricUIManager
   @Override
   @UiThread
   @ThreadConfined(UI)
-  public void synchronouslyUpdateViewOnUIThread(final int reactTag, final ReadableMap props) {
+  // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation]
+  public void synchronouslyUpdateViewOnUIThread(
+      final int reactTag, @NonNull final ReadableMap props) {
     UiThreadUtil.assertOnUiThread();
 
     int commitNumber = mCurrentSynchronousCommitNumber++;
@@ -673,7 +678,7 @@ public class FabricUIManager
     MountItem synchronousMountItem =
         new MountItem() {
           @Override
-          public void execute(MountingManager mountingManager) {
+          public void execute(@NonNull MountingManager mountingManager) {
             try {
               mountingManager.updateProps(reactTag, props);
             } catch (Exception ex) {
@@ -691,6 +696,7 @@ public class FabricUIManager
             return View.NO_ID;
           }
 
+          @NonNull
           @Override
           public String toString() {
             String propsString =
@@ -795,14 +801,11 @@ public class FabricUIManager
     // calls scheduleMountItems with a BatchMountItem.
     long scheduleMountItemStartTime = SystemClock.uptimeMillis();
     boolean isBatchMountItem = mountItem instanceof BatchMountItem;
-    boolean shouldSchedule = false;
-    if (isBatchMountItem) {
-      BatchMountItem batchMountItem = (BatchMountItem) mountItem;
-      Assertions.assertNotNull(batchMountItem, "BatchMountItem is null");
-      shouldSchedule = !batchMountItem.isBatchEmpty();
-    } else {
-      shouldSchedule = mountItem != null;
-    }
+    boolean shouldSchedule =
+        // NULLSAFE_FIXME[Nullable Dereference]
+        (isBatchMountItem && !((BatchMountItem) mountItem).isBatchEmpty())
+            || (!isBatchMountItem && 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) {
@@ -818,7 +821,7 @@ public class FabricUIManager
     }
 
     if (shouldSchedule) {
-      Assertions.assertNotNull(mountItem, "MountItem is null");
+      // NULLSAFE_FIXME[Parameter Not Nullable]
       mMountItemDispatcher.addMountItem(mountItem);
       Runnable runnable =
           new GuardedRunnable(mReactApplicationContext) {
@@ -915,7 +918,7 @@ public class FabricUIManager
       doLeftAndRightSwapInRTL = I18nUtil.getInstance().doLeftAndRightSwapInRTL(context);
     }
 
-    Assertions.assertNotNull(mBinding, "Binding in FabricUIManager is null");
+    // NULLSAFE_FIXME[Nullable Dereference]
     mBinding.setConstraints(
         surfaceId,
         getMinSize(widthMeasureSpec),
@@ -929,10 +932,11 @@ public class FabricUIManager
   }
 
   @Override
-  public @Nullable View resolveView(int reactTag) {
+  public View resolveView(int reactTag) {
     UiThreadUtil.assertOnUiThread();
 
     SurfaceMountingManager surfaceManager = mMountingManager.getSurfaceManagerForView(reactTag);
+    // NULLSAFE_FIXME[Return Not Nullable]
     return surfaceManager == null ? null : surfaceManager.getView(reactTag);
   }
 
@@ -975,7 +979,7 @@ public class FabricUIManager
   public void receiveEvent(
       int surfaceId,
       int reactTag,
-      String eventName,
+      @NonNull String eventName,
       boolean canCoalesceEvent,
       @Nullable WritableMap params,
       @EventCategoryDef int eventCategory,
@@ -1028,6 +1032,7 @@ public class FabricUIManager
   }
 
   @Override
+  @NonNull
   public EventDispatcher getEventDispatcher() {
     return mEventDispatcher;
   }
@@ -1140,7 +1145,7 @@ public class FabricUIManager
     mMountItemDispatcher.addMountItem(
         new MountItem() {
           @Override
-          public void execute(MountingManager mountingManager) {
+          public void execute(@NonNull MountingManager mountingManager) {
             SurfaceMountingManager surfaceMountingManager =
                 mountingManager.getSurfaceManager(surfaceId);
             if (surfaceMountingManager != null) {
@@ -1157,6 +1162,7 @@ public class FabricUIManager
             return surfaceId;
           }
 
+          @NonNull
           @SuppressLint("DefaultLocale")
           @Override
           public String toString() {
@@ -1173,7 +1179,7 @@ public class FabricUIManager
     mMountItemDispatcher.addMountItem(
         new MountItem() {
           @Override
-          public void execute(MountingManager mountingManager) {
+          public void execute(@NonNull MountingManager mountingManager) {
             mountingManager.clearJSResponder();
           }
 
@@ -1182,6 +1188,7 @@ public class FabricUIManager
             return View.NO_ID;
           }
 
+          @NonNull
           @Override
           public String toString() {
             return "CLEAR_JS_RESPONDER";
@@ -1330,7 +1337,7 @@ public class FabricUIManager
     @ThreadConfined(UI)
     private boolean mIsScheduled = false;
 
-    private DispatchUIFrameCallback(ReactContext reactContext) {
+    private DispatchUIFrameCallback(@NonNull ReactContext reactContext) {
       super(reactContext);
     }
 

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

@@ -33,12 +33,12 @@ internal class FabricUIManagerBinding : HybridClassBase() {
       componentsRegistry: ComponentFactory,
   )
 
-  external fun startSurface(surfaceId: Int, moduleName: String, initialProps: NativeMap?)
+  external fun startSurface(surfaceId: Int, moduleName: String, initialProps: NativeMap)
 
   external fun startSurfaceWithConstraints(
       surfaceId: Int,
       moduleName: String,
-      initialProps: NativeMap?,
+      initialProps: NativeMap,
       minWidth: Float,
       maxWidth: Float,
       minHeight: Float,

packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.cpp

@@ -172,9 +172,7 @@ void FabricUIManagerBinding::startSurface(
 
   auto surfaceHandler = SurfaceHandler{moduleName->toStdString(), surfaceId};
   surfaceHandler.setContextContainer(scheduler->getContextContainer());
-  if (initialProps != nullptr) {
-    surfaceHandler.setProps(initialProps->consume());
-  }
+  surfaceHandler.setProps(initialProps->consume());
   surfaceHandler.constraintLayout({}, layoutContext);
 
   scheduler->registerSurface(surfaceHandler);
@@ -243,9 +241,7 @@ void FabricUIManagerBinding::startSurfaceWithConstraints(
 
   auto surfaceHandler = SurfaceHandler{moduleName->toStdString(), surfaceId};
   surfaceHandler.setContextContainer(scheduler->getContextContainer());
-  if (initialProps != nullptr) {
-    surfaceHandler.setProps(initialProps->consume());
-  }
+  surfaceHandler.setProps(initialProps->consume());
   surfaceHandler.constraintLayout(constraints, context);
 
   scheduler->registerSurface(surfaceHandler);

packages/react-native/ReactAndroid/src/test/java/com/facebook/testutils/fakes/FakeUIManager.kt

@@ -30,7 +30,7 @@ class FakeUIManager : UIManager, UIBlockViewResolver {
   }
 
   @Deprecated("")
-  override fun <T : View> addRootView(rootView: T, initialProps: WritableMap?): Int {
+  override fun <T : View?> addRootView(rootView: T, initialProps: WritableMap?): Int {
     error("Not yet implemented")
   }
 
@@ -69,7 +69,7 @@ class FakeUIManager : UIManager, UIBlockViewResolver {
   override val eventDispatcher: EventDispatcher
     get() = TODO("Not yet implemented")
 
-  override fun synchronouslyUpdateViewOnUIThread(reactTag: Int, props: ReadableMap) {
+  override fun synchronouslyUpdateViewOnUIThread(reactTag: Int, props: ReadableMap?) {
     error("Not yet implemented")
   }
 
@@ -77,11 +77,11 @@ class FakeUIManager : UIManager, UIBlockViewResolver {
     error("Not yet implemented")
   }
 
-  override fun addUIManagerEventListener(listener: UIManagerListener) {
+  override fun addUIManagerEventListener(listener: UIManagerListener?) {
     error("Not yet implemented")
   }
 
-  override fun removeUIManagerEventListener(listener: UIManagerListener) {
+  override fun removeUIManagerEventListener(listener: UIManagerListener?) {
     error("Not yet implemented")
   }