Skip to content

Commit 721b1cf

Browse files
Bartlomiej Bloniarzmeta-codesync[bot]
authored andcommitted
Drive shared animation backend from Fabric frame callback (#57400)
Summary: Pull Request resolved: #57400 In this diff we drop the custom choreographer for the backend on Android and instead plug into the one in `FabricUIManager`. This reduces the area for possible mistakes, makes it clearer how Fabric interacts with animation on a per-frame basis, and simplifies the flow around invalidation and cleanup of the React instance. The crash this is meant to avoid comes from having two separate frame callback lifecycles. The old `AnimationBackendChoreographer` owned a self-reposting callback that could keep driving `FabricUIManagerBinding.driveAnimationBackend` independently from Fabric's own lifecycle. During React instance teardown, `FabricUIManager.invalidate()` pauses Fabric's frame callback and then unregisters the native binding. If a separate backend callback survives that sequence, it can invoke the binding after the native side has been uninstalled. The shared animation backend is now driven from Fabric's existing `DISPATCH_UI` frame callback after mount items are dispatched. The Android `AnimationChoreographer` implementation only owns backend pause/resume state and conditionally forwards active frames to the shared backend. Threading-wise: - If invalidation happens before a frame starts, `mDestroyed` makes the frame no-op. - If invalidation races with an already-running frame, `ReactChoreographer.removeFrameCallback` is serialized with callback execution via the `callbackQueues` monitor, so `onHostPause()` waits for the current `DISPATCH_UI` callback to finish before `unregister()` tears down the native binding. - If the frame reposts itself in `schedule()`, the blocked removal observes and removes that callback before teardown continues. Changelog: [Android][Fixed] - Drive the shared animation backend from Fabric's frame callback during React instance teardown Reviewed By: javache, zeyap Differential Revision: D110321362 fbshipit-source-id: 77c462ee30aeec2d3af0dcd48b8eed15846ae5da
1 parent 923e7dd commit 721b1cf

13 files changed

Lines changed: 31 additions & 213 deletions

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

Lines changed: 0 additions & 87 deletions
This file was deleted.

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1628,6 +1628,10 @@ public void doFrameGuarded(long frameTimeNanos) {
16281628
schedule();
16291629
}
16301630

1631+
if (ReactNativeFeatureFlags.useSharedAnimatedBackend() && mBinding != null) {
1632+
mBinding.driveAnimationBackend(frameTimeNanos);
1633+
}
1634+
16311635
mSynchronousEvents.clear();
16321636
}
16331637
}

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

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -79,16 +79,12 @@ internal class FabricUIManagerBinding : HybridClassBase() {
7979

8080
external fun driveCxxAnimations()
8181

82-
external fun driveAnimationBackend(frameTimeMs: Double)
82+
external fun driveAnimationBackend(frameTimeNanos: Long)
8383

8484
external fun drainPreallocateViewsQueue()
8585

8686
external fun reportMount(surfaceId: Int)
8787

88-
external fun setAnimationBackendChoreographer(
89-
animationBackendChoreographer: AnimationBackendChoreographer
90-
)
91-
9288
external fun mergeReactRevision(surfaceId: Int)
9389

9490
fun register(
@@ -97,13 +93,8 @@ internal class FabricUIManagerBinding : HybridClassBase() {
9793
fabricUIManager: FabricUIManager,
9894
eventBeatManager: EventBeatManager,
9995
componentFactory: ComponentFactory,
100-
animationBackendChoreographer: AnimationBackendChoreographer,
10196
) {
10297
fabricUIManager.setBinding(this)
103-
animationBackendChoreographer.frameCallback = AnimationFrameCallback { frameTimeMs: Double ->
104-
driveAnimationBackend(frameTimeMs)
105-
}
106-
setAnimationBackendChoreographer(animationBackendChoreographer)
10798

10899
installFabricUIManager(
109100
runtimeExecutor,

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,16 +58,13 @@ public class FabricUIManagerProviderImpl(
5858
val runtimeExecutor = catalystInstance?.runtimeExecutor
5959
val runtimeScheduler = catalystInstance?.runtimeScheduler
6060

61-
val animationBackendChoreographer = AnimationBackendChoreographer(context)
62-
6361
if (runtimeExecutor != null && runtimeScheduler != null) {
6462
binding.register(
6563
runtimeExecutor,
6664
runtimeScheduler,
6765
fabricUIManager,
6866
eventBeatManager,
6967
componentFactory,
70-
animationBackendChoreographer,
7168
)
7269
} else {
7370
throw IllegalStateException(

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactInstance.kt

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ import com.facebook.react.common.annotations.UnstableReactNativeAPI
4444
import com.facebook.react.devsupport.InspectorFlags.getIsProfilingBuild
4545
import com.facebook.react.devsupport.StackTraceHelper
4646
import com.facebook.react.devsupport.interfaces.DevSupportManager
47-
import com.facebook.react.fabric.AnimationBackendChoreographer
4847
import com.facebook.react.fabric.ComponentFactory
4948
import com.facebook.react.fabric.FabricUIManager
5049
import com.facebook.react.fabric.FabricUIManagerBinding
@@ -259,16 +258,13 @@ internal class ReactInstance(
259258
// Misc initialization that needs to be done before Fabric init
260259
DisplayMetricsHolder.initDisplayMetricsIfNotInitialized(context)
261260

262-
val animationBackendChoreographer = AnimationBackendChoreographer(context)
263-
264261
val binding = FabricUIManagerBinding()
265262
binding.register(
266263
getBufferedRuntimeExecutor(),
267264
getRuntimeScheduler(),
268265
fabricUIManager,
269266
eventBeatManager,
270267
componentFactory,
271-
animationBackendChoreographer,
272268
)
273269

274270
// Initialize the FabricUIManager

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,32 +7,32 @@
77

88
#pragma once
99

10-
#include <fbjni/fbjni.h>
1110
#include <react/renderer/animationbackend/AnimationChoreographer.h>
12-
13-
#include "JAnimationBackendChoreographer.h"
11+
#include <atomic>
1412

1513
namespace facebook::react {
1614

1715
class AndroidAnimationChoreographer : public AnimationChoreographer {
1816
public:
19-
explicit AndroidAnimationChoreographer(jni::alias_ref<JAnimationBackendChoreographer> jChoreographer)
20-
: jChoreographer_(jni::make_global(jChoreographer))
17+
void resume() override
2118
{
19+
active_.store(true);
2220
}
2321

24-
void resume() override
22+
void pause() override
2523
{
26-
jChoreographer_->resume();
24+
active_.store(false);
2725
}
2826

29-
void pause() override
27+
void onAnimationFrameIfActive(AnimationTimestamp timestamp) const
3028
{
31-
jChoreographer_->pause();
29+
if (active_.load()) {
30+
onAnimationFrame(timestamp);
31+
}
3232
}
3333

3434
private:
35-
jni::global_ref<JAnimationBackendChoreographer> jChoreographer_;
35+
std::atomic_bool active_{false};
3636
};
3737

3838
} // namespace facebook::react

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

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,15 @@ void FabricUIManagerBinding::driveCxxAnimations() {
6161
scheduler->animationTick();
6262
}
6363

64-
void FabricUIManagerBinding::driveAnimationBackend(jdouble frameTimeMs) {
65-
animationChoreographer_->onAnimationFrame(AnimationTimestamp{frameTimeMs});
64+
void FabricUIManagerBinding::driveAnimationBackend(jlong frameTimeNanos) {
65+
if (!animationChoreographer_) {
66+
LOG(ERROR)
67+
<< "FabricUIManagerBinding::driveAnimationBackend: animation choreographer disappeared";
68+
return;
69+
}
70+
auto frameTimeMs = static_cast<double>(frameTimeNanos) / 1000000.0;
71+
animationChoreographer_->onAnimationFrameIfActive(
72+
AnimationTimestamp{frameTimeMs});
6673
}
6774

6875
void FabricUIManagerBinding::drainPreallocateViewsQueue() {
@@ -572,6 +579,8 @@ void FabricUIManagerBinding::installFabricUIManager(
572579

573580
contextContainer->insert("FabricUIManager", globalJavaUiManager);
574581

582+
animationChoreographer_ = std::make_shared<AndroidAnimationChoreographer>();
583+
575584
auto toolbox = SchedulerToolbox{};
576585
toolbox.contextContainer = contextContainer;
577586
toolbox.componentRegistryFactory = componentsRegistry->buildRegistryFunction;
@@ -583,9 +592,6 @@ void FabricUIManagerBinding::installFabricUIManager(
583592

584593
toolbox.eventBeatFactory = eventBeatFactory;
585594

586-
react_native_assert(
587-
animationChoreographer_ != nullptr &&
588-
"AnimationChoreographer is nullptr");
589595
toolbox.animationChoreographer = animationChoreographer_;
590596

591597
animationDriver_ = std::make_shared<LayoutAnimationDriver>(
@@ -604,6 +610,7 @@ void FabricUIManagerBinding::uninstallFabricUIManager() {
604610
std::unique_lock lock(installMutex_);
605611
animationDriver_ = nullptr;
606612
scheduler_ = nullptr;
613+
animationChoreographer_ = nullptr;
607614
mountingManager_ = nullptr;
608615
}
609616

@@ -875,19 +882,9 @@ void FabricUIManagerBinding::registerNatives() {
875882
makeNativeMethod(
876883
"getRelativeAncestorList",
877884
FabricUIManagerBinding::getRelativeAncestorList),
878-
makeNativeMethod(
879-
"setAnimationBackendChoreographer",
880-
FabricUIManagerBinding::setAnimationBackendChoreographer),
881885
makeNativeMethod(
882886
"mergeReactRevision", FabricUIManagerBinding::mergeReactRevision),
883887
});
884888
}
885889

886-
void FabricUIManagerBinding::setAnimationBackendChoreographer(
887-
jni::alias_ref<JAnimationBackendChoreographer::javaobject>
888-
animationBackendChoreographer) {
889-
animationChoreographer_ = std::make_shared<AndroidAnimationChoreographer>(
890-
animationBackendChoreographer);
891-
}
892-
893890
} // namespace facebook::react

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ class FabricUIManagerBinding : public jni::HybridClass<FabricUIManagerBinding>,
127127

128128
void driveCxxAnimations();
129129

130-
void driveAnimationBackend(jdouble frameTimeMs);
130+
void driveAnimationBackend(jlong frameTimeNanos);
131131

132132
void drainPreallocateViewsQueue();
133133

@@ -168,8 +168,6 @@ class FabricUIManagerBinding : public jni::HybridClass<FabricUIManagerBinding>,
168168
bool enableFabricLogs_{false};
169169

170170
std::shared_ptr<AndroidAnimationChoreographer> animationChoreographer_;
171-
172-
void setAnimationBackendChoreographer(jni::alias_ref<JAnimationBackendChoreographer::javaobject> animationBackend);
173171
};
174172

175173
} // namespace facebook::react

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

Lines changed: 0 additions & 23 deletions
This file was deleted.

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

Lines changed: 0 additions & 37 deletions
This file was deleted.

0 commit comments

Comments
 (0)