Skip to content

Commit bd133b5

Browse files
javachefacebook-github-bot
authored andcommitted
Add featureflag to not re-order mount items in FabricMountingManager (facebook#46702)
Summary: Pull Request resolved: facebook#46702 In facebook#44188, we've started combining multiple transactions in a single transaction, to meet React's atomicity requirements, while also dealing with the constraints of Android's Fabric implementation. This revealed a bug where in some scenarios (especially when using transitions), a node may be deleted and created during the same transaction. The current implementation of FabricMountingManager assumes it can safely reorder some operations, which it does to optimize the size of IntBufferBatch mount items. This is however incorrect and unsafe when multiple transactions are merged. **Example:** Differentiator output: ``` # Transaction 1 Remove microsoft#100 from microsoft#11 Delete microsoft#100 # Transaction 2 Create microsoft#100 Insert microsoft#100 into microsoft#11 ``` FabricMountingManager output ``` Remove microsoft#100 from microsoft#11 Insert microsoft#100 into microsoft#11 Delete microsoft#100 ``` Note that the create action is also skipped, because we only update `allocatedViewTags` after processing all mutations, leading FabricMountingManager to assume creation is not required. This leads to an invalid state in SurfaceMountingManager, which will be surfaced as a crash in `getViewState` on the next mutation that interacts with these views. Changelog: [Android][Fixed] Fix crash in getViewState when using suspense fallbacks. Reviewed By: sammy-SC Differential Revision: D63148523 fbshipit-source-id: 07ae26b2f7b7eba1b9784041dd3059b0956c035e
1 parent dd43279 commit bd133b5

20 files changed

+257
-113
lines changed

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<f98a6ddd268fb59b5ee5edb4997b3a63>>
7+
* @generated SignedSource<<575eeb1e291c1a372eba7aabcdd948e3>>
88
*/
99

1010
/**
@@ -52,6 +52,12 @@ public object ReactNativeFeatureFlags {
5252
@JvmStatic
5353
public fun disableEventLoopOnBridgeless(): Boolean = accessor.disableEventLoopOnBridgeless()
5454

55+
/**
56+
* Prevent FabricMountingManager from reordering mountitems, which may lead to invalid state on the UI thread
57+
*/
58+
@JvmStatic
59+
public fun disableMountItemReorderingAndroid(): Boolean = accessor.disableMountItemReorderingAndroid()
60+
5561
/**
5662
* Kill-switch to turn off support for aling-items:baseline on Fabric iOS.
5763
*/

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<6b3d3512d88c836dd809204cad636211>>
7+
* @generated SignedSource<<f93759a639dbbb95d3307003e4907c86>>
88
*/
99

1010
/**
@@ -24,6 +24,7 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
2424
private var allowRecursiveCommitsWithSynchronousMountOnAndroidCache: Boolean? = null
2525
private var completeReactInstanceCreationOnBgThreadOnAndroidCache: Boolean? = null
2626
private var disableEventLoopOnBridgelessCache: Boolean? = null
27+
private var disableMountItemReorderingAndroidCache: Boolean? = null
2728
private var enableAlignItemsBaselineOnFabricIOSCache: Boolean? = null
2829
private var enableAndroidLineHeightCenteringCache: Boolean? = null
2930
private var enableBridgelessArchitectureCache: Boolean? = null
@@ -104,6 +105,15 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
104105
return cached
105106
}
106107

108+
override fun disableMountItemReorderingAndroid(): Boolean {
109+
var cached = disableMountItemReorderingAndroidCache
110+
if (cached == null) {
111+
cached = ReactNativeFeatureFlagsCxxInterop.disableMountItemReorderingAndroid()
112+
disableMountItemReorderingAndroidCache = cached
113+
}
114+
return cached
115+
}
116+
107117
override fun enableAlignItemsBaselineOnFabricIOS(): Boolean {
108118
var cached = enableAlignItemsBaselineOnFabricIOSCache
109119
if (cached == null) {

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<0ef9a66ccabeb0357f4b15c3e897f8fb>>
7+
* @generated SignedSource<<44d0fe9a36e5e51816e10b8799d451fe>>
88
*/
99

1010
/**
@@ -36,6 +36,8 @@ public object ReactNativeFeatureFlagsCxxInterop {
3636

3737
@DoNotStrip @JvmStatic public external fun disableEventLoopOnBridgeless(): Boolean
3838

39+
@DoNotStrip @JvmStatic public external fun disableMountItemReorderingAndroid(): Boolean
40+
3941
@DoNotStrip @JvmStatic public external fun enableAlignItemsBaselineOnFabricIOS(): Boolean
4042

4143
@DoNotStrip @JvmStatic public external fun enableAndroidLineHeightCentering(): Boolean

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<ee5400241a72652a2f8ff343a06e4e8c>>
7+
* @generated SignedSource<<917a6effbfd0a476cc05d90abee3c80b>>
88
*/
99

1010
/**
@@ -31,6 +31,8 @@ public open class ReactNativeFeatureFlagsDefaults : ReactNativeFeatureFlagsProvi
3131

3232
override fun disableEventLoopOnBridgeless(): Boolean = false
3333

34+
override fun disableMountItemReorderingAndroid(): Boolean = false
35+
3436
override fun enableAlignItemsBaselineOnFabricIOS(): Boolean = true
3537

3638
override fun enableAndroidLineHeightCentering(): Boolean = false

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<949f5cdf6d0a4015fbd680ba718dce6d>>
7+
* @generated SignedSource<<2ad36465b1a411cb55d85416bd8ba823>>
88
*/
99

1010
/**
@@ -28,6 +28,7 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
2828
private var allowRecursiveCommitsWithSynchronousMountOnAndroidCache: Boolean? = null
2929
private var completeReactInstanceCreationOnBgThreadOnAndroidCache: Boolean? = null
3030
private var disableEventLoopOnBridgelessCache: Boolean? = null
31+
private var disableMountItemReorderingAndroidCache: Boolean? = null
3132
private var enableAlignItemsBaselineOnFabricIOSCache: Boolean? = null
3233
private var enableAndroidLineHeightCenteringCache: Boolean? = null
3334
private var enableBridgelessArchitectureCache: Boolean? = null
@@ -112,6 +113,16 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
112113
return cached
113114
}
114115

116+
override fun disableMountItemReorderingAndroid(): Boolean {
117+
var cached = disableMountItemReorderingAndroidCache
118+
if (cached == null) {
119+
cached = currentProvider.disableMountItemReorderingAndroid()
120+
accessedFeatureFlags.add("disableMountItemReorderingAndroid")
121+
disableMountItemReorderingAndroidCache = cached
122+
}
123+
return cached
124+
}
125+
115126
override fun enableAlignItemsBaselineOnFabricIOS(): Boolean {
116127
var cached = enableAlignItemsBaselineOnFabricIOSCache
117128
if (cached == null) {

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<4e42e76c98b7434273e4f11212f0527b>>
7+
* @generated SignedSource<<9770a9f125b8bcb4b1daef9e3458433f>>
88
*/
99

1010
/**
@@ -31,6 +31,8 @@ public interface ReactNativeFeatureFlagsProvider {
3131

3232
@DoNotStrip public fun disableEventLoopOnBridgeless(): Boolean
3333

34+
@DoNotStrip public fun disableMountItemReorderingAndroid(): Boolean
35+
3436
@DoNotStrip public fun enableAlignItemsBaselineOnFabricIOS(): Boolean
3537

3638
@DoNotStrip public fun enableAndroidLineHeightCentering(): Boolean

0 commit comments

Comments
 (0)