For lifecycle correctness, call scheduleMountItems even with empty BatchMountItem

Summary:
In particular, NativeAnimatedModule relies on having some signal of when there's a commit from ReactJS. In Fabric, this is the only reliable signal. Failing to call scheduleMountItems even when there's no changes to the tree will result in certain animation operations being delayed way longer than necessary.

I pass nullptr instead of empty arrays in some cases to hopefully improve perf.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D22008981

fbshipit-source-id: 6bdeb46e03b5138dbfa5b077952ec0fa3dfe1eb3

ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java

@@ -542,7 +542,8 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
 
   /**
    * This method enqueues UI operations directly to the UI thread. This might change in the future
-   * to enforce execution order using {@link ReactChoreographer#CallbackType}.
+   * to enforce execution order using {@link ReactChoreographer#CallbackType}. This method should
+   * only be called as the result of a new tree being committed.
    */
   @DoNotStrip
   @SuppressWarnings("unused")
@@ -562,6 +563,13 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
     // a BatchMountItem. No other sites call into this with a BatchMountItem, and Binding.cpp only
     // calls scheduleMountItems with a BatchMountItem.
     boolean isBatchMountItem = mountItem instanceof BatchMountItem;
+    boolean shouldSchedule = !(isBatchMountItem && ((BatchMountItem) mountItem).getSize() == 0);
+
+    // 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) {
+      listener.didScheduleMountItems(this);
+    }
 
     if (isBatchMountItem) {
       mCommitStartTime = commitStartTime;
@@ -571,13 +579,15 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
       mDispatchViewUpdatesTime = SystemClock.uptimeMillis();
     }
 
-    synchronized (mMountItemsLock) {
-      mMountItems.add(mountItem);
-    }
+    if (shouldSchedule) {
+      synchronized (mMountItemsLock) {
+        mMountItems.add(mountItem);
+      }
 
-    if (UiThreadUtil.isOnUiThread()) {
-      // We only read these flags on the UI thread.
-      tryDispatchMountItems();
+      if (UiThreadUtil.isOnUiThread()) {
+        // We only read these flags on the UI thread.
+        tryDispatchMountItems();
+      }
     }
 
     // Post markers outside of lock and after sync mounting finishes its execution

ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp

@@ -842,22 +842,18 @@ void Binding::schedulerDidFinishTransaction(
     toRemove.clear();
   }
 
-  if (position <= 0) {
-    // If there are no mountItems to be sent to the platform, then it is not
-    // necessary to even call.
-    return;
-  }
-
   static auto createMountItemsBatchContainer =
       jni::findClassStatic(UIManagerJavaDescriptor)
           ->getMethod<alias_ref<JMountItem>(
               jint, jtypeArray<JMountItem::javaobject>, jint, jint)>(
               "createBatchMountItem");
 
+  // If there are no items, we pass a nullptr instead of passing the object
+  // through the JNI
   auto batch = createMountItemsBatchContainer(
       localJavaUIManager,
       surfaceId,
-      mountItemsArray.get(),
+      position == 0 ? nullptr : mountItemsArray.get(),
       position,
       commitNumber);
 

ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/BatchMountItem.java

@@ -34,12 +34,10 @@ public class BatchMountItem implements MountItem {
   private final int mCommitNumber;
 
   public BatchMountItem(int rootTag, MountItem[] items, int size, int commitNumber) {
-    if (items == null) {
-      throw new NullPointerException();
-    }
-    if (size < 0 || size > items.length) {
+    int itemsLength = (items == null ? 0 : items.length);
+    if (size < 0 || size > itemsLength) {
       throw new IllegalArgumentException(
-          "Invalid size received by parameter size: " + size + " items.size = " + items.length);
+          "Invalid size received by parameter size: " + size + " items.size = " + itemsLength);
     }
     mRootTag = rootTag;
     mMountItems = items;
@@ -74,6 +72,10 @@ public class BatchMountItem implements MountItem {
     return mRootTag;
   }
 
+  public int getSize() {
+    return mSize;
+  }
+
   @Override
   public String toString() {
     StringBuilder s = new StringBuilder();