From ce0c985d5907d1d419d81d02386b64c43caa9f4f Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Wed, 16 Oct 2024 03:05:18 -0700 Subject: [PATCH 1/5] refactor EventBeat owner Differential Revision: D64291288 --- .../react-native/React/Fabric/RCTSurfacePresenter.mm | 4 ++-- .../src/main/jni/react/fabric/AsyncEventBeat.cpp | 4 ++-- .../src/main/jni/react/fabric/AsyncEventBeat.h | 2 +- .../main/jni/react/fabric/FabricUIManagerBinding.cpp | 11 +++++++---- .../ReactCommon/react/renderer/core/EventBeat.cpp | 2 +- .../ReactCommon/react/renderer/core/EventBeat.h | 10 +++++----- .../react/renderer/core/EventDispatcher.cpp | 6 +++--- .../ReactCommon/react/renderer/core/EventDispatcher.h | 4 ++-- .../react/renderer/scheduler/Scheduler.cpp | 2 +- .../react/renderer/scheduler/SchedulerToolbox.h | 3 +-- 10 files changed, 25 insertions(+), 23 deletions(-) diff --git a/packages/react-native/React/Fabric/RCTSurfacePresenter.mm b/packages/react-native/React/Fabric/RCTSurfacePresenter.mm index 5d24d70de628..5cdaec9dd658 100644 --- a/packages/react-native/React/Fabric/RCTSurfacePresenter.mm +++ b/packages/react-native/React/Fabric/RCTSurfacePresenter.mm @@ -257,8 +257,8 @@ - (RCTScheduler *)_createScheduler toolbox.runtimeExecutor = runtimeExecutor; toolbox.bridgelessBindingsExecutor = _bridgelessBindingsExecutor; - toolbox.asynchronousEventBeatFactory = - [runtimeExecutor](const EventBeat::SharedOwnerBox &ownerBox) -> std::unique_ptr { + toolbox.eventBeatFactory = + [runtimeExecutor](std::shared_ptr ownerBox) -> std::unique_ptr { auto runLoopObserver = std::make_unique(RunLoopObserver::Activity::BeforeWaiting, ownerBox->owner); return std::make_unique(std::move(runLoopObserver), runtimeExecutor); diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AsyncEventBeat.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AsyncEventBeat.cpp index 4160e35da17d..99931acb0db4 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AsyncEventBeat.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AsyncEventBeat.cpp @@ -14,11 +14,11 @@ namespace facebook::react { AsyncEventBeat::AsyncEventBeat( - const EventBeat::SharedOwnerBox& ownerBox, + std::shared_ptr ownerBox, EventBeatManager* eventBeatManager, RuntimeExecutor runtimeExecutor, jni::global_ref javaUIManager) - : EventBeat(ownerBox), + : EventBeat(std::move(ownerBox)), eventBeatManager_(eventBeatManager), runtimeExecutor_(std::move(runtimeExecutor)), javaUIManager_(std::move(javaUIManager)) { diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AsyncEventBeat.h b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AsyncEventBeat.h index a45567a39489..878c8b98d48d 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AsyncEventBeat.h +++ b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AsyncEventBeat.h @@ -16,7 +16,7 @@ namespace facebook::react { class AsyncEventBeat final : public EventBeat, public EventBeatManagerObserver { public: AsyncEventBeat( - const EventBeat::SharedOwnerBox& ownerBox, + std::shared_ptr ownerBox, EventBeatManager* eventBeatManager, RuntimeExecutor runtimeExecutor, jni::global_ref javaUIManager); 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 76384785d33d..4c96f489e9a0 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 @@ -485,12 +485,15 @@ void FabricUIManagerBinding::installFabricUIManager( } } - EventBeat::Factory asynchronousBeatFactory = + EventBeat::Factory eventBeatFactory = [eventBeatManager, runtimeExecutor, globalJavaUiManager]( - const EventBeat::SharedOwnerBox& ownerBox) + std::shared_ptr ownerBox) -> std::unique_ptr { return std::make_unique( - ownerBox, eventBeatManager, runtimeExecutor, globalJavaUiManager); + std::move(ownerBox), + eventBeatManager, + runtimeExecutor, + globalJavaUiManager); }; contextContainer->insert("ReactNativeConfig", config); @@ -513,7 +516,7 @@ void FabricUIManagerBinding::installFabricUIManager( toolbox.bridgelessBindingsExecutor = std::nullopt; toolbox.runtimeExecutor = runtimeExecutor; - toolbox.asynchronousEventBeatFactory = asynchronousBeatFactory; + toolbox.eventBeatFactory = eventBeatFactory; animationDriver_ = std::make_shared( runtimeExecutor, contextContainer, this); diff --git a/packages/react-native/ReactCommon/react/renderer/core/EventBeat.cpp b/packages/react-native/ReactCommon/react/renderer/core/EventBeat.cpp index 17262807c0fe..4306ef8cd03f 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/EventBeat.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/EventBeat.cpp @@ -11,7 +11,7 @@ namespace facebook::react { -EventBeat::EventBeat(SharedOwnerBox ownerBox) +EventBeat::EventBeat(std::shared_ptr ownerBox) : ownerBox_(std::move(ownerBox)) {} void EventBeat::request() const { diff --git a/packages/react-native/ReactCommon/react/renderer/core/EventBeat.h b/packages/react-native/ReactCommon/react/renderer/core/EventBeat.h index 2a13a4af44e1..12c4471d1cd7 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/EventBeat.h +++ b/packages/react-native/ReactCommon/react/renderer/core/EventBeat.h @@ -38,17 +38,17 @@ class EventBeat { * creation of some other object that owns an `EventBeat`. */ using Owner = std::weak_ptr; + struct OwnerBox { Owner owner; }; - using SharedOwnerBox = std::shared_ptr; - using Factory = - std::function(const SharedOwnerBox& ownerBox)>; + using Factory = std::function( + std::shared_ptr ownerBox)>; using BeatCallback = std::function; - EventBeat(SharedOwnerBox ownerBox); + explicit EventBeat(std::shared_ptr ownerBox); virtual ~EventBeat() = default; @@ -73,7 +73,7 @@ class EventBeat { virtual void induce() const; BeatCallback beatCallback_; - SharedOwnerBox ownerBox_; + std::shared_ptr ownerBox_; mutable std::atomic isRequested_{false}; }; diff --git a/packages/react-native/ReactCommon/react/renderer/core/EventDispatcher.cpp b/packages/react-native/ReactCommon/react/renderer/core/EventDispatcher.cpp index daf4edc10777..31032da20c3a 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/EventDispatcher.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/EventDispatcher.cpp @@ -17,14 +17,14 @@ namespace facebook::react { EventDispatcher::EventDispatcher( const EventQueueProcessor& eventProcessor, - const EventBeat::Factory& asynchronousEventBeatFactory, - const EventBeat::SharedOwnerBox& ownerBox, + const EventBeat::Factory& eventBeatFactory, + std::shared_ptr ownerBox, RuntimeScheduler& runtimeScheduler, StatePipe statePipe, std::weak_ptr eventLogger) : eventQueue_(EventQueue( eventProcessor, - asynchronousEventBeatFactory(ownerBox), + eventBeatFactory(std::move(ownerBox)), runtimeScheduler)), statePipe_(std::move(statePipe)), eventLogger_(std::move(eventLogger)) {} diff --git a/packages/react-native/ReactCommon/react/renderer/core/EventDispatcher.h b/packages/react-native/ReactCommon/react/renderer/core/EventDispatcher.h index 36e4d03c6d9e..2099daf37525 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/EventDispatcher.h +++ b/packages/react-native/ReactCommon/react/renderer/core/EventDispatcher.h @@ -32,8 +32,8 @@ class EventDispatcher { EventDispatcher( const EventQueueProcessor& eventProcessor, - const EventBeat::Factory& asynchronousEventBeatFactory, - const EventBeat::SharedOwnerBox& ownerBox, + const EventBeat::Factory& eventBeatFactory, + std::shared_ptr ownerBox, RuntimeScheduler& runtimeScheduler, StatePipe statePipe, std::weak_ptr eventLogger); diff --git a/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp b/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp index 4b69f2f2b049..6a74c3c99f58 100644 --- a/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp +++ b/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp @@ -98,7 +98,7 @@ Scheduler::Scheduler( eventDispatcher_->emplace( EventQueueProcessor( eventPipe, eventPipeConclusion, statePipe, eventPerformanceLogger_), - schedulerToolbox.asynchronousEventBeatFactory, + schedulerToolbox.eventBeatFactory, eventOwnerBox, *runtimeScheduler, statePipe, diff --git a/packages/react-native/ReactCommon/react/renderer/scheduler/SchedulerToolbox.h b/packages/react-native/ReactCommon/react/renderer/scheduler/SchedulerToolbox.h index 86a38aba9342..c68ad6eee3a7 100644 --- a/packages/react-native/ReactCommon/react/renderer/scheduler/SchedulerToolbox.h +++ b/packages/react-native/ReactCommon/react/renderer/scheduler/SchedulerToolbox.h @@ -49,11 +49,10 @@ struct SchedulerToolbox final { RuntimeExecutor runtimeExecutor; /* - * Asynchronous & synchronous event beats. * Represent connections with the platform-specific run loops and general * purpose background queue. */ - EventBeat::Factory asynchronousEventBeatFactory; + EventBeat::Factory eventBeatFactory; /* * A list of `UIManagerCommitHook`s that should be registered in `UIManager`. From 0ff8e442964777aa95f65a2b5d7f6a8b5217cbed Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Wed, 16 Oct 2024 03:05:19 -0700 Subject: [PATCH 2/5] copy by value in RunLoopObserver ctor Differential Revision: D64291515 --- .../React/Fabric/Utils/PlatformRunLoopObserver.h | 9 ++++++--- .../React/Fabric/Utils/PlatformRunLoopObserver.mm | 7 ++----- .../ReactCommon/react/utils/RunLoopObserver.cpp | 6 ++---- .../ReactCommon/react/utils/RunLoopObserver.h | 2 +- 4 files changed, 11 insertions(+), 13 deletions(-) diff --git a/packages/react-native/React/Fabric/Utils/PlatformRunLoopObserver.h b/packages/react-native/React/Fabric/Utils/PlatformRunLoopObserver.h index d8e456619a28..4f097aaac506 100644 --- a/packages/react-native/React/Fabric/Utils/PlatformRunLoopObserver.h +++ b/packages/react-native/React/Fabric/Utils/PlatformRunLoopObserver.h @@ -22,7 +22,7 @@ class PlatformRunLoopObserver : public RunLoopObserver { public: PlatformRunLoopObserver( RunLoopObserver::Activity activities, - const RunLoopObserver::WeakOwner& owner, + RunLoopObserver::WeakOwner owner, CFRunLoopRef runLoop); ~PlatformRunLoopObserver(); @@ -45,8 +45,11 @@ class MainRunLoopObserver final : public PlatformRunLoopObserver { public: MainRunLoopObserver( RunLoopObserver::Activity activities, - const RunLoopObserver::WeakOwner& owner) - : PlatformRunLoopObserver(activities, owner, CFRunLoopGetMain()) {} + RunLoopObserver::WeakOwner owner) + : PlatformRunLoopObserver( + activities, + std::move(owner), + CFRunLoopGetMain()) {} }; } // namespace facebook::react diff --git a/packages/react-native/React/Fabric/Utils/PlatformRunLoopObserver.mm b/packages/react-native/React/Fabric/Utils/PlatformRunLoopObserver.mm index d243c8f912ad..65f48b48a4f6 100644 --- a/packages/react-native/React/Fabric/Utils/PlatformRunLoopObserver.mm +++ b/packages/react-native/React/Fabric/Utils/PlatformRunLoopObserver.mm @@ -45,13 +45,10 @@ static CFRunLoopActivity toCFRunLoopActivity(RunLoopObserver::Activity activity) PlatformRunLoopObserver::PlatformRunLoopObserver( RunLoopObserver::Activity activities, - const RunLoopObserver::WeakOwner &owner, + RunLoopObserver::WeakOwner owner, CFRunLoopRef runLoop) : RunLoopObserver(activities, owner), runLoop_(runLoop) { - // A value (not a reference) to be captured by the block. - auto weakOwner = owner; - // The documentation for `CFRunLoop` family API states that all of the methods are thread-safe. // See "Thread Safety and Run Loop Objects" section of the "Threading Programming Guide" for more details. mainRunLoopObserver_ = CFRunLoopObserverCreateWithHandler( @@ -60,7 +57,7 @@ static CFRunLoopActivity toCFRunLoopActivity(RunLoopObserver::Activity activity) true /* repeats */, 0 /* order */, ^(CFRunLoopObserverRef observer, CFRunLoopActivity activity) { - auto strongOwner = weakOwner.lock(); + auto strongOwner = owner.lock(); if (!strongOwner) { return; } diff --git a/packages/react-native/ReactCommon/react/utils/RunLoopObserver.cpp b/packages/react-native/ReactCommon/react/utils/RunLoopObserver.cpp index 74707e8d91a6..b89955025625 100644 --- a/packages/react-native/ReactCommon/react/utils/RunLoopObserver.cpp +++ b/packages/react-native/ReactCommon/react/utils/RunLoopObserver.cpp @@ -11,10 +11,8 @@ namespace facebook::react { -RunLoopObserver::RunLoopObserver( - Activity activities, - const WeakOwner& owner) noexcept - : activities_(activities), owner_(owner) {} +RunLoopObserver::RunLoopObserver(Activity activities, WeakOwner owner) noexcept + : activities_(activities), owner_(std::move(owner)) {} void RunLoopObserver::setDelegate(const Delegate* delegate) const noexcept { // We need these constraints to ensure basic thread-safety. diff --git a/packages/react-native/ReactCommon/react/utils/RunLoopObserver.h b/packages/react-native/ReactCommon/react/utils/RunLoopObserver.h index 43a7918aba22..0f6001e1563b 100644 --- a/packages/react-native/ReactCommon/react/utils/RunLoopObserver.h +++ b/packages/react-native/ReactCommon/react/utils/RunLoopObserver.h @@ -76,7 +76,7 @@ class RunLoopObserver { /* * Constructs a run loop observer. */ - RunLoopObserver(Activity activities, const WeakOwner& owner) noexcept; + RunLoopObserver(Activity activities, WeakOwner owner) noexcept; virtual ~RunLoopObserver() noexcept = default; /* From 33e7162db5d5a7268043eae42cf2215c9a44f7e9 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Wed, 16 Oct 2024 03:05:19 -0700 Subject: [PATCH 3/5] pass eventBeat directly into EventDispatcher Differential Revision: D64291401 --- .../ReactCommon/react/renderer/core/EventDispatcher.cpp | 9 +++------ .../ReactCommon/react/renderer/core/EventDispatcher.h | 3 +-- .../ReactCommon/react/renderer/scheduler/Scheduler.cpp | 5 +++-- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/core/EventDispatcher.cpp b/packages/react-native/ReactCommon/react/renderer/core/EventDispatcher.cpp index 31032da20c3a..10d6d0b46590 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/EventDispatcher.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/EventDispatcher.cpp @@ -17,15 +17,12 @@ namespace facebook::react { EventDispatcher::EventDispatcher( const EventQueueProcessor& eventProcessor, - const EventBeat::Factory& eventBeatFactory, - std::shared_ptr ownerBox, + std::unique_ptr eventBeat, RuntimeScheduler& runtimeScheduler, StatePipe statePipe, std::weak_ptr eventLogger) - : eventQueue_(EventQueue( - eventProcessor, - eventBeatFactory(std::move(ownerBox)), - runtimeScheduler)), + : eventQueue_( + EventQueue(eventProcessor, std::move(eventBeat), runtimeScheduler)), statePipe_(std::move(statePipe)), eventLogger_(std::move(eventLogger)) {} diff --git a/packages/react-native/ReactCommon/react/renderer/core/EventDispatcher.h b/packages/react-native/ReactCommon/react/renderer/core/EventDispatcher.h index 2099daf37525..79eab45e1200 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/EventDispatcher.h +++ b/packages/react-native/ReactCommon/react/renderer/core/EventDispatcher.h @@ -32,8 +32,7 @@ class EventDispatcher { EventDispatcher( const EventQueueProcessor& eventProcessor, - const EventBeat::Factory& eventBeatFactory, - std::shared_ptr ownerBox, + std::unique_ptr eventBeat, RuntimeScheduler& runtimeScheduler, StatePipe statePipe, std::weak_ptr eventLogger); diff --git a/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp b/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp index 6a74c3c99f58..a80f53afcc14 100644 --- a/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp +++ b/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp @@ -93,13 +93,14 @@ Scheduler::Scheduler( uiManager->updateState(stateUpdate); }; + auto eventBeat = schedulerToolbox.eventBeatFactory(std::move(eventOwnerBox)); + // Creating an `EventDispatcher` instance inside the already allocated // container (inside the optional). eventDispatcher_->emplace( EventQueueProcessor( eventPipe, eventPipeConclusion, statePipe, eventPerformanceLogger_), - schedulerToolbox.eventBeatFactory, - eventOwnerBox, + std::move(eventBeat), *runtimeScheduler, statePipe, eventPerformanceLogger_); From cea27b6092c5fdfe3dd6cae0b78589c1312c52a5 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Wed, 16 Oct 2024 03:05:19 -0700 Subject: [PATCH 4/5] rename AsyncEventBeat to AndroidEventBeat Differential Revision: D64301829 --- .../{AsyncEventBeat.cpp => AndroidEventBeat.cpp} | 12 ++++++------ .../fabric/{AsyncEventBeat.h => AndroidEventBeat.h} | 7 ++++--- .../main/jni/react/fabric/FabricUIManagerBinding.cpp | 4 ++-- 3 files changed, 12 insertions(+), 11 deletions(-) rename packages/react-native/ReactAndroid/src/main/jni/react/fabric/{AsyncEventBeat.cpp => AndroidEventBeat.cpp} (87%) rename packages/react-native/ReactAndroid/src/main/jni/react/fabric/{AsyncEventBeat.h => AndroidEventBeat.h} (83%) diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AsyncEventBeat.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AndroidEventBeat.cpp similarity index 87% rename from packages/react-native/ReactAndroid/src/main/jni/react/fabric/AsyncEventBeat.cpp rename to packages/react-native/ReactAndroid/src/main/jni/react/fabric/AndroidEventBeat.cpp index 99931acb0db4..33d484857f24 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AsyncEventBeat.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AndroidEventBeat.cpp @@ -9,11 +9,11 @@ #include #include -#include "AsyncEventBeat.h" +#include "AndroidEventBeat.h" namespace facebook::react { -AsyncEventBeat::AsyncEventBeat( +AndroidEventBeat::AndroidEventBeat( std::shared_ptr ownerBox, EventBeatManager* eventBeatManager, RuntimeExecutor runtimeExecutor, @@ -25,11 +25,11 @@ AsyncEventBeat::AsyncEventBeat( eventBeatManager->addObserver(*this); } -AsyncEventBeat::~AsyncEventBeat() { +AndroidEventBeat::~AndroidEventBeat() { eventBeatManager_->removeObserver(*this); } -void AsyncEventBeat::tick() const { +void AndroidEventBeat::tick() const { if (!isRequested_ || isBeatCallbackScheduled_) { return; } @@ -50,11 +50,11 @@ void AsyncEventBeat::tick() const { }); } -void AsyncEventBeat::induce() const { +void AndroidEventBeat::induce() const { tick(); } -void AsyncEventBeat::request() const { +void AndroidEventBeat::request() const { bool alreadyRequested = isRequested_; EventBeat::request(); if (!alreadyRequested) { diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AsyncEventBeat.h b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AndroidEventBeat.h similarity index 83% rename from packages/react-native/ReactAndroid/src/main/jni/react/fabric/AsyncEventBeat.h rename to packages/react-native/ReactAndroid/src/main/jni/react/fabric/AndroidEventBeat.h index 878c8b98d48d..a80875d5e222 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AsyncEventBeat.h +++ b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AndroidEventBeat.h @@ -13,15 +13,16 @@ namespace facebook::react { -class AsyncEventBeat final : public EventBeat, public EventBeatManagerObserver { +class AndroidEventBeat final : public EventBeat, + public EventBeatManagerObserver { public: - AsyncEventBeat( + AndroidEventBeat( std::shared_ptr ownerBox, EventBeatManager* eventBeatManager, RuntimeExecutor runtimeExecutor, jni::global_ref javaUIManager); - ~AsyncEventBeat() override; + ~AndroidEventBeat() override; void tick() const override; 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 4c96f489e9a0..d0762aaa1b51 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 @@ -7,7 +7,7 @@ #include "FabricUIManagerBinding.h" -#include "AsyncEventBeat.h" +#include "AndroidEventBeat.h" #include "ComponentFactory.h" #include "EventBeatManager.h" #include "EventEmitterWrapper.h" @@ -489,7 +489,7 @@ void FabricUIManagerBinding::installFabricUIManager( [eventBeatManager, runtimeExecutor, globalJavaUiManager]( std::shared_ptr ownerBox) -> std::unique_ptr { - return std::make_unique( + return std::make_unique( std::move(ownerBox), eventBeatManager, runtimeExecutor, From db5bf9dd5bd97f06c1339c3ffae34f94010fd108 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Wed, 16 Oct 2024 06:14:31 -0700 Subject: [PATCH 5/5] delete EventBeat's copy ctor and remove default implementation for induce Summary: changelog: [internal] EventBeat should never be copied. Make that explicit. EventBeat::induce must never be empty, remove the default implementation. # Goal of this stack: Centralise event beat logic into EventBeat class inside react-native-github. Subclasses should only override EventBeat::request and EventBeat::induce. Differential Revision: D64291845 --- .../ReactCommon/react/renderer/core/EventBeat.cpp | 4 ---- .../ReactCommon/react/renderer/core/EventBeat.h | 8 +++++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/core/EventBeat.cpp b/packages/react-native/ReactCommon/react/renderer/core/EventBeat.cpp index 4306ef8cd03f..6b920f06dc2f 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/EventBeat.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/EventBeat.cpp @@ -18,10 +18,6 @@ void EventBeat::request() const { isRequested_ = true; } -void EventBeat::induce() const { - // Default implementation does nothing. -} - void EventBeat::setBeatCallback(BeatCallback beatCallback) { beatCallback_ = std::move(beatCallback); } diff --git a/packages/react-native/ReactCommon/react/renderer/core/EventBeat.h b/packages/react-native/ReactCommon/react/renderer/core/EventBeat.h index 12c4471d1cd7..285ec98ffd8e 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/EventBeat.h +++ b/packages/react-native/ReactCommon/react/renderer/core/EventBeat.h @@ -38,7 +38,6 @@ class EventBeat { * creation of some other object that owns an `EventBeat`. */ using Owner = std::weak_ptr; - struct OwnerBox { Owner owner; }; @@ -52,6 +51,10 @@ class EventBeat { virtual ~EventBeat() = default; + // not copyable + EventBeat(const EventBeat& other) = delete; + EventBeat& operator=(const EventBeat& other) = delete; + /* * Communicates to the Beat that a consumer is waiting for the coming beat. * A consumer must request coming beat after the previous beat happened @@ -60,7 +63,6 @@ class EventBeat { virtual void request() const; /* - * Sets the beat callback function. * The callback is must be called on the proper thread. */ void setBeatCallback(BeatCallback beatCallback); @@ -70,7 +72,7 @@ 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; + virtual void induce() const = 0; BeatCallback beatCallback_; std::shared_ptr ownerBox_;