Split scheduler commit and flush delegate methods (facebook#44188)

Summary:
Pull Request resolved: facebook#44188

The current approach used for `batchRenderingUpdatesInEventLoop` is not compatible with Android due to limitations in its props processing model. The raw props changeset is passed through to Android, and must be available for the Android mounting layer to correctly apply changes.

We have some logic to merge these payloads when multiple ShadowNode clones take place but were previously assuming that a ShadowTree commit was a safe state to synchronize.

In the current implementation this means that two commits driven from layout effects (triggering states A → B → C) may cause Android to observe only the B → C props change, and miss out on any props changed in A → B.

Changelog: [Android][Fixed] Cascading renders were not mounting correctly when `batchRenderingUpdatesInEventLoop` is enabled.

Reviewed By: rubennorte

Differential Revision: D56414689

fbshipit-source-id: 7c74d81620db0f8b7bd67e640168afc795c7a1d7

packages/react-native/React/Fabric/RCTScheduler.h

@@ -28,6 +28,8 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (void)schedulerDidFinishTransaction:(facebook::react::MountingCoordinator::Shared)mountingCoordinator;
 
+- (void)schedulerShouldRenderTransactions:(facebook::react::MountingCoordinator::Shared)mountingCoordinator;
+
 - (void)schedulerDidDispatchCommand:(const facebook::react::ShadowView &)shadowView
                         commandName:(const std::string &)commandName
                                args:(const folly::dynamic &)args;

packages/react-native/React/Fabric/RCTScheduler.mm

@@ -31,6 +31,12 @@ void schedulerDidFinishTransaction(const MountingCoordinator::Shared &mountingCo
     [scheduler.delegate schedulerDidFinishTransaction:mountingCoordinator];
   }
 
+  void schedulerShouldRenderTransactions(const MountingCoordinator::Shared &mountingCoordinator) override
+  {
+    RCTScheduler *scheduler = (__bridge RCTScheduler *)scheduler_;
+    [scheduler.delegate schedulerShouldRenderTransactions:mountingCoordinator];
+  }
+
   void schedulerDidRequestPreliminaryViewAllocation(SurfaceId surfaceId, const ShadowNode &shadowNode) override
   {
     // Does nothing.

packages/react-native/React/Fabric/RCTSurfacePresenter.mm

@@ -333,6 +333,11 @@ - (void)_applicationWillTerminate
 #pragma mark - RCTSchedulerDelegate
 
 - (void)schedulerDidFinishTransaction:(MountingCoordinator::Shared)mountingCoordinator
+{
+  // no-op, we will flush the transaction from schedulerShouldRenderTransactions
+}
+
+- (void)schedulerShouldRenderTransactions:(MountingCoordinator::Shared)mountingCoordinator
 {
   [_mountingManager scheduleTransaction:mountingCoordinator];
 }

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<<cf91b15910f94812cc0d4628fea91f97>>
+ * @generated SignedSource<<f3d26e8e345068bc7ef4e156be460e09>>
  */
 
 /**
@@ -34,6 +34,12 @@ public object ReactNativeFeatureFlags {
   @JvmStatic
   public fun commonTestFlag(): Boolean = accessor.commonTestFlag()
 
+  /**
+   * To be used with batchRenderingUpdatesInEventLoop. When enbled, the Android mounting layer will concatenate pending transactions to ensure they're applied atomatically
+   */
+  @JvmStatic
+  public fun androidEnablePendingFabricTransactions(): Boolean = accessor.androidEnablePendingFabricTransactions()
+
   /**
    * When enabled, the RuntimeScheduler processing the event loop will batch all rendering updates and dispatch them together at the end of each iteration of the loop.
    */

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<<d23f1c4cd3f8960397455c495ed240ba>>
+ * @generated SignedSource<<40668dcd951123da7c0b4ddde23f94c9>>
  */
 
 /**
@@ -21,6 +21,7 @@ package com.facebook.react.internal.featureflags
 
 public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccessor {
   private var commonTestFlagCache: Boolean? = null
+  private var androidEnablePendingFabricTransactionsCache: Boolean? = null
   private var batchRenderingUpdatesInEventLoopCache: Boolean? = null
   private var destroyFabricSurfacesInReactInstanceManagerCache: Boolean? = null
   private var enableBackgroundExecutorCache: Boolean? = null
@@ -47,6 +48,15 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
     return cached
   }
 
+  override fun androidEnablePendingFabricTransactions(): Boolean {
+    var cached = androidEnablePendingFabricTransactionsCache
+    if (cached == null) {
+      cached = ReactNativeFeatureFlagsCxxInterop.androidEnablePendingFabricTransactions()
+      androidEnablePendingFabricTransactionsCache = cached
+    }
+    return cached
+  }
+
   override fun batchRenderingUpdatesInEventLoop(): Boolean {
     var cached = batchRenderingUpdatesInEventLoopCache
     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<<4470db3808c33009365864c597ceaf16>>
+ * @generated SignedSource<<c3f02dff409c10aa7922141cef3a6990>>
  */
 
 /**
@@ -30,6 +30,8 @@ public object ReactNativeFeatureFlagsCxxInterop {
 
   @DoNotStrip @JvmStatic public external fun commonTestFlag(): Boolean
 
+  @DoNotStrip @JvmStatic public external fun androidEnablePendingFabricTransactions(): Boolean
+
   @DoNotStrip @JvmStatic public external fun batchRenderingUpdatesInEventLoop(): Boolean
 
   @DoNotStrip @JvmStatic public external fun destroyFabricSurfacesInReactInstanceManager(): 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<<f9ed3dc67e050bc741efd3dc82f4b62c>>
+ * @generated SignedSource<<d12888990ebca7c6199f4b51802ee59b>>
  */
 
 /**
@@ -25,6 +25,8 @@ public open class ReactNativeFeatureFlagsDefaults : ReactNativeFeatureFlagsProvi
 
   override fun commonTestFlag(): Boolean = false
 
+  override fun androidEnablePendingFabricTransactions(): Boolean = false
+
   override fun batchRenderingUpdatesInEventLoop(): Boolean = false
 
   override fun destroyFabricSurfacesInReactInstanceManager(): 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<<17d6ac9c31290ac19caa64d87ce7215b>>
+ * @generated SignedSource<<c06b3b34cea24459f6ade0ec5665dae7>>
  */
 
 /**
@@ -25,6 +25,7 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
   private val accessedFeatureFlags = mutableSetOf<String>()
 
   private var commonTestFlagCache: Boolean? = null
+  private var androidEnablePendingFabricTransactionsCache: Boolean? = null
   private var batchRenderingUpdatesInEventLoopCache: Boolean? = null
   private var destroyFabricSurfacesInReactInstanceManagerCache: Boolean? = null
   private var enableBackgroundExecutorCache: Boolean? = null
@@ -52,6 +53,16 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
     return cached
   }
 
+  override fun androidEnablePendingFabricTransactions(): Boolean {
+    var cached = androidEnablePendingFabricTransactionsCache
+    if (cached == null) {
+      cached = currentProvider.androidEnablePendingFabricTransactions()
+      accessedFeatureFlags.add("androidEnablePendingFabricTransactions")
+      androidEnablePendingFabricTransactionsCache = cached
+    }
+    return cached
+  }
+
   override fun batchRenderingUpdatesInEventLoop(): Boolean {
     var cached = batchRenderingUpdatesInEventLoopCache
     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<<5ab1ec5a95d9fd6b66b30c38c7f8f199>>
+ * @generated SignedSource<<dceb20e62a9ddbb98c872a24fab9765c>>
  */
 
 /**
@@ -25,6 +25,8 @@ import com.facebook.proguard.annotations.DoNotStrip
 public interface ReactNativeFeatureFlagsProvider {
   @DoNotStrip public fun commonTestFlag(): Boolean
 
+  @DoNotStrip public fun androidEnablePendingFabricTransactions(): Boolean
+
   @DoNotStrip public fun batchRenderingUpdatesInEventLoop(): Boolean
 
   @DoNotStrip public fun destroyFabricSurfacesInReactInstanceManager(): Boolean

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

@@ -462,16 +462,51 @@ std::shared_ptr<FabricMountingManager> Binding::getMountingManager(
 
 void Binding::schedulerDidFinishTransaction(
     const MountingCoordinator::Shared& mountingCoordinator) {
-  auto mountingManager = getMountingManager("schedulerDidFinishTransaction");
-  if (!mountingManager) {
+  if (!ReactNativeFeatureFlags::androidEnablePendingFabricTransactions()) {
     return;
   }
 
   auto mountingTransaction = mountingCoordinator->pullTransaction();
   if (!mountingTransaction.has_value()) {
     return;
   }
-  mountingManager->executeMount(*mountingTransaction);
+
+  std::unique_lock<std::mutex> lock(pendingTransactionsMutex_);
+  auto pendingTransaction = std::find_if(
+      pendingTransactions_.begin(),
+      pendingTransactions_.end(),
+      [&](const auto& transaction) {
+        return transaction.getSurfaceId() ==
+            mountingTransaction->getSurfaceId();
+      });
+
+  if (pendingTransaction != pendingTransactions_.end()) {
+    pendingTransaction->mergeWith(std::move(*mountingTransaction));
+  } else {
+    pendingTransactions_.push_back(std::move(*mountingTransaction));
+  }
+}
+
+void Binding::schedulerShouldRenderTransactions(
+    const MountingCoordinator::Shared& mountingCoordinator) {
+  auto mountingManager =
+      getMountingManager("schedulerShouldRenderTransactions");
+  if (!mountingManager) {
+    return;
+  }
+
+  if (ReactNativeFeatureFlags::androidEnablePendingFabricTransactions()) {
+    std::unique_lock<std::mutex> lock(pendingTransactionsMutex_);
+    for (auto& transaction : pendingTransactions_) {
+      mountingManager->executeMount(transaction);
+    }
+    pendingTransactions_.clear();
+  } else {
+    auto mountingTransaction = mountingCoordinator->pullTransaction();
+    if (mountingTransaction.has_value()) {
+      mountingManager->executeMount(*mountingTransaction);
+    }
+  }
 }
 
 void Binding::schedulerDidRequestPreliminaryViewAllocation(

packages/react-native/ReactAndroid/src/main/jni/react/fabric/Binding.h

@@ -8,6 +8,7 @@
 #pragma once
 
 #include <memory>
+#include <mutex>
 #include <shared_mutex>
 #include <unordered_map>
 
@@ -101,6 +102,9 @@ class Binding : public jni::HybridClass<Binding, JBinding>,
   void schedulerDidFinishTransaction(
       const MountingCoordinator::Shared& mountingCoordinator) override;
 
+  void schedulerShouldRenderTransactions(
+      const MountingCoordinator::Shared& mountingCoordinator) override;
+
   void schedulerDidRequestPreliminaryViewAllocation(
       const SurfaceId surfaceId,
       const ShadowNode& shadowNode) override;
@@ -146,6 +150,10 @@ class Binding : public jni::HybridClass<Binding, JBinding>,
   std::shared_mutex
       surfaceHandlerRegistryMutex_; // Protects `surfaceHandlerRegistry_`.
 
+  // Track pending transactions, one per surfaceId
+  std::mutex pendingTransactionsMutex_;
+  std::vector<MountingTransaction> pendingTransactions_;
+
   float pointScaleFactor_ = 1;
 
   std::shared_ptr<const ReactNativeConfig> reactNativeConfig_{nullptr};

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<<65395bf89177951ff8f66f9567fd4399>>
+ * @generated SignedSource<<ec76fca802fcc6f2c2357de21f482cb3>>
  */
 
 /**
@@ -45,6 +45,12 @@ class ReactNativeFeatureFlagsProviderHolder
     return method(javaProvider_);
   }
 
+  bool androidEnablePendingFabricTransactions() override {
+    static const auto method =
+        getReactNativeFeatureFlagsProviderJavaClass()->getMethod<jboolean()>("androidEnablePendingFabricTransactions");
+    return method(javaProvider_);
+  }
+
   bool batchRenderingUpdatesInEventLoop() override {
     static const auto method =
         getReactNativeFeatureFlagsProviderJavaClass()->getMethod<jboolean()>("batchRenderingUpdatesInEventLoop");
@@ -150,6 +156,11 @@ bool JReactNativeFeatureFlagsCxxInterop::commonTestFlag(
   return ReactNativeFeatureFlags::commonTestFlag();
 }
 
+bool JReactNativeFeatureFlagsCxxInterop::androidEnablePendingFabricTransactions(
+    facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop> /*unused*/) {
+  return ReactNativeFeatureFlags::androidEnablePendingFabricTransactions();
+}
+
 bool JReactNativeFeatureFlagsCxxInterop::batchRenderingUpdatesInEventLoop(
     facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop> /*unused*/) {
   return ReactNativeFeatureFlags::batchRenderingUpdatesInEventLoop();
@@ -250,6 +261,9 @@ void JReactNativeFeatureFlagsCxxInterop::registerNatives() {
       makeNativeMethod(
         "commonTestFlag",
         JReactNativeFeatureFlagsCxxInterop::commonTestFlag),
+      makeNativeMethod(
+        "androidEnablePendingFabricTransactions",
+        JReactNativeFeatureFlagsCxxInterop::androidEnablePendingFabricTransactions),
       makeNativeMethod(
         "batchRenderingUpdatesInEventLoop",
         JReactNativeFeatureFlagsCxxInterop::batchRenderingUpdatesInEventLoop),

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<<4d8c4b2a92dac45194b7e7b6ab9bab58>>
+ * @generated SignedSource<<3a6ff4e2f6d4056d903542cc620e07a9>>
  */
 
 /**
@@ -33,6 +33,9 @@ class JReactNativeFeatureFlagsCxxInterop
   static bool commonTestFlag(
     facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);
 
+  static bool androidEnablePendingFabricTransactions(
+    facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);
+
   static bool batchRenderingUpdatesInEventLoop(
     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<<491afbd43963ba14bb2d4741e23e0879>>
+ * @generated SignedSource<<6305ea7c2cb59caeaf2ea9cea69b8203>>
  */
 
 /**
@@ -25,6 +25,10 @@ bool ReactNativeFeatureFlags::commonTestFlag() {
   return getAccessor().commonTestFlag();
 }
 
+bool ReactNativeFeatureFlags::androidEnablePendingFabricTransactions() {
+  return getAccessor().androidEnablePendingFabricTransactions();
+}
+
 bool ReactNativeFeatureFlags::batchRenderingUpdatesInEventLoop() {
   return getAccessor().batchRenderingUpdatesInEventLoop();
 }

packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.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<<d440f151b718198b83d18dcd1ca157d1>>
+ * @generated SignedSource<<5a5c6772253f49b0b768cd7ef090af14>>
  */
 
 /**
@@ -42,6 +42,11 @@ class ReactNativeFeatureFlags {
    */
   RN_EXPORT static bool commonTestFlag();
 
+  /**
+   * To be used with batchRenderingUpdatesInEventLoop. When enbled, the Android mounting layer will concatenate pending transactions to ensure they're applied atomatically
+   */
+  RN_EXPORT static bool androidEnablePendingFabricTransactions();
+
   /**
    * When enabled, the RuntimeScheduler processing the event loop will batch all rendering updates and dispatch them together at the end of each iteration of the loop.
    */