unify overlapping logic among platform specific EventBeats (#47085)

Summary: Pull Request resolved: #47085 changelog: [internal] Move induce method from AppleEventBeat and AndroidEventBeat to its subclass EventBeat. # Goal of this stack: Centralise event beat logic into EventBeat class inside react-native-github. Subclasses should only override EventBeat::request and EventBeat::induce. Reviewed By: christophpurrer Differential Revision: D64302639 fbshipit-source-id: cdea5ee64dc25633ae59fdce5b1ce6aff8a97d68
facebook · Oct 17, 2024 · d3fa580 · d3fa580
1 parent 9fe253d
commit d3fa580
Show file tree

Hide file tree

Showing 7 changed files with 42 additions and 67 deletions.
diff --git a/packages/react-native/React/Fabric/AppleEventBeat.cpp b/packages/react-native/React/Fabric/AppleEventBeat.cpp
@@ -12,11 +12,11 @@
 namespace facebook::react {
 
 AppleEventBeat::AppleEventBeat(
+    std::shared_ptr<OwnerBox> ownerBox,
     RunLoopObserver::Unique uiRunLoopObserver,
     RuntimeExecutor runtimeExecutor)
-    : EventBeat({}),
-      uiRunLoopObserver_(std::move(uiRunLoopObserver)),
-      runtimeExecutor_(std::move(runtimeExecutor)) {
+    : EventBeat(std::move(ownerBox), std::move(runtimeExecutor)),
+      uiRunLoopObserver_(std::move(uiRunLoopObserver)) {
   uiRunLoopObserver_->setDelegate(this);
   uiRunLoopObserver_->enable();
 }
@@ -28,30 +28,4 @@ void AppleEventBeat::activityDidChange(
   induce();
 }
 
-void AppleEventBeat::induce() const {
-  if (!isRequested_ || isBeatCallbackScheduled_) {
-    return;
-  }
-
-  isRequested_ = false;
-
-  // Here we know that `this` object exists because the caller has a strong
-  // pointer to `owner`. To ensure the object will exist inside
-  // `runtimeExecutor_` callback, we need to copy the  pointer there.
-  auto weakOwner = uiRunLoopObserver_->getOwner();
-
-  isBeatCallbackScheduled_ = true;
-
-  runtimeExecutor_([this, weakOwner](jsi::Runtime& runtime) {
-    auto owner = weakOwner.lock();
-    if (!owner) {
-      return;
-    }
-
-    isBeatCallbackScheduled_ = false;
-    if (beatCallback_) {
-      beatCallback_(runtime);
-    }
-  });
-}
 } // namespace facebook::react
diff --git a/packages/react-native/React/Fabric/AppleEventBeat.h b/packages/react-native/React/Fabric/AppleEventBeat.h
@@ -21,11 +21,10 @@ namespace facebook::react {
 class AppleEventBeat : public EventBeat, public RunLoopObserver::Delegate {
  public:
   AppleEventBeat(
+      std::shared_ptr<OwnerBox> ownerBox,
       RunLoopObserver::Unique uiRunLoopObserver,
       RuntimeExecutor runtimeExecutor);
 
-  void induce() const override;
-
 #pragma mark - RunLoopObserver::Delegate
 
   void activityDidChange(
@@ -34,9 +33,6 @@ class AppleEventBeat : public EventBeat, public RunLoopObserver::Delegate {
 
  private:
   RunLoopObserver::Unique uiRunLoopObserver_;
-  RuntimeExecutor runtimeExecutor_;
-
-  mutable std::atomic<bool> isBeatCallbackScheduled_{false};
 };
 
 } // namespace facebook::react
diff --git a/packages/react-native/React/Fabric/RCTSurfacePresenter.mm b/packages/react-native/React/Fabric/RCTSurfacePresenter.mm
@@ -261,7 +261,7 @@ - (RCTScheduler *)_createScheduler
       [runtimeExecutor](std::shared_ptr<EventBeat::OwnerBox> ownerBox) -> std::unique_ptr<EventBeat> {
     auto runLoopObserver =
         std::make_unique<const MainRunLoopObserver>(RunLoopObserver::Activity::BeforeWaiting, ownerBox->owner);
-    return std::make_unique<AppleEventBeat>(std::move(runLoopObserver), runtimeExecutor);
+    return std::make_unique<AppleEventBeat>(std::move(ownerBox), std::move(runLoopObserver), runtimeExecutor);
   };
 
   RCTScheduler *scheduler = [[RCTScheduler alloc] initWithToolbox:toolbox];

diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AndroidEventBeat.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AndroidEventBeat.cpp
@@ -18,9 +18,8 @@ AndroidEventBeat::AndroidEventBeat(
     EventBeatManager* eventBeatManager,
     RuntimeExecutor runtimeExecutor,
     jni::global_ref<jobject> javaUIManager)
-    : EventBeat(std::move(ownerBox)),
+    : EventBeat(std::move(ownerBox), std::move(runtimeExecutor)),
       eventBeatManager_(eventBeatManager),
-      runtimeExecutor_(std::move(runtimeExecutor)),
       javaUIManager_(std::move(javaUIManager)) {
   eventBeatManager->addObserver(*this);
 }
@@ -30,28 +29,7 @@ AndroidEventBeat::~AndroidEventBeat() {
 }
 
 void AndroidEventBeat::tick() const {
-  if (!isRequested_ || isBeatCallbackScheduled_) {
-    return;
-  }
-
-  isRequested_ = false;
-  isBeatCallbackScheduled_ = true;
-
-  runtimeExecutor_([this, ownerBox = ownerBox_](jsi::Runtime& runtime) {
-    auto owner = ownerBox->owner.lock();
-    if (!owner) {
-      return;
-    }
-
-    isBeatCallbackScheduled_ = false;
-    if (beatCallback_) {
-      beatCallback_(runtime);
-    }
-  });
-}
-
-void AndroidEventBeat::induce() const {
-  tick();
+  induce();
 }
 
 void AndroidEventBeat::request() const {

diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AndroidEventBeat.h b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AndroidEventBeat.h
@@ -26,15 +26,11 @@ class AndroidEventBeat final : public EventBeat,
 
   void tick() const override;
 
-  void induce() const override;
-
   void request() const override;
 
  private:
   EventBeatManager* eventBeatManager_;
-  RuntimeExecutor runtimeExecutor_;
   jni::global_ref<jobject> javaUIManager_;
-  mutable std::atomic<bool> isBeatCallbackScheduled_{false};
 };
 
 } // namespace facebook::react
diff --git a/packages/react-native/ReactCommon/react/renderer/core/EventBeat.cpp b/packages/react-native/ReactCommon/react/renderer/core/EventBeat.cpp
@@ -11,8 +11,11 @@
 
 namespace facebook::react {
 
-EventBeat::EventBeat(std::shared_ptr<OwnerBox> ownerBox)
-    : ownerBox_(std::move(ownerBox)) {}
+EventBeat::EventBeat(
+    std::shared_ptr<OwnerBox> ownerBox,
+    RuntimeExecutor runtimeExecutor)
+    : ownerBox_(std::move(ownerBox)),
+      runtimeExecutor_(std::move(runtimeExecutor)) {}
 
 void EventBeat::request() const {
   isRequested_ = true;
@@ -22,4 +25,25 @@ void EventBeat::setBeatCallback(BeatCallback beatCallback) {
   beatCallback_ = std::move(beatCallback);
 }
 
+void EventBeat::induce() const {
+  if (!isRequested_ || isBeatCallbackScheduled_) {
+    return;
+  }
+
+  isRequested_ = false;
+  isBeatCallbackScheduled_ = true;
+
+  runtimeExecutor_([this, ownerBox = ownerBox_](jsi::Runtime& runtime) {
+    auto owner = ownerBox->owner.lock();
+    if (!owner) {
+      return;
+    }
+
+    isBeatCallbackScheduled_ = false;
+    if (beatCallback_) {
+      beatCallback_(runtime);
+    }
+  });
+}
+
 } // namespace facebook::react
diff --git a/packages/react-native/ReactCommon/react/renderer/core/EventBeat.h b/packages/react-native/ReactCommon/react/renderer/core/EventBeat.h
@@ -7,6 +7,7 @@
 
 #pragma once
 
+#include <ReactCommon/RuntimeExecutor.h>
 #include <atomic>
 #include <functional>
 #include <memory>
@@ -47,7 +48,9 @@ class EventBeat {
 
   using BeatCallback = std::function<void(jsi::Runtime& runtime)>;
 
-  explicit EventBeat(std::shared_ptr<OwnerBox> ownerBox);
+  explicit EventBeat(
+      std::shared_ptr<OwnerBox> ownerBox,
+      RuntimeExecutor runtimeExecutor);
 
   virtual ~EventBeat() = default;
 
@@ -72,11 +75,15 @@ class EventBeat {
    * Induces the next beat to happen as soon as possible.
    * Receiver might ignore the call if a beat was not requested.
    */
-  virtual void induce() const = 0;
+  void induce() const;
 
   BeatCallback beatCallback_;
   std::shared_ptr<OwnerBox> ownerBox_;
   mutable std::atomic<bool> isRequested_{false};
+
+ private:
+  RuntimeExecutor runtimeExecutor_;
+  mutable std::atomic<bool> isBeatCallbackScheduled_{false};
 };
 
 } // namespace facebook::react