Execute commands in order (#54074)

Summary:
Pull Request resolved: #54074

Currently, we dispatch view commands eagerly at the beginning of a batch. This behavior originated with b54257c.

However, this can make it difficult to allow developers to control mount item execution order. For example, someone may want to make sure a view command is processed *after* a particular mount item, and not exclusively rely on retryable command exceptions.

This change sets up a feature flag to enable in-order execution of view command mount items.

## Changelog

[Internal]

Reviewed By: mdvacca, sammy-SC

Differential Revision: D84061054

fbshipit-source-id: 7cb68c43823ed67f541f4e8f2d7471629609a905

Author: rozele

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

@@ -43,7 +43,11 @@ internal class MountItemDispatcher(
   private var lastFrameTimeNanos: Long = 0L
 
   fun addViewCommandMountItem(mountItem: DispatchCommandMountItem) {
-    viewCommandMountItems.add(mountItem)
+    if (!ReactNativeFeatureFlags.disableEarlyViewCommandExecution()) {
+      viewCommandMountItems.add(mountItem)
+    } else {
+      mountItems.add(mountItem)
+    }
   }
 
   fun addMountItem(mountItem: MountItem) {
@@ -147,13 +151,47 @@ internal class MountItemDispatcher(
 
     itemDispatchListener.willMountItems(mountItemsToDispatch)
 
+    val dispatchViewCommand: (command: DispatchCommandMountItem) -> Unit = { command ->
+      if (ReactNativeFeatureFlags.enableFabricLogs()) {
+        printMountItem(command, "dispatchMountItems: Executing viewCommandMountItem")
+      }
+      try {
+        executeOrEnqueue(command)
+      } catch (e: RetryableMountingLayerException) {
+        // If the exception is marked as Retryable, we retry the viewcommand exactly once, after
+        // the current batch of mount items has finished executing.
+        if (command.getRetries() == 0) {
+          command.incrementRetries()
+          addViewCommandMountItem(command)
+        } else {
+          // It's very common for commands to be executed on views that no longer exist - for
+          // example, a blur event on TextInput being fired because of a navigation event away
+          // from the current screen. If the exception is marked as Retryable, we log a soft
+          // exception but never crash in debug.
+          // It's not clear that logging this is even useful, because these events are very
+          // common, mundane, and there's not much we can do about them currently.
+          ReactSoftExceptionLogger.logSoftException(
+              TAG,
+              ReactNoCrashSoftException("Caught exception executing ViewCommand: $command", e),
+          )
+        }
+      } catch (e: Throwable) {
+        // Non-retryable exceptions are logged as soft exceptions in prod, but crash in Debug.
+        ReactSoftExceptionLogger.logSoftException(
+            TAG,
+            RuntimeException("Caught exception executing ViewCommand: $command", e),
+        )
+      }
+    }
+
     // As an optimization, execute all ViewCommands first
     // This should be:
     // 1) Performant: ViewCommands are often a replacement for SetNativeProps, which we've always
     // wanted to be as "synchronous" as possible.
     // 2) Safer: ViewCommands are inherently disconnected from the tree commit/diff/mount process.
     // JS imperatively queues these commands.
-    //    If JS has queued a command, it's reasonable to assume that the more time passes, the more
+    //    If JS has queued a command, it's reasonable to assume that the more time passes, the
+    // more
     // likely it is that the view disappears.
     //    Thus, by executing ViewCommands early, we should actually avoid a category of
     // errors/glitches.
@@ -164,36 +202,7 @@ internal class MountItemDispatcher(
       )
 
       for (command in commands) {
-        if (ReactNativeFeatureFlags.enableFabricLogs()) {
-          printMountItem(command, "dispatchMountItems: Executing viewCommandMountItem")
-        }
-        try {
-          executeOrEnqueue(command)
-        } catch (e: RetryableMountingLayerException) {
-          // If the exception is marked as Retryable, we retry the viewcommand exactly once, after
-          // the current batch of mount items has finished executing.
-          if (command.getRetries() == 0) {
-            command.incrementRetries()
-            addViewCommandMountItem(command)
-          } else {
-            // It's very common for commands to be executed on views that no longer exist - for
-            // example, a blur event on TextInput being fired because of a navigation event away
-            // from the current screen. If the exception is marked as Retryable, we log a soft
-            // exception but never crash in debug.
-            // It's not clear that logging this is even useful, because these events are very
-            // common, mundane, and there's not much we can do about them currently.
-            ReactSoftExceptionLogger.logSoftException(
-                TAG,
-                ReactNoCrashSoftException("Caught exception executing ViewCommand: $command", e),
-            )
-          }
-        } catch (e: Throwable) {
-          // Non-retryable exceptions are logged as soft exceptions in prod, but crash in Debug.
-          ReactSoftExceptionLogger.logSoftException(
-              TAG,
-              RuntimeException("Caught exception executing ViewCommand: $command", e),
-          )
-        }
+        dispatchViewCommand(command)
       }
 
       Systrace.endSection(Systrace.TRACE_TAG_REACT)
@@ -227,6 +236,12 @@ internal class MountItemDispatcher(
           printMountItem(mountItem, "dispatchMountItems: Executing mountItem")
         }
 
+        val command = mountItem as? DispatchCommandMountItem
+        if (command != null) {
+          dispatchViewCommand(command)
+          continue
+        }
+
         try {
           executeOrEnqueue(mountItem)
         } catch (e: Throwable) {

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlags.kt

@@ -4,7 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @generated SignedSource<<4f9777b3b50f714214f9f0f86d5176f1>>
+ * @generated SignedSource<<3fee7079eaa30dca86e3cb366cdd261e>>
  */
 
 /**
@@ -54,6 +54,12 @@ public object ReactNativeFeatureFlags {
   @JvmStatic
   public fun cxxNativeAnimatedRemoveJsSync(): Boolean = accessor.cxxNativeAnimatedRemoveJsSync()
 
+  /**
+   * Dispatch view commands in mount item order.
+   */
+  @JvmStatic
+  public fun disableEarlyViewCommandExecution(): Boolean = accessor.disableEarlyViewCommandExecution()
+
   /**
    * Prevents use of Fabric commit in C++ Animated implementation
    */

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxAccessor.kt

@@ -4,7 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @generated SignedSource<<7e45b713bab183402e93ee058ea7cb36>>
+ * @generated SignedSource<<7ea729886da71d6d117442c83736fc5f>>
  */
 
 /**
@@ -24,6 +24,7 @@ internal class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAcces
   private var cdpInteractionMetricsEnabledCache: Boolean? = null
   private var cxxNativeAnimatedEnabledCache: Boolean? = null
   private var cxxNativeAnimatedRemoveJsSyncCache: Boolean? = null
+  private var disableEarlyViewCommandExecutionCache: Boolean? = null
   private var disableFabricCommitInCXXAnimatedCache: Boolean? = null
   private var disableMountItemReorderingAndroidCache: Boolean? = null
   private var disableOldAndroidAttachmentMetricsWorkaroundsCache: Boolean? = null
@@ -135,6 +136,15 @@ internal class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAcces
     return cached
   }
 
+  override fun disableEarlyViewCommandExecution(): Boolean {
+    var cached = disableEarlyViewCommandExecutionCache
+    if (cached == null) {
+      cached = ReactNativeFeatureFlagsCxxInterop.disableEarlyViewCommandExecution()
+      disableEarlyViewCommandExecutionCache = cached
+    }
+    return cached
+  }
+
   override fun disableFabricCommitInCXXAnimated(): Boolean {
     var cached = disableFabricCommitInCXXAnimatedCache
     if (cached == null) {

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxInterop.kt

@@ -4,7 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @generated SignedSource<<a8097e06e4ad0e329bd81fd6d23c031a>>
+ * @generated SignedSource<<e72b92811e1f318291484d55f24f15b2>>
  */
 
 /**
@@ -36,6 +36,8 @@ public object ReactNativeFeatureFlagsCxxInterop {
 
   @DoNotStrip @JvmStatic public external fun cxxNativeAnimatedRemoveJsSync(): Boolean
 
+  @DoNotStrip @JvmStatic public external fun disableEarlyViewCommandExecution(): Boolean
+
   @DoNotStrip @JvmStatic public external fun disableFabricCommitInCXXAnimated(): Boolean
 
   @DoNotStrip @JvmStatic public external fun disableMountItemReorderingAndroid(): Boolean

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsDefaults.kt

@@ -4,7 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @generated SignedSource<<c2e602217d9d88c23e958c8b73f22e5c>>
+ * @generated SignedSource<<bd9b92174bcc46a6df01d0489b2aebc8>>
  */
 
 /**
@@ -31,6 +31,8 @@ public open class ReactNativeFeatureFlagsDefaults : ReactNativeFeatureFlagsProvi
 
   override fun cxxNativeAnimatedRemoveJsSync(): Boolean = false
 
+  override fun disableEarlyViewCommandExecution(): Boolean = false
+
   override fun disableFabricCommitInCXXAnimated(): Boolean = false
 
   override fun disableMountItemReorderingAndroid(): Boolean = false

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsLocalAccessor.kt

@@ -4,7 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @generated SignedSource<<f88ede5a21467165ef04b5d16d869d1f>>
+ * @generated SignedSource<<226b5967122a840a4b6ac3fa41d3fb90>>
  */
 
 /**
@@ -28,6 +28,7 @@ internal class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcc
   private var cdpInteractionMetricsEnabledCache: Boolean? = null
   private var cxxNativeAnimatedEnabledCache: Boolean? = null
   private var cxxNativeAnimatedRemoveJsSyncCache: Boolean? = null
+  private var disableEarlyViewCommandExecutionCache: Boolean? = null
   private var disableFabricCommitInCXXAnimatedCache: Boolean? = null
   private var disableMountItemReorderingAndroidCache: Boolean? = null
   private var disableOldAndroidAttachmentMetricsWorkaroundsCache: Boolean? = null
@@ -143,6 +144,16 @@ internal class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcc
     return cached
   }
 
+  override fun disableEarlyViewCommandExecution(): Boolean {
+    var cached = disableEarlyViewCommandExecutionCache
+    if (cached == null) {
+      cached = currentProvider.disableEarlyViewCommandExecution()
+      accessedFeatureFlags.add("disableEarlyViewCommandExecution")
+      disableEarlyViewCommandExecutionCache = cached
+    }
+    return cached
+  }
+
   override fun disableFabricCommitInCXXAnimated(): Boolean {
     var cached = disableFabricCommitInCXXAnimatedCache
     if (cached == null) {

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsProvider.kt

@@ -4,7 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @generated SignedSource<<aa0e9e5830b13c4b290dcae42b757a3a>>
+ * @generated SignedSource<<bdb7047972cb5fe84dcc11ea6409e603>>
  */
 
 /**
@@ -31,6 +31,8 @@ public interface ReactNativeFeatureFlagsProvider {
 
   @DoNotStrip public fun cxxNativeAnimatedRemoveJsSync(): Boolean
 
+  @DoNotStrip public fun disableEarlyViewCommandExecution(): Boolean
+
   @DoNotStrip public fun disableFabricCommitInCXXAnimated(): Boolean
 
   @DoNotStrip public fun disableMountItemReorderingAndroid(): Boolean

packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.cpp

@@ -4,7 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @generated SignedSource<<5cf0077264f7ceca6617dbe20bcc122f>>
+ * @generated SignedSource<<bd78270ae3d4461c8f54a9bfb9fc8b20>>
  */
 
 /**
@@ -63,6 +63,12 @@ class ReactNativeFeatureFlagsJavaProvider
     return method(javaProvider_);
   }
 
+  bool disableEarlyViewCommandExecution() override {
+    static const auto method =
+        getReactNativeFeatureFlagsProviderJavaClass()->getMethod<jboolean()>("disableEarlyViewCommandExecution");
+    return method(javaProvider_);
+  }
+
   bool disableFabricCommitInCXXAnimated() override {
     static const auto method =
         getReactNativeFeatureFlagsProviderJavaClass()->getMethod<jboolean()>("disableFabricCommitInCXXAnimated");
@@ -531,6 +537,11 @@ bool JReactNativeFeatureFlagsCxxInterop::cxxNativeAnimatedRemoveJsSync(
   return ReactNativeFeatureFlags::cxxNativeAnimatedRemoveJsSync();
 }
 
+bool JReactNativeFeatureFlagsCxxInterop::disableEarlyViewCommandExecution(
+    facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop> /*unused*/) {
+  return ReactNativeFeatureFlags::disableEarlyViewCommandExecution();
+}
+
 bool JReactNativeFeatureFlagsCxxInterop::disableFabricCommitInCXXAnimated(
     facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop> /*unused*/) {
   return ReactNativeFeatureFlags::disableFabricCommitInCXXAnimated();
@@ -944,6 +955,9 @@ void JReactNativeFeatureFlagsCxxInterop::registerNatives() {
       makeNativeMethod(
         "cxxNativeAnimatedRemoveJsSync",
         JReactNativeFeatureFlagsCxxInterop::cxxNativeAnimatedRemoveJsSync),
+      makeNativeMethod(
+        "disableEarlyViewCommandExecution",
+        JReactNativeFeatureFlagsCxxInterop::disableEarlyViewCommandExecution),
       makeNativeMethod(
         "disableFabricCommitInCXXAnimated",
         JReactNativeFeatureFlagsCxxInterop::disableFabricCommitInCXXAnimated),

packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.h

@@ -4,7 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @generated SignedSource<<04929fb70beb00c1f690bdb11703b322>>
+ * @generated SignedSource<<d6f0bbc55218e36f040c04baa9eb6d8d>>
  */
 
 /**
@@ -42,6 +42,9 @@ class JReactNativeFeatureFlagsCxxInterop
   static bool cxxNativeAnimatedRemoveJsSync(
     facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);
 
+  static bool disableEarlyViewCommandExecution(
+    facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);
+
   static bool disableFabricCommitInCXXAnimated(
     facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);
 

packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.cpp

@@ -4,7 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @generated SignedSource<<bbe47fc20b10610f0ef14e2d35406145>>
+ * @generated SignedSource<<68811ae2e7fcb8b8b406097839f669dd>>
  */
 
 /**
@@ -42,6 +42,10 @@ bool ReactNativeFeatureFlags::cxxNativeAnimatedRemoveJsSync() {
   return getAccessor().cxxNativeAnimatedRemoveJsSync();
 }
 
+bool ReactNativeFeatureFlags::disableEarlyViewCommandExecution() {
+  return getAccessor().disableEarlyViewCommandExecution();
+}
+
 bool ReactNativeFeatureFlags::disableFabricCommitInCXXAnimated() {
   return getAccessor().disableFabricCommitInCXXAnimated();
 }