Fix EXC_BAD_ACCESS introduced in #54211 (#54319)

Summary:
Pull Request resolved: #54319

## Changelog:

[Internal] [Changed] - Fix EXC_BAD_ACCESS introduced in #54211

Reviewed By: sammy-SC

Differential Revision: D85779978

fbshipit-source-id: 7b240e1484fdcc57283325febf75faedaba585cd

Author: zeyap

packages/react-native/ReactCommon/react/renderer/animated/NativeAnimatedNodesManager.cpp

@@ -74,11 +74,13 @@ NativeAnimatedNodesManager::NativeAnimatedNodesManager(
     DirectManipulationCallback&& directManipulationCallback,
     FabricCommitCallback&& fabricCommitCallback,
     StartOnRenderCallback&& startOnRenderCallback,
-    StopOnRenderCallback&& stopOnRenderCallback) noexcept
+    StopOnRenderCallback&& stopOnRenderCallback,
+    FrameRateListenerCallback&& frameRateListenerCallback) noexcept
     : directManipulationCallback_(std::move(directManipulationCallback)),
       fabricCommitCallback_(std::move(fabricCommitCallback)),
       startOnRenderCallback_(std::move(startOnRenderCallback)),
-      stopOnRenderCallback_(std::move(stopOnRenderCallback)) {
+      stopOnRenderCallback_(std::move(stopOnRenderCallback)),
+      frameRateListenerCallback_(std::move(frameRateListenerCallback)) {
   if (!fabricCommitCallback_) {
     LOG(WARNING)
         << "C++ Animated was setup without commit callback. This may lead to issue where buttons are not tappable when animation is driven by onScroll event.";
@@ -541,7 +543,7 @@ void NativeAnimatedNodesManager::startRenderCallbackIfNeeded(bool isAsync) {
   }
 
   if (startOnRenderCallback_) {
-    startOnRenderCallback_(isAsync);
+    startOnRenderCallback_([this]() { onRender(); }, isAsync);
   }
 }
 
@@ -563,6 +565,10 @@ void NativeAnimatedNodesManager::stopRenderCallbackIfNeeded(
     if (stopOnRenderCallback_) {
       stopOnRenderCallback_(isAsync);
     }
+
+    if (frameRateListenerCallback_) {
+      frameRateListenerCallback_(false);
+    }
   }
 }
 
@@ -1030,6 +1036,10 @@ void NativeAnimatedNodesManager::onRender() {
       "numActiveAnimations",
       activeAnimations_.size());
 
+  if (frameRateListenerCallback_) {
+    frameRateListenerCallback_(true);
+  }
+
   isOnRenderThread_ = true;
 
   {

packages/react-native/ReactCommon/react/renderer/animated/NativeAnimatedNodesManager.h

@@ -55,14 +55,16 @@ class NativeAnimatedNodesManager {
  public:
   using DirectManipulationCallback = std::function<void(Tag, const folly::dynamic &)>;
   using FabricCommitCallback = std::function<void(std::unordered_map<Tag, folly::dynamic> &)>;
-  using StartOnRenderCallback = std::function<void(bool isAsync)>;
+  using StartOnRenderCallback = std::function<void(std::function<void()> &&, bool isAsync)>;
   using StopOnRenderCallback = std::function<void(bool isAsync)>;
+  using FrameRateListenerCallback = std::function<void(bool /* shouldEnableListener */)>;
 
   explicit NativeAnimatedNodesManager(
       DirectManipulationCallback &&directManipulationCallback,
       FabricCommitCallback &&fabricCommitCallback,
       StartOnRenderCallback &&startOnRenderCallback = nullptr,
-      StopOnRenderCallback &&stopOnRenderCallback = nullptr) noexcept;
+      StopOnRenderCallback &&stopOnRenderCallback = nullptr,
+      FrameRateListenerCallback &&frameRateListenerCallback = nullptr) noexcept;
 
   explicit NativeAnimatedNodesManager(std::shared_ptr<UIManagerAnimationBackend> animationBackend) noexcept;
 
@@ -237,6 +239,7 @@ class NativeAnimatedNodesManager {
   std::atomic_bool isRenderCallbackStarted_{false};
   const StartOnRenderCallback startOnRenderCallback_;
   const StopOnRenderCallback stopOnRenderCallback_;
+  const FrameRateListenerCallback frameRateListenerCallback_;
 
   std::shared_ptr<EventEmitterListener> eventEmitterListener_{nullptr};
 

packages/react-native/ReactCommon/react/renderer/animated/NativeAnimatedNodesManagerProvider.cpp

@@ -19,41 +19,26 @@
 namespace facebook::react {
 
 UIManagerNativeAnimatedDelegateImpl::UIManagerNativeAnimatedDelegateImpl(
-    NativeAnimatedNodesManagerProvider::FrameRateListenerCallback
-        frameRateListenerCallback)
-    : frameRateListenerCallback_(std::move(frameRateListenerCallback)) {}
+    std::weak_ptr<NativeAnimatedNodesManager> manager)
+    : nativeAnimatedNodesManager_(manager) {}
 
 void UIManagerNativeAnimatedDelegateImpl::runAnimationFrame() {
   if (auto nativeAnimatedNodesManagerStrong =
           nativeAnimatedNodesManager_.lock()) {
-    if (frameRateListenerCallback_) {
-      frameRateListenerCallback_(true);
-    }
     nativeAnimatedNodesManagerStrong->onRender();
   }
 }
 
 NativeAnimatedNodesManagerProvider::NativeAnimatedNodesManagerProvider(
-    StartOnRenderCallback startOnRenderCallback,
-    StopOnRenderCallback stopOnRenderCallback,
-    FrameRateListenerCallback frameRateListenerCallback)
+    NativeAnimatedNodesManager::StartOnRenderCallback startOnRenderCallback,
+    NativeAnimatedNodesManager::StopOnRenderCallback stopOnRenderCallback,
+    NativeAnimatedNodesManager::FrameRateListenerCallback
+        frameRateListenerCallback)
     : eventEmitterListenerContainer_(
           std::make_shared<EventEmitterListenerContainer>()),
-      frameRateListenerCallback_(std::move(frameRateListenerCallback)),
-      startOnRenderCallback_(std::move(startOnRenderCallback)) {
-  if (frameRateListenerCallback_) {
-    stopOnRenderCallback_ = [this, stopOnRenderCallback](bool isAsync) {
-      if (stopOnRenderCallback) {
-        stopOnRenderCallback(isAsync);
-      }
-      if (frameRateListenerCallback_) {
-        frameRateListenerCallback_(false);
-      }
-    };
-  } else {
-    stopOnRenderCallback_ = std::move(stopOnRenderCallback);
-  }
-}
+      startOnRenderCallback_(std::move(startOnRenderCallback)),
+      stopOnRenderCallback_(std::move(stopOnRenderCallback)),
+      frameRateListenerCallback_(std::move(frameRateListenerCallback)) {}
 
 std::shared_ptr<NativeAnimatedNodesManager>
 NativeAnimatedNodesManagerProvider::getOrCreate(
@@ -85,10 +70,6 @@ NativeAnimatedNodesManagerProvider::getOrCreate(
           uiManager->synchronouslyUpdateViewOnUIThread(viewTag, props);
         };
 
-    nativeAnimatedDelegate_ =
-        std::make_shared<UIManagerNativeAnimatedDelegateImpl>(
-            frameRateListenerCallback_);
-
     if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
 #ifdef RN_USE_ANIMATION_BACKEND
       // TODO: this should be initialized outside of animated, but for now it
@@ -106,31 +87,14 @@ NativeAnimatedNodesManagerProvider::getOrCreate(
 
       uiManager->unstable_setAnimationBackend(animationBackend_);
     } else {
-      auto startOnRenderCallback =
-          [this, startOnRenderCallbackFn = std::move(startOnRenderCallback_)](
-              bool isAsync) {
-            if (startOnRenderCallbackFn) {
-              startOnRenderCallbackFn(
-                  [this]() {
-                    if (nativeAnimatedDelegate_) {
-                      nativeAnimatedDelegate_->runAnimationFrame();
-                    }
-                  },
-                  isAsync);
-            }
-          };
       nativeAnimatedNodesManager_ =
           std::make_shared<NativeAnimatedNodesManager>(
               std::move(directManipulationCallback),
               std::move(fabricCommitCallback),
-              std::move(startOnRenderCallback),
+              std::move(startOnRenderCallback_),
               std::move(stopOnRenderCallback_));
     }
 
-    std::static_pointer_cast<UIManagerNativeAnimatedDelegateImpl>(
-        nativeAnimatedDelegate_)
-        ->setNativeAnimatedNodesManager(nativeAnimatedNodesManager_);
-
     addEventEmitterListener(
         nativeAnimatedNodesManager_->getEventEmitterListener());
 
@@ -153,6 +117,10 @@ NativeAnimatedNodesManagerProvider::getOrCreate(
               return false;
             }));
 
+    nativeAnimatedDelegate_ =
+        std::make_shared<UIManagerNativeAnimatedDelegateImpl>(
+            nativeAnimatedNodesManager_);
+
     uiManager->setNativeAnimatedDelegate(nativeAnimatedDelegate_);
 
     // TODO: remove force casting.

packages/react-native/ReactCommon/react/renderer/animated/NativeAnimatedNodesManagerProvider.h

@@ -17,16 +17,10 @@ class AnimatedMountingOverrideDelegate;
 
 class NativeAnimatedNodesManagerProvider {
  public:
-  using FrameRateListenerCallback = std::function<void(bool /* shouldEnableListener */)>;
-  // when isAsync is true, it means StartOnRenderCallback is invoked from js
-  // thread, otherwise from main thread
-  using StartOnRenderCallback = std::function<void(std::function<void()> &&, bool /* isAsync */)>;
-  using StopOnRenderCallback = NativeAnimatedNodesManager::StopOnRenderCallback;
-
   NativeAnimatedNodesManagerProvider(
-      StartOnRenderCallback startOnRenderCallback = nullptr,
-      StopOnRenderCallback stopOnRenderCallback = nullptr,
-      FrameRateListenerCallback frameRateListenerCallback = nullptr);
+      NativeAnimatedNodesManager::StartOnRenderCallback startOnRenderCallback = nullptr,
+      NativeAnimatedNodesManager::StopOnRenderCallback stopOnRenderCallback = nullptr,
+      NativeAnimatedNodesManager::FrameRateListenerCallback frameRateListenerCallback = nullptr);
 
   std::shared_ptr<NativeAnimatedNodesManager> getOrCreate(
       jsi::Runtime &runtime,
@@ -47,29 +41,21 @@ class NativeAnimatedNodesManagerProvider {
   std::shared_ptr<UIManagerNativeAnimatedDelegate> nativeAnimatedDelegate_;
   std::shared_ptr<AnimatedMountingOverrideDelegate> animatedMountingOverrideDelegate_;
 
-  FrameRateListenerCallback frameRateListenerCallback_;
-
-  StartOnRenderCallback startOnRenderCallback_;
-  StopOnRenderCallback stopOnRenderCallback_;
+  NativeAnimatedNodesManager::StartOnRenderCallback startOnRenderCallback_;
+  NativeAnimatedNodesManager::StopOnRenderCallback stopOnRenderCallback_;
+  NativeAnimatedNodesManager::FrameRateListenerCallback frameRateListenerCallback_;
 
   std::unique_ptr<MergedValueDispatcher> mergedValueDispatcher_;
 };
 
 class UIManagerNativeAnimatedDelegateImpl : public UIManagerNativeAnimatedDelegate {
  public:
-  explicit UIManagerNativeAnimatedDelegateImpl(
-      NativeAnimatedNodesManagerProvider::FrameRateListenerCallback frameRateListenerCallback);
+  explicit UIManagerNativeAnimatedDelegateImpl(std::weak_ptr<NativeAnimatedNodesManager> manager);
 
   void runAnimationFrame() override;
 
-  void setNativeAnimatedNodesManager(std::weak_ptr<NativeAnimatedNodesManager> manager)
-  {
-    nativeAnimatedNodesManager_ = manager;
-  }
-
  private:
   std::weak_ptr<NativeAnimatedNodesManager> nativeAnimatedNodesManager_;
-  NativeAnimatedNodesManagerProvider::FrameRateListenerCallback frameRateListenerCallback_;
 };
 
 } // namespace facebook::react