Skip to content

Commit 490b05d

Browse files
javachefacebook-github-bot
authored andcommitted
Extract SynchronousMountItem (#53936)
Summary: Pull Request resolved: #53936 Align with other mount items and make it easier to identify in memory traces. Changelog: [Internal] Reviewed By: lenaic Differential Revision: D83241794 fbshipit-source-id: 9df1523e265e560c15f93bad8cd91a651bc5a4e2
1 parent 09441d8 commit 490b05d

6 files changed

Lines changed: 68 additions & 62 deletions

File tree

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java

Lines changed: 18 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@
6666
import com.facebook.react.fabric.mounting.mountitems.MountItem;
6767
import com.facebook.react.fabric.mounting.mountitems.MountItemFactory;
6868
import com.facebook.react.fabric.mounting.mountitems.PrefetchResourcesMountItem;
69+
import com.facebook.react.fabric.mounting.mountitems.SynchronousMountItem;
6970
import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags;
7071
import com.facebook.react.internal.featureflags.ReactNativeNewArchitectureFeatureFlags;
7172
import com.facebook.react.internal.interop.InteropEventEmitter;
@@ -178,7 +179,7 @@ public class FabricUIManager
178179

179180
private final BatchEventDispatchedListener mBatchEventDispatchedListener;
180181

181-
private final CopyOnWriteArrayList<UIManagerListener> mListeners = new CopyOnWriteArrayList<>();
182+
private final List<UIManagerListener> mListeners = new CopyOnWriteArrayList<>();
182183

183184
private boolean mMountNotificationScheduled = false;
184185
private List<Integer> mSurfaceIdsWithPendingMountNotification = new ArrayList<>();
@@ -791,35 +792,7 @@ public void synchronouslyUpdateViewOnUIThread(final int reactTag, final Readable
791792
// android.view.View.updateDisplayListIfDirty(View.java:20466)
792793
// 3. A view is deleted while its parent is being drawn, causing a crash.
793794

794-
MountItem synchronousMountItem =
795-
new MountItem() {
796-
@Override
797-
public void execute(MountingManager mountingManager) {
798-
try {
799-
mountingManager.storeSynchronousMountPropsOverride(reactTag, props);
800-
mountingManager.updatePropsSynchronously(reactTag, props);
801-
} catch (Exception ex) {
802-
// TODO T42943890: Fix animations in Fabric and remove this try/catch?
803-
// There might always be race conditions between surface teardown and
804-
// animations/other operations, so it may not be feasible to remove this.
805-
// Practically 100% of reported errors from this point are because the
806-
// surface has stopped by this point, but the MountItem was queued before
807-
// the surface was stopped. It's likely not feasible to prevent all such races.
808-
}
809-
}
810-
811-
@Override
812-
public int getSurfaceId() {
813-
return View.NO_ID;
814-
}
815-
816-
@Override
817-
public String toString() {
818-
String propsString =
819-
IS_DEVELOPMENT_ENVIRONMENT ? props.toHashMap().toString() : "<hidden>";
820-
return String.format("SYNC UPDATE PROPS [%d]: %s", reactTag, propsString);
821-
}
822-
};
795+
MountItem synchronousMountItem = new SynchronousMountItem(reactTag, props);
823796

824797
// If the reactTag exists, we assume that it might at the end of the next
825798
// batch of MountItems. Otherwise, we try to execute immediately.
@@ -1399,24 +1372,21 @@ public void didMountItems(@Nullable List<? extends MountItem> mountItems) {
13991372
// delay paint.
14001373
UiThreadUtil.getUiThreadHandler()
14011374
.postAtFrontOfQueue(
1402-
new Runnable() {
1403-
@Override
1404-
public void run() {
1405-
mMountNotificationScheduled = false;
1406-
1407-
// Create a copy in case mount hooks trigger more mutations
1408-
final List<Integer> surfaceIdsToReportMount =
1409-
mSurfaceIdsWithPendingMountNotification;
1410-
mSurfaceIdsWithPendingMountNotification = new ArrayList<>();
1411-
1412-
final @Nullable FabricUIManagerBinding binding = mBinding;
1413-
if (binding == null || mDestroyed) {
1414-
return;
1415-
}
1416-
1417-
for (int surfaceId : surfaceIdsToReportMount) {
1418-
binding.reportMount(surfaceId);
1419-
}
1375+
() -> {
1376+
mMountNotificationScheduled = false;
1377+
1378+
// Create a copy in case mount hooks trigger more mutations
1379+
final List<Integer> surfaceIdsToReportMount =
1380+
mSurfaceIdsWithPendingMountNotification;
1381+
mSurfaceIdsWithPendingMountNotification = new ArrayList<>();
1382+
1383+
final @Nullable FabricUIManagerBinding binding = mBinding;
1384+
if (binding == null || mDestroyed) {
1385+
return;
1386+
}
1387+
1388+
for (int surfaceId : surfaceIdsToReportMount) {
1389+
binding.reportMount(surfaceId);
14201390
}
14211391
});
14221392
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ internal class MountingManager(
5151
private val rootViewManager = RootViewManager()
5252

5353
internal fun interface MountItemExecutor {
54-
@UiThread @ThreadConfined(ThreadConfined.UI) fun executeItems(items: Queue<MountItem?>?)
54+
@UiThread @ThreadConfined(ThreadConfined.UI) fun executeItems(items: Queue<MountItem>)
5555
}
5656

5757
/**

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,14 @@ import com.facebook.react.fabric.mounting.MountingManager
1414
* for views that were preallcated but never mounted on the screen.
1515
*/
1616
internal class DestroyUnmountedViewMountItem(
17-
private val _surfaceId: Int,
17+
private val surfaceId: Int,
1818
private val reactTag: Int,
1919
) : MountItem {
2020

2121
override fun execute(mountingManager: MountingManager) {
22-
val surfaceMountingManager = mountingManager.getSurfaceManager(_surfaceId) ?: return
22+
val surfaceMountingManager = mountingManager.getSurfaceManager(surfaceId) ?: return
2323
surfaceMountingManager.deleteView(reactTag)
2424
}
2525

26-
override fun getSurfaceId(): Int = _surfaceId
26+
override fun getSurfaceId(): Int = surfaceId
2727
}

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
package com.facebook.react.fabric.mounting.mountitems
99

1010
import com.facebook.common.logging.FLog
11-
import com.facebook.proguard.annotations.DoNotStripAny
1211
import com.facebook.react.bridge.ReactMarker
1312
import com.facebook.react.bridge.ReactMarkerConstants
1413
import com.facebook.react.bridge.ReadableMap
@@ -31,7 +30,8 @@ import java.util.Locale
3130
* The purpose of encapsulating the array of MountItems this way, is to reduce the amount of
3231
* allocations in C++ and JNI round-trips.
3332
*/
34-
@DoNotStripAny
33+
private const val TAG = "IntBufferBatchMountItem"
34+
3535
internal class IntBufferBatchMountItem(
3636
private val surfaceId: Int,
3737
private val intBuffer: IntArray,
@@ -341,8 +341,6 @@ internal class IntBufferBatchMountItem(
341341
}
342342

343343
companion object {
344-
val TAG: String = IntBufferBatchMountItem::class.java.simpleName
345-
346344
const val INSTRUCTION_FLAG_MULTIPLE: Int = 1
347345

348346
const val INSTRUCTION_CREATE: Int = 2

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,17 @@ import com.facebook.react.bridge.ReactSoftExceptionLogger
1212
import com.facebook.react.bridge.RetryableMountingLayerException
1313
import com.facebook.react.fabric.mounting.MountingManager
1414

15+
private const val TAG = "SendAccessibilityEventMountItem"
16+
1517
internal class SendAccessibilityEventMountItem(
16-
private val _surfaceId: Int,
18+
private val surfaceId: Int,
1719
private val reactTag: Int,
1820
private val eventType: Int,
1921
) : MountItem {
2022

21-
private val TAG = "Fabric.SendAccessibilityEvent"
22-
2323
override fun execute(mountingManager: MountingManager) {
2424
try {
25-
mountingManager.sendAccessibilityEvent(_surfaceId, reactTag, eventType)
25+
mountingManager.sendAccessibilityEvent(surfaceId, reactTag, eventType)
2626
} catch (e: RetryableMountingLayerException) {
2727
// Accessibility events are similar to commands in that they're imperative
2828
// calls from JS, disconnected from the commit lifecycle, and therefore
@@ -35,7 +35,7 @@ internal class SendAccessibilityEventMountItem(
3535
}
3636
}
3737

38-
override fun getSurfaceId(): Int = _surfaceId
38+
override fun getSurfaceId(): Int = surfaceId
3939

40-
override fun toString(): String = "SendAccessibilityEventMountItem [$reactTag] $eventType"
40+
override fun toString(): String = "$TAG [$reactTag] $eventType"
4141
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
package com.facebook.react.fabric.mounting.mountitems
9+
10+
import android.view.View
11+
import com.facebook.react.bridge.ReadableMap
12+
import com.facebook.react.fabric.FabricUIManager.IS_DEVELOPMENT_ENVIRONMENT
13+
import com.facebook.react.fabric.mounting.MountingManager
14+
15+
internal class SynchronousMountItem(private val reactTag: Int, private val props: ReadableMap) :
16+
MountItem {
17+
18+
override fun execute(mountingManager: MountingManager) {
19+
try {
20+
mountingManager.storeSynchronousMountPropsOverride(reactTag, props)
21+
mountingManager.updatePropsSynchronously(reactTag, props)
22+
} catch (ex: Exception) {
23+
// TODO T42943890: Fix animations in Fabric and remove this try/catch?
24+
// There might always be race conditions between surface teardown and
25+
// animations/other operations, so it may not be feasible to remove this.
26+
// Practically 100% of reported errors from this point are because the
27+
// surface has stopped by this point, but the MountItem was queued before
28+
// the surface was stopped. It's likely not feasible to prevent all such races.
29+
}
30+
}
31+
32+
override fun toString(): String {
33+
val propsString = if (IS_DEVELOPMENT_ENVIRONMENT) props.toHashMap().toString() else "<hidden>"
34+
return "SYNC UPDATE PROPS [$reactTag]: $propsString"
35+
}
36+
37+
override fun getSurfaceId(): Int = View.NO_ID
38+
}

0 commit comments

Comments
 (0)