From 85ed048a5b237748d5176962a6fc72571de36b35 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rub=C3=A9n=20Norte?= <rubennorte@meta.com>
Date: Wed, 6 Mar 2024 07:56:16 -0800
Subject: [PATCH] Improve logic to report mounted surfaces (#43337)

Summary:

Changelog: [internal]

Mount instructions and listeners are only called from the UI thread, so there's no need to have synchronization mechanisms for concurrency.

We're also scheduling mount hooks notifications once, but subsequent calls are ignored instead of accumulated to be notified together. This also changes that to collect all the surface IDs in the array that is read on notification.

Differential Revision: D54547194
---
 .../react/fabric/FabricUIManager.java         | 41 ++++++++++---------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java
index 4b5adcbf972a78..e11dd3637030ae 100644
--- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java
+++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java
@@ -91,7 +91,6 @@
 import java.util.Map;
 import java.util.Queue;
 import java.util.concurrent.CopyOnWriteArrayList;
-import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
  * We instruct ProGuard not to strip out any fields or methods, because many of these methods are
@@ -173,7 +172,8 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
   @NonNull
   private final CopyOnWriteArrayList<UIManagerListener> mListeners = new CopyOnWriteArrayList<>();
 
-  @NonNull private final AtomicBoolean mMountNotificationScheduled = new AtomicBoolean(false);
+  private boolean mMountNotificationScheduled = false;
+  private final List<Integer> mMountedSurfaceIds = new ArrayList<>();
 
   @ThreadConfined(UI)
   @NonNull
@@ -1203,6 +1203,8 @@ public Map<String, Long> getPerformanceCounters() {
   }
 
   private class MountItemDispatchListener implements MountItemDispatcher.ItemDispatchListener {
+    @UiThread
+    @ThreadConfined(UI)
     @Override
     public void willMountItems(@Nullable List<MountItem> mountItems) {
       for (UIManagerListener listener : mListeners) {
@@ -1210,18 +1212,26 @@ public void willMountItems(@Nullable List<MountItem> mountItems) {
       }
     }
 
+    @UiThread
+    @ThreadConfined(UI)
     @Override
     public void didMountItems(@Nullable List<MountItem> mountItems) {
       for (UIManagerListener listener : mListeners) {
         listener.didMountItems(FabricUIManager.this);
       }
 
-      if (!ReactFeatureFlags.enableMountHooks) {
+      if (!ReactFeatureFlags.enableMountHooks || mountItems == null || mountItems.isEmpty()) {
         return;
       }
 
-      boolean mountNotificationScheduled = mMountNotificationScheduled.getAndSet(true);
-      if (!mountNotificationScheduled) {
+      // Collect surface IDs for all the mount items
+      for (MountItem mountItem : mountItems) {
+        if (mountItem != null && !mMountedSurfaceIds.contains(mountItem.getSurfaceId())) {
+          mMountedSurfaceIds.add(mountItem.getSurfaceId());
+        }
+      }
+
+      if (!mMountNotificationScheduled && !mMountedSurfaceIds.isEmpty()) {
         // Notify mount when the effects are visible and prevent mount hooks to
         // delay paint.
         UiThreadUtil.getUiThreadHandler()
@@ -1229,28 +1239,19 @@ public void didMountItems(@Nullable List<MountItem> mountItems) {
                 new Runnable() {
                   @Override
                   public void run() {
-                    mMountNotificationScheduled.set(false);
-
-                    if (mDestroyed) {
-                      return;
-                    }
+                    mMountNotificationScheduled = false;
 
                     final @Nullable Binding binding = mBinding;
-                    if (mountItems == null || binding == null) {
+                    if (binding == null || mDestroyed) {
+                      mMountedSurfaceIds.clear();
                       return;
                     }
 
-                    // Collect surface IDs for all the mount items
-                    List<Integer> surfaceIds = new ArrayList<>();
-                    for (MountItem mountItem : mountItems) {
-                      if (mountItem != null && !surfaceIds.contains(mountItem.getSurfaceId())) {
-                        surfaceIds.add(mountItem.getSurfaceId());
-                      }
-                    }
-
-                    for (int surfaceId : surfaceIds) {
+                    for (int surfaceId : mMountedSurfaceIds) {
                       binding.reportMount(surfaceId);
                     }
+
+                    mMountedSurfaceIds.clear();
                   }
                 });
       }