Proposed fix for ScrollView race condition between C++ state update and onScroll

Summary:
FlatList relies heavily on onScroll events + the measure API. In Fabric, usage of `measure` relies on C++ having an accurate view of the current scroll position of the ScrollView.

We already have a mechanism for updating the scroll position in C++ using UpdateState. But, it is only used currently at the /beginning/ and /end/ of scrolling, and UpdateState is not called /during/ scrolling at all.

This means that we will see a series of events like this while scrolling:

```
Scrolling begins
UPDATE C++ STATE: scrollLeft = 0, scrollTop = 0
JS event: onScroll x=0, y=0
JS event: onScroll x=0, y=100
JS event: onScroll x=0, y=200
...
JS event: onScroll x=0, y=1000
UPDATE C++ STATE: scrollLeft = 0, scrollTop = 1000
```

Notably, not many C++ state updates are queued; and the last one is queued AFTER the JS event is sent. The last JS event and UpdateState will race, which means that sometimes the C++ update will /lose/ and C++ will have an inaccurate view of the world when FlatList receives the onScroll event and calls `measure`.

My proposed solution, gated behind a feature flag, is to delay /some/ onScroll events until the C++ UpdateState has made its way back to Java, and send UpdateStates more frequently. The balance here is that UpdateState is a relatively expensive operation, so we probably still want to call it /less/ than we call onScroll. This means that `measure` will still return some incorrect results but will return correct results more frequently. Win?

Changelog: [Internal[

Reviewed By: mdvacca

Differential Revision: D28558380

fbshipit-source-id: 11c7cd714fae67ee5a94c4413be988481413ec03

ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java

@@ -19,6 +19,11 @@ import com.facebook.proguard.annotations.DoNotStripAny;
 @DoNotStripAny
 public class ReactFeatureFlags {
 
+  /** An interface used to compute flags on demand. */
+  public interface FlagProvider {
+    boolean get();
+  }
+
   /**
    * Should this application use TurboModules? If yes, then any module that inherits {@link
    * com.facebook.react.turbomodule.core.interfaces.TurboModule} will NOT be passed in to C++
@@ -63,11 +68,6 @@ public class ReactFeatureFlags {
   /** Feature flag to configure eager initialization of MapBuffer So file */
   public static boolean enableEagerInitializeMapBufferSoFile = false;
 
-  /** An interface used to compute flags on demand. */
-  public interface FlagProvider {
-    boolean get();
-  }
-
   /** Should the RuntimeExecutor call JSIExecutor::flush()? */
   private static FlagProvider enableRuntimeExecutorFlushingProvider = null;
 
@@ -96,4 +96,17 @@ public class ReactFeatureFlags {
 
   /** Enables Fabric for LogBox */
   public static boolean enableFabricInLogBox = false;
+
+  //
+  // ScrollView C++ UpdateState vs onScroll race fixes
+  //
+
+  /* Enables a "state race condition fix" for ScrollViews StateUpdate + onScroll event emitter */
+  public static boolean enableScrollViewStateEventRaceFix = false;
+
+  /* Enables another "state race condition fix" for ScrollViews StateUpdate + onScroll event emitter. Races a StateUpdate with every onScroll event. */
+  public static boolean enableScrollViewStateEventAlwaysRace = false;
+
+  /* Configure a min scroll delta for UpdateState to be called while still actively scrolling. */
+  public static int scrollViewUpdateStateMinScrollDelta = 0;
 }

ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java

@@ -30,12 +30,15 @@ import com.facebook.react.bridge.ReactContext;
 import com.facebook.react.bridge.WritableMap;
 import com.facebook.react.bridge.WritableNativeMap;
 import com.facebook.react.common.ReactConstants;
+import com.facebook.react.config.ReactFeatureFlags;
 import com.facebook.react.uimanager.FabricViewStateManager;
 import com.facebook.react.uimanager.MeasureSpecAssertions;
 import com.facebook.react.uimanager.PixelUtil;
 import com.facebook.react.uimanager.ReactClippingViewGroup;
 import com.facebook.react.uimanager.ReactClippingViewGroupHelper;
 import com.facebook.react.uimanager.ViewProps;
+import com.facebook.react.uimanager.common.UIManagerType;
+import com.facebook.react.uimanager.common.ViewUtil;
 import com.facebook.react.uimanager.events.NativeGestureUtil;
 import com.facebook.react.views.view.ReactViewBackgroundManager;
 import java.lang.reflect.Field;
@@ -98,6 +101,7 @@ public class ReactScrollView extends ScrollView
 
   private int mScrollAwayPaddingTop = 0;
 
+  private boolean mWaitingForStateUpdateRoundTrip = false;
   private int mLastStateUpdateScrollX = -1;
   private int mLastStateUpdateScrollY = -1;
 
@@ -284,10 +288,22 @@ public class ReactScrollView extends ScrollView
         updateClippingRect();
       }
 
-      ReactScrollViewHelper.emitScrollEvent(
-          this,
-          mOnScrollDispatchHelper.getXFlingVelocity(),
-          mOnScrollDispatchHelper.getYFlingVelocity());
+      // Another potential UpdateState vs onScroll fix: race an UpdateState with every onScroll
+      // TODO T91209139: if this mechanism works well, port it to HorizontalScrollView
+      if (ReactFeatureFlags.enableScrollViewStateEventAlwaysRace) {
+        updateStateOnScroll();
+      }
+
+      // TODO T91209139: if this mechanism works well, port it to HorizontalScrollView
+      boolean deferEvent =
+          ReactFeatureFlags.enableScrollViewStateEventRaceFix
+              && (mWaitingForStateUpdateRoundTrip || updateStateOnScroll());
+      if (!deferEvent) {
+        ReactScrollViewHelper.emitScrollEvent(
+            this,
+            mOnScrollDispatchHelper.getXFlingVelocity(),
+            mOnScrollDispatchHelper.getYFlingVelocity());
+      }
     }
   }
 
@@ -997,23 +1013,57 @@ public class ReactScrollView extends ScrollView
     setRemoveClippedSubviews(mRemoveClippedSubviews);
   }
 
+  /**
+   * If we know we are sending a State update, we defer emitting scroll events until the State
+   * update makes it back to Java. When that happens, we should immediately emit the scroll event.
+   */
+  void onStateUpdate() {
+    mWaitingForStateUpdateRoundTrip = false;
+
+    // For now we don't dedupe these - we want to send an event whenever the metrics have changed
+    // from the perspective of C++
+    if (ReactFeatureFlags.enableScrollViewStateEventRaceFix) {
+      ReactScrollViewHelper.emitScrollEvent(
+          this,
+          mOnScrollDispatchHelper.getXFlingVelocity(),
+          mOnScrollDispatchHelper.getYFlingVelocity());
+    }
+  }
+
   /**
    * Called on any stabilized onScroll change to propagate content offset value to a Shadow Node.
    */
-  private void updateStateOnScroll(final int scrollX, final int scrollY) {
+  private boolean updateStateOnScroll(final int scrollX, final int scrollY) {
+    if (ViewUtil.getUIManagerType(getId()) == UIManagerType.DEFAULT) {
+      return false;
+    }
+
     // Dedupe events to reduce JNI traffic
     if (scrollX == mLastStateUpdateScrollX && scrollY == mLastStateUpdateScrollY) {
-      return;
+      return false;
+    }
+
+    // Require a certain delta if we're still scrolling
+    if (mActivelyScrolling) {
+      int MIN_DELTA_TO_UPDATE_SCROLL_STATE = ReactFeatureFlags.scrollViewUpdateStateMinScrollDelta;
+      int deltaX = Math.abs(mLastStateUpdateScrollX - scrollX);
+      int deltaY = Math.abs(mLastStateUpdateScrollY - scrollY);
+      if (deltaX < MIN_DELTA_TO_UPDATE_SCROLL_STATE && deltaY < MIN_DELTA_TO_UPDATE_SCROLL_STATE) {
+        return false;
+      }
     }
 
     mLastStateUpdateScrollX = scrollX;
     mLastStateUpdateScrollY = scrollY;
 
+    mWaitingForStateUpdateRoundTrip = true;
     forceUpdateState();
+
+    return true;
   }
 
-  private void updateStateOnScroll() {
-    updateStateOnScroll(getScrollX(), getScrollY());
+  private boolean updateStateOnScroll() {
+    return updateStateOnScroll(getScrollX(), getScrollY());
   }
 
   private void updateScrollAwayState(int scrollAwayPaddingTop) {

ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollViewManager.java

@@ -320,6 +320,7 @@ public class ReactScrollViewManager extends ViewGroupManager<ReactScrollView>
   public Object updateState(
       ReactScrollView view, ReactStylesDiffMap props, @Nullable StateWrapper stateWrapper) {
     view.getFabricViewStateManager().setStateWrapper(stateWrapper);
+    view.onStateUpdate();
     return null;
   }