Skip to content

Commit c8ced57

Browse files
Document committed semantics of hasBeenMounted and threading (#59108)
Summary: Pull Request resolved: #59108 Review follow-ups for the preallocated view cleanup in the pull model: - Document that `ShadowNodeFamily::setMounted()` and `hasBeenMounted()` refer to a node of the family being part of a committed shadow tree, which can happen before the platform mounts its view. This matches how `ShadowNode::setMounted` and `updateMountedFlag` use "mounted", so the names stay. - Document the threading requirements of the `FabricMountingManager` preallocation and mount methods. - Rename the test helper `mountCreate` to `executeCreateMutation`. Changelog: [Internal] Differential Revision: D125436483
1 parent 8237407 commit c8ced57

6 files changed

Lines changed: 20 additions & 9 deletions

File tree

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,22 @@ class FabricMountingManager final {
3232

3333
void onSurfaceStop(SurfaceId surfaceId);
3434

35+
/*
36+
* Can be called from any thread.
37+
*/
3538
void maybePreallocateShadowNode(const ShadowNode &shadowNode);
3639

40+
/*
41+
* Can be called from any thread.
42+
*/
3743
void destroyUnmountedShadowNode(const ShadowNodeFamily &family);
3844

3945
/*
4046
* Drains preallocatedViewsQueue_: preallocates each queued view on the Java
4147
* side and registers its tag in allocatedViewRegistry_ so that executeMount
4248
* skips the redundant Create mount item for this tag. In the pull model, it
43-
* skips the views whose family was destroyed. Can be called by any thread.
49+
* skips the views whose family was destroyed. Must be called on the UI
50+
* thread.
4451
*/
4552
void drainPreallocateViewsQueue();
4653

‎packages/react-native/ReactAndroid/src/main/jni/react/fabric/test_helper/FabricMountingManagerTestHelper.cpp‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,9 @@ void FabricMountingManagerTestHelper::drainPreallocationQueue() {
117117
mountingManager_->drainPreallocateViewsQueue();
118118
}
119119

120-
void FabricMountingManagerTestHelper::mountCreate(jint surfaceId, jint tag) {
120+
void FabricMountingManagerTestHelper::executeCreateMutation(
121+
jint surfaceId,
122+
jint tag) {
121123
auto telemetry = TransactionTelemetry{};
122124
telemetry.willCommit();
123125
telemetry.willDiff();
@@ -300,7 +302,8 @@ void FabricMountingManagerTestHelper::registerNatives() {
300302
"drainPreallocationQueue",
301303
FabricMountingManagerTestHelper::drainPreallocationQueue),
302304
makeNativeMethod(
303-
"mountCreate", FabricMountingManagerTestHelper::mountCreate),
305+
"executeCreateMutation",
306+
FabricMountingManagerTestHelper::executeCreateMutation),
304307
makeNativeMethod(
305308
"dropFamily", FabricMountingManagerTestHelper::dropFamily),
306309
makeNativeMethod(

‎packages/react-native/ReactAndroid/src/main/jni/react/fabric/test_helper/FabricMountingManagerTestHelper.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ class FabricMountingManagerTestHelper : public jni::HybridClass<FabricMountingMa
3131
void preallocateView(jint surfaceId, jint tag);
3232
void queuePreallocation(jint surfaceId, jint tag);
3333
void drainPreallocationQueue();
34-
void mountCreate(jint surfaceId, jint tag);
34+
void executeCreateMutation(jint surfaceId, jint tag);
3535
void dropFamily(jint tag);
3636
void destroyUnmountedView(jint surfaceId, jint tag);
3737
void destroyCommittedView(jint surfaceId, jint tag);

‎packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/FabricMountingManagerInstrumentationTest.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,7 @@ class FabricMountingManagerInstrumentationTest {
432432
val helper = createTestHelper()
433433
helper.startSurface(surfaceId)
434434
helper.preallocateView(surfaceId, 42)
435-
helper.mountCreate(surfaceId, 42)
435+
helper.executeCreateMutation(surfaceId, 42)
436436

437437
helper.destroyCommittedView(surfaceId, 42)
438438

‎packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/FabricMountingManagerTestHelper.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ class FabricMountingManagerTestHelper private constructor(fabricUIManager: Fabri
3535

3636
external fun drainPreallocationQueue()
3737

38-
external fun mountCreate(surfaceId: Int, tag: Int)
38+
external fun executeCreateMutation(surfaceId: Int, tag: Int)
3939

4040
external fun dropFamily(tag: Int)
4141

‎packages/react-native/ReactCommon/react/renderer/core/ShadowNodeFamily.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,13 @@ class ShadowNodeFamily final : public jsi::NativeState {
107107
void setMostRecentState(const std::shared_ptr<const State> &state);
108108

109109
/**
110-
* Mark this ShadowNodeFamily as mounted.
110+
* Mark this ShadowNodeFamily as part of a committed shadow tree.
111111
*/
112112
void setMounted();
113113

114114
/**
115-
* Whether this ShadowNodeFamily was ever mounted.
115+
* Whether this ShadowNodeFamily was ever part of a committed shadow tree.
116+
* Its view may not have been mounted on the platform yet.
116117
*/
117118
bool hasBeenMounted() const;
118119

@@ -206,7 +207,7 @@ class ShadowNodeFamily final : public jsi::NativeState {
206207
bool hasParent_{false};
207208

208209
/*
209-
* Determines if the ShadowNodeFamily was ever mounted on the screen.
210+
* Determines if the ShadowNodeFamily was ever part of a committed shadow tree.
210211
*/
211212
bool hasBeenMounted_{false};
212213
};

0 commit comments

Comments
 (0)