Skip to content

Commit 7fef845

Browse files
Bartlomiej Bloniarzfacebook-github-bot
authored andcommitted
Drive shared animation backend from Fabric frame callback
Summary: 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 Differential Revision: D110321362
1 parent 1136e41 commit 7fef845

10 files changed

Lines changed: 23 additions & 192 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
@@ -1620,6 +1620,10 @@ public void doFrameGuarded(long frameTimeNanos) {
16201620
// 2. In case there are no view commands or mount items, wait until next frame.
16211621
mMountItemDispatcher.dispatchPreMountItems(frameTimeNanos);
16221622
mMountItemDispatcher.tryDispatchMountItems();
1623+
FabricUIManagerBinding binding = mBinding;
1624+
if (ReactNativeFeatureFlags.useSharedAnimatedBackend() && binding != null) {
1625+
binding.driveAnimationBackend(frameTimeNanos);
1626+
}
16231627
} catch (Exception ex) {
16241628
FLog.e(TAG, "Exception thrown when executing UIFrameGuarded", ex);
16251629
mIsMountingEnabled = false;

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: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,10 @@ 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+
auto frameTimeMs = static_cast<double>(frameTimeNanos) / 1000000.0;
66+
animationChoreographer_->onAnimationFrameIfActive(
67+
AnimationTimestamp{frameTimeMs});
6668
}
6769

6870
void FabricUIManagerBinding::drainPreallocateViewsQueue() {
@@ -572,6 +574,8 @@ void FabricUIManagerBinding::installFabricUIManager(
572574

573575
contextContainer->insert("FabricUIManager", globalJavaUiManager);
574576

577+
animationChoreographer_ = std::make_shared<AndroidAnimationChoreographer>();
578+
575579
auto toolbox = SchedulerToolbox{};
576580
toolbox.contextContainer = contextContainer;
577581
toolbox.componentRegistryFactory = componentsRegistry->buildRegistryFunction;
@@ -583,9 +587,6 @@ void FabricUIManagerBinding::installFabricUIManager(
583587

584588
toolbox.eventBeatFactory = eventBeatFactory;
585589

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

591592
animationDriver_ = std::make_shared<LayoutAnimationDriver>(
@@ -604,6 +605,7 @@ void FabricUIManagerBinding::uninstallFabricUIManager() {
604605
std::unique_lock lock(installMutex_);
605606
animationDriver_ = nullptr;
606607
scheduler_ = nullptr;
608+
animationChoreographer_ = nullptr;
607609
mountingManager_ = nullptr;
608610
}
609611

@@ -875,19 +877,9 @@ void FabricUIManagerBinding::registerNatives() {
875877
makeNativeMethod(
876878
"getRelativeAncestorList",
877879
FabricUIManagerBinding::getRelativeAncestorList),
878-
makeNativeMethod(
879-
"setAnimationBackendChoreographer",
880-
FabricUIManagerBinding::setAnimationBackendChoreographer),
881880
makeNativeMethod(
882881
"mergeReactRevision", FabricUIManagerBinding::mergeReactRevision),
883882
});
884883
}
885884

886-
void FabricUIManagerBinding::setAnimationBackendChoreographer(
887-
jni::alias_ref<JAnimationBackendChoreographer::javaobject>
888-
animationBackendChoreographer) {
889-
animationChoreographer_ = std::make_shared<AndroidAnimationChoreographer>(
890-
animationBackendChoreographer);
891-
}
892-
893885
} // 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)