From 3afb5b82419f2ff2ce6581a3e428003816be5aaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Fri, 26 May 2023 09:06:00 -0700 Subject: [PATCH] Remove incorrect const constraint from UIManager commit hooks (#37588) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/37588 The `const` modified in UIManagerCommitHook prevents us from mutating the commit hook itself (which doesn't make any sense as commit hooks might want to update their internal state as response to new commits). This removes the constraint so we can remove the `const` modifier from `MutationObserverManager` in a future diff. Changelog: [internal] Reviewed By: sammy-SC Differential Revision: D46149085 fbshipit-source-id: d3bf24f1125ad3878ca36a6ceb9dd509c3cf7b1e --- .../ReactCommon/react/renderer/scheduler/Scheduler.cpp | 4 ++-- .../ReactCommon/react/renderer/scheduler/Scheduler.h | 2 +- .../react/renderer/scheduler/SchedulerToolbox.h | 2 +- .../react/renderer/timeline/TimelineController.cpp | 6 +++--- .../react/renderer/timeline/TimelineController.h | 9 +++------ .../ReactCommon/react/renderer/uimanager/UIManager.cpp | 8 +++----- .../ReactCommon/react/renderer/uimanager/UIManager.h | 6 +++--- .../react/renderer/uimanager/UIManagerCommitHook.h | 7 +++---- 8 files changed, 19 insertions(+), 25 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp b/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp index 661b0f88003..1d812fcccf8 100644 --- a/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp +++ b/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp @@ -114,7 +114,7 @@ Scheduler::Scheduler( commitHooks_ = schedulerToolbox.commitHooks; uiManager_ = uiManager; - for (auto const &commitHook : commitHooks_) { + for (auto &commitHook : commitHooks_) { uiManager->registerCommitHook(*commitHook); } @@ -147,7 +147,7 @@ Scheduler::~Scheduler() { LOG(WARNING) << "Scheduler::~Scheduler() was called (address: " << this << ")."; - for (auto const &commitHook : commitHooks_) { + for (auto &commitHook : commitHooks_) { uiManager_->unregisterCommitHook(*commitHook); } diff --git a/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.h b/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.h index 8b4c0b89678..53a14e75513 100644 --- a/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.h +++ b/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.h @@ -124,7 +124,7 @@ class Scheduler final : public UIManagerDelegate { std::shared_ptr uiManager_; std::shared_ptr reactNativeConfig_; - std::vector> commitHooks_; + std::vector> commitHooks_; /* * At some point, we have to have an owning shared pointer to something that diff --git a/packages/react-native/ReactCommon/react/renderer/scheduler/SchedulerToolbox.h b/packages/react-native/ReactCommon/react/renderer/scheduler/SchedulerToolbox.h index 844da3aff5b..2ada81212d1 100644 --- a/packages/react-native/ReactCommon/react/renderer/scheduler/SchedulerToolbox.h +++ b/packages/react-native/ReactCommon/react/renderer/scheduler/SchedulerToolbox.h @@ -75,7 +75,7 @@ struct SchedulerToolbox final { /* * A list of `UIManagerCommitHook`s that should be registered in `UIManager`. */ - std::vector> commitHooks; + std::vector> commitHooks; }; } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/renderer/timeline/TimelineController.cpp b/packages/react-native/ReactCommon/react/renderer/timeline/TimelineController.cpp index 68749cc6c2e..dd57e7b9caa 100644 --- a/packages/react-native/ReactCommon/react/renderer/timeline/TimelineController.cpp +++ b/packages/react-native/ReactCommon/react/renderer/timeline/TimelineController.cpp @@ -42,19 +42,19 @@ void TimelineController::disable(TimelineHandler &&handler) const { } void TimelineController::commitHookWasRegistered( - UIManager const &uiManager) const noexcept { + UIManager const &uiManager) noexcept { uiManager_ = &uiManager; } void TimelineController::commitHookWasUnregistered( - UIManager const & /*uiManager*/) const noexcept { + UIManager const & /*uiManager*/) noexcept { uiManager_ = nullptr; } RootShadowNode::Unshared TimelineController::shadowTreeWillCommit( ShadowTree const &shadowTree, RootShadowNode::Shared const &oldRootShadowNode, - RootShadowNode::Unshared const &newRootShadowNode) const noexcept { + RootShadowNode::Unshared const &newRootShadowNode) noexcept { std::shared_lock lock(timelinesMutex_); assert(uiManager_ && "`uiManager_` must not be `nullptr`."); diff --git a/packages/react-native/ReactCommon/react/renderer/timeline/TimelineController.h b/packages/react-native/ReactCommon/react/renderer/timeline/TimelineController.h index fe09e203cc9..c69a247791f 100644 --- a/packages/react-native/ReactCommon/react/renderer/timeline/TimelineController.h +++ b/packages/react-native/ReactCommon/react/renderer/timeline/TimelineController.h @@ -52,14 +52,11 @@ class TimelineController final : public UIManagerCommitHook { RootShadowNode::Unshared shadowTreeWillCommit( ShadowTree const &shadowTree, RootShadowNode::Shared const &oldRootShadowNode, - RootShadowNode::Unshared const &newRootShadowNode) - const noexcept override; + RootShadowNode::Unshared const &newRootShadowNode) noexcept override; - void commitHookWasRegistered( - UIManager const &uiManager) const noexcept override; + void commitHookWasRegistered(UIManager const &uiManager) noexcept override; - void commitHookWasUnregistered( - UIManager const &uiManager) const noexcept override; + void commitHookWasUnregistered(UIManager const &uiManager) noexcept override; private: /* diff --git a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.cpp b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.cpp index 8078977d41b..09c92088aff 100644 --- a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.cpp +++ b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.cpp @@ -594,8 +594,7 @@ ShadowTreeRegistry const &UIManager::getShadowTreeRegistry() const { return shadowTreeRegistry_; } -void UIManager::registerCommitHook( - UIManagerCommitHook const &commitHook) const { +void UIManager::registerCommitHook(UIManagerCommitHook &commitHook) { std::unique_lock lock(commitHookMutex_); react_native_assert( std::find(commitHooks_.begin(), commitHooks_.end(), &commitHook) == @@ -604,8 +603,7 @@ void UIManager::registerCommitHook( commitHooks_.push_back(&commitHook); } -void UIManager::unregisterCommitHook( - UIManagerCommitHook const &commitHook) const { +void UIManager::unregisterCommitHook(UIManagerCommitHook &commitHook) { std::unique_lock lock(commitHookMutex_); auto iterator = std::find(commitHooks_.begin(), commitHooks_.end(), &commitHook); @@ -638,7 +636,7 @@ RootShadowNode::Unshared UIManager::shadowTreeWillCommit( std::shared_lock lock(commitHookMutex_); auto resultRootShadowNode = newRootShadowNode; - for (auto const *commitHook : commitHooks_) { + for (auto *commitHook : commitHooks_) { resultRootShadowNode = commitHook->shadowTreeWillCommit( shadowTree, oldRootShadowNode, resultRootShadowNode); } diff --git a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.h b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.h index f2382c3b579..97db13c7927 100644 --- a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.h +++ b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.h @@ -80,8 +80,8 @@ class UIManager final : public ShadowTreeDelegate { /* * Registers and unregisters a commit hook. */ - void registerCommitHook(UIManagerCommitHook const &commitHook) const; - void unregisterCommitHook(UIManagerCommitHook const &commitHook) const; + void registerCommitHook(UIManagerCommitHook &commitHook); + void unregisterCommitHook(UIManagerCommitHook &commitHook); /* * Registers and unregisters a mount hook. @@ -224,7 +224,7 @@ class UIManager final : public ShadowTreeDelegate { ContextContainer::Shared contextContainer_; mutable std::shared_mutex commitHookMutex_; - mutable std::vector commitHooks_; + mutable std::vector commitHooks_; mutable std::shared_mutex mountHookMutex_; mutable std::vector mountHooks_; diff --git a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerCommitHook.h b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerCommitHook.h index c6fdfaab4e0..9424cb79123 100644 --- a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerCommitHook.h +++ b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerCommitHook.h @@ -22,10 +22,9 @@ class UIManagerCommitHook { /* * Called right after the commit hook is registered or unregistered. */ - virtual void commitHookWasRegistered( - UIManager const &uiManager) const noexcept = 0; + virtual void commitHookWasRegistered(UIManager const &uiManager) noexcept = 0; virtual void commitHookWasUnregistered( - UIManager const &uiManager) const noexcept = 0; + UIManager const &uiManager) noexcept = 0; /* * Called right before a `ShadowTree` commits a new tree. @@ -35,7 +34,7 @@ class UIManagerCommitHook { virtual RootShadowNode::Unshared shadowTreeWillCommit( ShadowTree const &shadowTree, RootShadowNode::Shared const &oldRootShadowNode, - RootShadowNode::Unshared const &newRootShadowNode) const noexcept = 0; + RootShadowNode::Unshared const &newRootShadowNode) noexcept = 0; virtual ~UIManagerCommitHook() noexcept = default; };