From 6450a739c04ec178b7da5f199e08298efd6200a6 Mon Sep 17 00:00:00 2001 From: Bartlomiej Bloniarz Date: Wed, 1 Jul 2026 11:04:55 -0700 Subject: [PATCH] Drive shared animation backend from Fabric frame callback (#57400) 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 Reviewed By: javache, zeyap Differential Revision: D110321362 --- .../fabric/AnimationBackendChoreographer.kt | 87 ------------------- .../react/fabric/FabricUIManager.java | 4 + .../react/fabric/FabricUIManagerBinding.kt | 11 +-- .../fabric/FabricUIManagerProviderImpl.kt | 3 - .../facebook/react/runtime/ReactInstance.kt | 4 - .../fabric/AndroidAnimationChoreographer.h | 20 ++--- .../react/fabric/FabricUIManagerBinding.cpp | 27 +++--- .../jni/react/fabric/FabricUIManagerBinding.h | 4 +- .../fabric/JAnimationBackendChoreographer.cpp | 23 ----- .../fabric/JAnimationBackendChoreographer.h | 37 -------- .../api-snapshots/ReactAndroidDebugCxx.api | 8 +- .../api-snapshots/ReactAndroidNewarchCxx.api | 8 +- .../api-snapshots/ReactAndroidReleaseCxx.api | 8 +- 13 files changed, 31 insertions(+), 213 deletions(-) delete mode 100644 packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/AnimationBackendChoreographer.kt delete mode 100644 packages/react-native/ReactAndroid/src/main/jni/react/fabric/JAnimationBackendChoreographer.cpp delete mode 100644 packages/react-native/ReactAndroid/src/main/jni/react/fabric/JAnimationBackendChoreographer.h diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/AnimationBackendChoreographer.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/AnimationBackendChoreographer.kt deleted file mode 100644 index c1134664e1a9..000000000000 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/AnimationBackendChoreographer.kt +++ /dev/null @@ -1,87 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -package com.facebook.react.fabric - -import com.facebook.proguard.annotations.DoNotStripAny -import com.facebook.react.bridge.ReactApplicationContext -import com.facebook.react.bridge.UiThreadUtil -import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags -import com.facebook.react.modules.core.ReactChoreographer -import com.facebook.react.uimanager.GuardedFrameCallback -import java.util.concurrent.atomic.AtomicBoolean - -internal fun interface AnimationFrameCallback { - fun onAnimationFrame(frameTimeMs: Double) -} - -@DoNotStripAny -internal class AnimationBackendChoreographer( - reactApplicationContext: ReactApplicationContext, -) { - - var frameCallback: AnimationFrameCallback? = null - private var lastFrameTimeMs: Double = 0.0 - private val reactChoreographer: ReactChoreographer = ReactChoreographer.getInstance() - private val choreographerCallback: GuardedFrameCallback = - object : GuardedFrameCallback(reactApplicationContext) { - override fun doFrameGuarded(frameTimeNanos: Long) { - executeFrameCallback(frameTimeNanos) - } - } - - // When true, the always-registered frame callback runs as a no-op. - // - // The callback is registered with ReactChoreographer once (on the UI thread) - // and re-posts itself every frame regardless of this flag, so it stays - // registered for the lifetime of the choreographer. - private val paused: AtomicBoolean = AtomicBoolean(true) - - init { - if (ReactNativeFeatureFlags.useSharedAnimatedBackend()) { - // Register the self-reposting callback once, on the UI thread, so the - // callback queues are only ever mutated from the UI thread. - UiThreadUtil.runOnUiThread { postCallback() } - } - } - - fun resume() { - paused.set(false) - } - - fun pause() { - paused.set(true) - } - - private fun postCallback() { - reactChoreographer.postFrameCallback( - ReactChoreographer.CallbackType.NATIVE_ANIMATED_MODULE, - choreographerCallback, - ) - } - - private fun executeFrameCallback(frameTimeNanos: Long) { - val currentFrameTimeMs = calculateTimestamp(frameTimeNanos) - // Only drive the animation backend while enabled. It is possible for the - // ChoreographerCallback to be executed twice within the same frame due to - // frame drops; if this occurs, the additional callback execution should be - // ignored. - if (!paused.get() && currentFrameTimeMs > lastFrameTimeMs) { - frameCallback?.onAnimationFrame(currentFrameTimeMs) - } - - lastFrameTimeMs = currentFrameTimeMs - // Always re-post (on the UI thread) so the callback stays registered for the - // next frame, whether or not we are currently paused. - postCallback() - } - - private fun calculateTimestamp(frameTimeNanos: Long): Double { - val nanosecondsInMilliseconds = 1000000.0 - return frameTimeNanos / nanosecondsInMilliseconds - } -} diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java index 6bc5d8f60f65..d96a0fa1e59c 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -1628,6 +1628,10 @@ public void doFrameGuarded(long frameTimeNanos) { schedule(); } + if (ReactNativeFeatureFlags.useSharedAnimatedBackend() && mBinding != null) { + mBinding.driveAnimationBackend(frameTimeNanos); + } + mSynchronousEvents.clear(); } } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManagerBinding.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManagerBinding.kt index cad82b4f1b13..84a6a17b208d 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManagerBinding.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManagerBinding.kt @@ -79,16 +79,12 @@ internal class FabricUIManagerBinding : HybridClassBase() { external fun driveCxxAnimations() - external fun driveAnimationBackend(frameTimeMs: Double) + external fun driveAnimationBackend(frameTimeNanos: Long) external fun drainPreallocateViewsQueue() external fun reportMount(surfaceId: Int) - external fun setAnimationBackendChoreographer( - animationBackendChoreographer: AnimationBackendChoreographer - ) - external fun mergeReactRevision(surfaceId: Int) fun register( @@ -97,13 +93,8 @@ internal class FabricUIManagerBinding : HybridClassBase() { fabricUIManager: FabricUIManager, eventBeatManager: EventBeatManager, componentFactory: ComponentFactory, - animationBackendChoreographer: AnimationBackendChoreographer, ) { fabricUIManager.setBinding(this) - animationBackendChoreographer.frameCallback = AnimationFrameCallback { frameTimeMs: Double -> - driveAnimationBackend(frameTimeMs) - } - setAnimationBackendChoreographer(animationBackendChoreographer) installFabricUIManager( runtimeExecutor, diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManagerProviderImpl.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManagerProviderImpl.kt index a0a81ff1fa38..8bcd7e658e0c 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManagerProviderImpl.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManagerProviderImpl.kt @@ -58,8 +58,6 @@ public class FabricUIManagerProviderImpl( val runtimeExecutor = catalystInstance?.runtimeExecutor val runtimeScheduler = catalystInstance?.runtimeScheduler - val animationBackendChoreographer = AnimationBackendChoreographer(context) - if (runtimeExecutor != null && runtimeScheduler != null) { binding.register( runtimeExecutor, @@ -67,7 +65,6 @@ public class FabricUIManagerProviderImpl( fabricUIManager, eventBeatManager, componentFactory, - animationBackendChoreographer, ) } else { throw IllegalStateException( diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactInstance.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactInstance.kt index 02942aca5b41..63b6802ad3db 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactInstance.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactInstance.kt @@ -44,7 +44,6 @@ import com.facebook.react.common.annotations.UnstableReactNativeAPI import com.facebook.react.devsupport.InspectorFlags.getIsProfilingBuild import com.facebook.react.devsupport.StackTraceHelper import com.facebook.react.devsupport.interfaces.DevSupportManager -import com.facebook.react.fabric.AnimationBackendChoreographer import com.facebook.react.fabric.ComponentFactory import com.facebook.react.fabric.FabricUIManager import com.facebook.react.fabric.FabricUIManagerBinding @@ -259,8 +258,6 @@ internal class ReactInstance( // Misc initialization that needs to be done before Fabric init DisplayMetricsHolder.initDisplayMetricsIfNotInitialized(context) - val animationBackendChoreographer = AnimationBackendChoreographer(context) - val binding = FabricUIManagerBinding() binding.register( getBufferedRuntimeExecutor(), @@ -268,7 +265,6 @@ internal class ReactInstance( fabricUIManager, eventBeatManager, componentFactory, - animationBackendChoreographer, ) // Initialize the FabricUIManager diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AndroidAnimationChoreographer.h b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AndroidAnimationChoreographer.h index b4cc3a4ced40..17124e4ecea4 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AndroidAnimationChoreographer.h +++ b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AndroidAnimationChoreographer.h @@ -7,32 +7,32 @@ #pragma once -#include #include - -#include "JAnimationBackendChoreographer.h" +#include namespace facebook::react { class AndroidAnimationChoreographer : public AnimationChoreographer { public: - explicit AndroidAnimationChoreographer(jni::alias_ref jChoreographer) - : jChoreographer_(jni::make_global(jChoreographer)) + void resume() override { + active_.store(true); } - void resume() override + void pause() override { - jChoreographer_->resume(); + active_.store(false); } - void pause() override + void onAnimationFrameIfActive(AnimationTimestamp timestamp) const { - jChoreographer_->pause(); + if (active_.load()) { + onAnimationFrame(timestamp); + } } private: - jni::global_ref jChoreographer_; + std::atomic_bool active_{false}; }; } // namespace facebook::react diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.cpp index dfaac96caefc..e1d3b3f32e0f 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.cpp @@ -61,8 +61,15 @@ void FabricUIManagerBinding::driveCxxAnimations() { scheduler->animationTick(); } -void FabricUIManagerBinding::driveAnimationBackend(jdouble frameTimeMs) { - animationChoreographer_->onAnimationFrame(AnimationTimestamp{frameTimeMs}); +void FabricUIManagerBinding::driveAnimationBackend(jlong frameTimeNanos) { + if (!animationChoreographer_) { + LOG(ERROR) + << "FabricUIManagerBinding::driveAnimationBackend: animation choreographer disappeared"; + return; + } + auto frameTimeMs = static_cast(frameTimeNanos) / 1000000.0; + animationChoreographer_->onAnimationFrameIfActive( + AnimationTimestamp{frameTimeMs}); } void FabricUIManagerBinding::drainPreallocateViewsQueue() { @@ -572,6 +579,8 @@ void FabricUIManagerBinding::installFabricUIManager( contextContainer->insert("FabricUIManager", globalJavaUiManager); + animationChoreographer_ = std::make_shared(); + auto toolbox = SchedulerToolbox{}; toolbox.contextContainer = contextContainer; toolbox.componentRegistryFactory = componentsRegistry->buildRegistryFunction; @@ -583,9 +592,6 @@ void FabricUIManagerBinding::installFabricUIManager( toolbox.eventBeatFactory = eventBeatFactory; - react_native_assert( - animationChoreographer_ != nullptr && - "AnimationChoreographer is nullptr"); toolbox.animationChoreographer = animationChoreographer_; animationDriver_ = std::make_shared( @@ -604,6 +610,7 @@ void FabricUIManagerBinding::uninstallFabricUIManager() { std::unique_lock lock(installMutex_); animationDriver_ = nullptr; scheduler_ = nullptr; + animationChoreographer_ = nullptr; mountingManager_ = nullptr; } @@ -875,19 +882,9 @@ void FabricUIManagerBinding::registerNatives() { makeNativeMethod( "getRelativeAncestorList", FabricUIManagerBinding::getRelativeAncestorList), - makeNativeMethod( - "setAnimationBackendChoreographer", - FabricUIManagerBinding::setAnimationBackendChoreographer), makeNativeMethod( "mergeReactRevision", FabricUIManagerBinding::mergeReactRevision), }); } -void FabricUIManagerBinding::setAnimationBackendChoreographer( - jni::alias_ref - animationBackendChoreographer) { - animationChoreographer_ = std::make_shared( - animationBackendChoreographer); -} - } // namespace facebook::react diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.h b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.h index caf652d2352e..60e730006778 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.h +++ b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.h @@ -127,7 +127,7 @@ class FabricUIManagerBinding : public jni::HybridClass, void driveCxxAnimations(); - void driveAnimationBackend(jdouble frameTimeMs); + void driveAnimationBackend(jlong frameTimeNanos); void drainPreallocateViewsQueue(); @@ -168,8 +168,6 @@ class FabricUIManagerBinding : public jni::HybridClass, bool enableFabricLogs_{false}; std::shared_ptr animationChoreographer_; - - void setAnimationBackendChoreographer(jni::alias_ref animationBackend); }; } // namespace facebook::react diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/JAnimationBackendChoreographer.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/JAnimationBackendChoreographer.cpp deleted file mode 100644 index 1f0272d11ae4..000000000000 --- a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/JAnimationBackendChoreographer.cpp +++ /dev/null @@ -1,23 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -#include "JAnimationBackendChoreographer.h" - -namespace facebook::react { - -void JAnimationBackendChoreographer::resume() const { - static const auto resumeMethod = - javaClassStatic()->getMethod("resume"); - resumeMethod(self()); -} - -void JAnimationBackendChoreographer::pause() const { - static const auto pauseMethod = javaClassStatic()->getMethod("pause"); - pauseMethod(self()); -} - -} // namespace facebook::react diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/JAnimationBackendChoreographer.h b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/JAnimationBackendChoreographer.h deleted file mode 100644 index 064dac173aca..000000000000 --- a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/JAnimationBackendChoreographer.h +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -#pragma once - -#include - -namespace facebook::react { - -/** - * JNI wrapper for the AnimationBackendChoreographer Kotlin class. - * This class provides the bridge between C++ and the Android AnimationBackendChoreographer - * which handles animation frame scheduling via ReactChoreographer. - */ -class JAnimationBackendChoreographer : public jni::JavaClass { - public: - static constexpr auto kJavaDescriptor = "Lcom/facebook/react/fabric/AnimationBackendChoreographer;"; - - /** - * Resumes animation frame callbacks. - * This method should be called when animations need to start or resume. - */ - void resume() const; - - /** - * Pauses animation frame callbacks. - * This method should be called when animations should be paused (e.g., when - * the app goes to background). - */ - void pause() const; -}; - -} // namespace facebook::react diff --git a/scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api b/scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api index a106856ceeed..c0f6046ba0ac 100644 --- a/scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api @@ -1245,9 +1245,9 @@ class facebook::react::AdditionAnimatedNode : public facebook::react::OperatorAn } class facebook::react::AndroidAnimationChoreographer : public facebook::react::AnimationChoreographer { - public AndroidAnimationChoreographer(jni::alias_ref jChoreographer); public virtual void pause() override; public virtual void resume() override; + public void onAnimationFrameIfActive(facebook::react::AnimationTimestamp timestamp) const; } class facebook::react::AndroidDrawerLayoutEventEmitter : public facebook::react::BaseViewEventEmitter { @@ -2780,12 +2780,6 @@ class facebook::react::IntersectionObserverState { public static facebook::react::IntersectionObserverState NotIntersecting(); } -class facebook::react::JAnimationBackendChoreographer : public facebook::jni::JavaClass { - public static constexpr auto kJavaDescriptor; - public void pause() const; - public void resume() const; -} - class facebook::react::JBindingsInstaller : public jni::HybridClass, public facebook::react::BindingsInstaller { public static constexpr auto kJavaDescriptor; public ~JBindingsInstaller(); diff --git a/scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api b/scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api index 955d8d0bf053..6025b1f3c3d2 100644 --- a/scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api @@ -1241,9 +1241,9 @@ class facebook::react::AdditionAnimatedNode : public facebook::react::OperatorAn } class facebook::react::AndroidAnimationChoreographer : public facebook::react::AnimationChoreographer { - public AndroidAnimationChoreographer(jni::alias_ref jChoreographer); public virtual void pause() override; public virtual void resume() override; + public void onAnimationFrameIfActive(facebook::react::AnimationTimestamp timestamp) const; } class facebook::react::AndroidDrawerLayoutEventEmitter : public facebook::react::BaseViewEventEmitter { @@ -2738,12 +2738,6 @@ class facebook::react::IntersectionObserverState { public static facebook::react::IntersectionObserverState NotIntersecting(); } -class facebook::react::JAnimationBackendChoreographer : public facebook::jni::JavaClass { - public static constexpr auto kJavaDescriptor; - public void pause() const; - public void resume() const; -} - class facebook::react::JBindingsInstaller : public jni::HybridClass, public facebook::react::BindingsInstaller { public static constexpr auto kJavaDescriptor; public ~JBindingsInstaller(); diff --git a/scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api b/scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api index 9c9545d3b33a..43646033d428 100644 --- a/scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api @@ -1245,9 +1245,9 @@ class facebook::react::AdditionAnimatedNode : public facebook::react::OperatorAn } class facebook::react::AndroidAnimationChoreographer : public facebook::react::AnimationChoreographer { - public AndroidAnimationChoreographer(jni::alias_ref jChoreographer); public virtual void pause() override; public virtual void resume() override; + public void onAnimationFrameIfActive(facebook::react::AnimationTimestamp timestamp) const; } class facebook::react::AndroidDrawerLayoutEventEmitter : public facebook::react::BaseViewEventEmitter { @@ -2777,12 +2777,6 @@ class facebook::react::IntersectionObserverState { public static facebook::react::IntersectionObserverState NotIntersecting(); } -class facebook::react::JAnimationBackendChoreographer : public facebook::jni::JavaClass { - public static constexpr auto kJavaDescriptor; - public void pause() const; - public void resume() const; -} - class facebook::react::JBindingsInstaller : public jni::HybridClass, public facebook::react::BindingsInstaller { public static constexpr auto kJavaDescriptor; public ~JBindingsInstaller();