NativeAnimatedModule: allow JS to control queueing of Animated operations

Summary:
In the past I tried a few heuristics to guess when a batch of Animated Operations were ready, and none of these were super reliable. But it turns out we can safely allow JS to manage that explicitly.

Non-Fabric still uses the old behavior which seems fine.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D23010844

fbshipit-source-id: 4c688d3a61460118557a4971e549ec7457f3eb8f

ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java

@@ -89,14 +89,14 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec
   private abstract class UIThreadOperation {
     abstract void execute(NativeAnimatedNodesManager animatedNodesManager);
 
-    long mFrameNumber = -1;
+    long mBatchNumber = -1;
 
-    public void setFrameNumber(long frameNumber) {
-      mFrameNumber = frameNumber;
+    public void setBatchNumber(long batchNumber) {
+      mBatchNumber = batchNumber;
     }
 
-    public long getFrameNumber() {
-      return mFrameNumber;
+    public long getBatchNumber() {
+      return mBatchNumber;
     }
   }
 
@@ -113,8 +113,10 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec
 
   private @Nullable NativeAnimatedNodesManager mNodesManager;
 
-  private volatile long mFrameNumber = 0;
-  private long mDispatchedFrameNumber = 0;
+  private boolean mBatchingControlledByJS = false; // TODO T71377544: delete
+  private volatile long mCurrentFrameNumber; // TODO T71377544: delete
+  private volatile long mCurrentBatchNumber;
+
   private boolean mInitializedForFabric = false;
   private boolean mInitializedForNonFabric = false;
   private @UIManagerType int mUIManagerType = UIManagerType.DEFAULT;
@@ -169,23 +171,24 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec
   }
 
   private void addOperation(UIThreadOperation operation) {
-    operation.setFrameNumber(mFrameNumber);
+    operation.setBatchNumber(mCurrentBatchNumber);
+    mOperations.add(operation);
+  }
+
+  private void addUnbatchedOperation(UIThreadOperation operation) {
+    operation.setBatchNumber(-1);
     mOperations.add(operation);
   }
 
   private void addPreOperation(UIThreadOperation operation) {
-    operation.setFrameNumber(mFrameNumber);
+    operation.setBatchNumber(mCurrentBatchNumber);
     mPreOperations.add(operation);
   }
 
   // For FabricUIManager only
   @Override
   public void didScheduleMountItems(UIManager uiManager) {
-    if (mUIManagerType != UIManagerType.FABRIC) {
-      return;
-    }
-
-    mFrameNumber++;
+    mCurrentFrameNumber++;
   }
 
   // For FabricUIManager only
@@ -196,37 +199,31 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec
       return;
     }
 
-    // The problem we're trying to solve here: we could be in the middle of queueing
-    // a batch of related animation operations when Fabric flushes a batch of MountItems.
-    // It's visually bad if we execute half of the animation ops and then wait another frame
-    // (or more) to execute the rest.
-    // See mFrameNumber. If the dispatchedFrameNumber drifts too far - that
-    // is, if no MountItems are scheduled for a while, which can happen if a tree
-    // is committed but there are no changes - bring these counts back in sync and
-    // execute any queued operations. This number is arbitrary, but we want it low
-    // enough that the user shouldn't be able to see this delay in most cases.
-    mDispatchedFrameNumber++;
-    long currentFrameNo = mFrameNumber - 1;
-    if ((mDispatchedFrameNumber - mFrameNumber) > 2) {
-      mFrameNumber = mDispatchedFrameNumber;
-      currentFrameNo = mFrameNumber;
+    long batchNumber = mCurrentBatchNumber - 1;
+
+    // TODO T71377544: delete this when the JS method is confirmed safe
+    if (!mBatchingControlledByJS) {
+      // The problem we're trying to solve here: we could be in the middle of queueing
+      // a batch of related animation operations when Fabric flushes a batch of MountItems.
+      // It's visually bad if we execute half of the animation ops and then wait another frame
+      // (or more) to execute the rest.
+      // See mFrameNumber. If the dispatchedFrameNumber drifts too far - that
+      // is, if no MountItems are scheduled for a while, which can happen if a tree
+      // is committed but there are no changes - bring these counts back in sync and
+      // execute any queued operations. This number is arbitrary, but we want it low
+      // enough that the user shouldn't be able to see this delay in most cases.
+      mCurrentFrameNumber++;
+      if ((mCurrentFrameNumber - mCurrentBatchNumber) > 2) {
+        mCurrentBatchNumber = mCurrentFrameNumber;
+        batchNumber = mCurrentBatchNumber;
+      }
     }
 
-    // This will execute all operations and preOperations queued
-    // since the last time this was run, and will race with anything
-    // being queued from the JS thread. That is, if the JS thread
-    // is still queuing operations, we might execute some of them
-    // at the very end until we exhaust the queue faster than the
-    // JS thread can queue up new items.
-    // The reason we increment in scheduleMountItems and subtract 1 here
-    // is that `scheduleMountItems` happens as close to the JS commit as
-    // possible, whereas execution of those same items might happen sometime
-    // later on the UI thread while the JS thread keeps plugging along.
-    executeAllOperations(mPreOperations, currentFrameNo);
-    executeAllOperations(mOperations, currentFrameNo);
+    executeAllOperations(mPreOperations, batchNumber);
+    executeAllOperations(mOperations, batchNumber);
   }
 
-  private void executeAllOperations(Queue<UIThreadOperation> operationQueue, long maxFrameNumber) {
+  private void executeAllOperations(Queue<UIThreadOperation> operationQueue, long maxBatchNumber) {
     NativeAnimatedNodesManager nodesManager = getNodesManager();
     while (true) {
       // There is a race condition where `peek` may return a non-null value and isEmpty() is false,
@@ -241,7 +238,7 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec
         return;
       }
       // The rest of the operations are for the next frame.
-      if (peekedOperation.getFrameNumber() > maxFrameNumber) {
+      if (peekedOperation.getBatchNumber() > maxBatchNumber) {
         return;
       }
 
@@ -270,7 +267,7 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec
       return;
     }
 
-    final long frameNo = mFrameNumber++;
+    final long frameNo = mCurrentBatchNumber++;
 
     UIBlock preOperationsUIBlock =
         new UIBlock() {
@@ -417,6 +414,18 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec
     }
   }
 
+  @Override
+  public void startOperationBatch() {
+    mBatchingControlledByJS = true;
+    mCurrentBatchNumber++;
+  }
+
+  @Override
+  public void finishOperationBatch() {
+    mBatchingControlledByJS = true;
+    mCurrentBatchNumber++;
+  }
+
   @Override
   public void createAnimatedNode(final double tagDouble, final ReadableMap config) {
     final int tag = (int) tagDouble;
@@ -604,7 +613,7 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec
       FLog.d(NAME, "queue startAnimatingNode: ID: " + animationId + " tag: " + animatedNodeTag);
     }
 
-    addOperation(
+    addUnbatchedOperation(
         new UIThreadOperation() {
           @Override
           public void execute(NativeAnimatedNodesManager animatedNodesManager) {