From 4120078b8aa0732b4b678ca383e499f19cf41595 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Fri, 14 Sep 2018 15:17:07 -0700 Subject: [PATCH] Fabric: All JavaScript-facing methods of UIManager marked as `const` Summary: In modern C++ `const` basically means `thread-safe` and we commit that all that methods are thread-safe. Reviewed By: mdvacca Differential Revision: D9836100 fbshipit-source-id: 4241ca80da77338b25246e622cf8d7e8c360eff7 --- .../fabric/uimanager/FabricUIManager.cpp | 23 ++++++++------ .../fabric/uimanager/FabricUIManager.h | 31 ++++++++++++------- 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/ReactCommon/fabric/uimanager/FabricUIManager.cpp b/ReactCommon/fabric/uimanager/FabricUIManager.cpp index ff8456a0946..6e48b41e22e 100644 --- a/ReactCommon/fabric/uimanager/FabricUIManager.cpp +++ b/ReactCommon/fabric/uimanager/FabricUIManager.cpp @@ -122,7 +122,7 @@ void FabricUIManager::dispatchEventToTarget(const EventTarget *eventTarget, cons } } -SharedShadowNode FabricUIManager::createNode(int tag, std::string viewName, int rootTag, folly::dynamic props, SharedEventTarget eventTarget) { +SharedShadowNode FabricUIManager::createNode(int tag, std::string viewName, int rootTag, folly::dynamic props, SharedEventTarget eventTarget) const { isLoggingEnabled && LOG(INFO) << "FabricUIManager::createNode(tag: " << tag << ", name: " << viewName << ", rootTag: " << rootTag << ", props: " << props << ")"; ComponentName componentName = componentNameByReactViewName(viewName); @@ -146,7 +146,7 @@ SharedShadowNode FabricUIManager::createNode(int tag, std::string viewName, int return shadowNode; } -SharedShadowNode FabricUIManager::cloneNode(const SharedShadowNode &shadowNode) { +SharedShadowNode FabricUIManager::cloneNode(const SharedShadowNode &shadowNode) const { isLoggingEnabled && LOG(INFO) << "FabricUIManager::cloneNode(shadowNode: " << shadowNode->getDebugDescription(DebugStringConvertibleOptions {.format = false}) << ")"; const SharedComponentDescriptor &componentDescriptor = (*componentDescriptorRegistry_)[shadowNode]; @@ -157,7 +157,7 @@ SharedShadowNode FabricUIManager::cloneNode(const SharedShadowNode &shadowNode) return clonedShadowNode; } -SharedShadowNode FabricUIManager::cloneNodeWithNewChildren(const SharedShadowNode &shadowNode) { +SharedShadowNode FabricUIManager::cloneNodeWithNewChildren(const SharedShadowNode &shadowNode) const { isLoggingEnabled && LOG(INFO) << "FabricUIManager::cloneNodeWithNewChildren(shadowNode: " << shadowNode->getDebugDescription(DebugStringConvertibleOptions {.format = false}) << ")"; // Assuming semantic: Cloning with same props but empty children. const SharedComponentDescriptor &componentDescriptor = (*componentDescriptorRegistry_)[shadowNode]; @@ -174,7 +174,7 @@ SharedShadowNode FabricUIManager::cloneNodeWithNewChildren(const SharedShadowNod return clonedShadowNode; } -SharedShadowNode FabricUIManager::cloneNodeWithNewProps(const SharedShadowNode &shadowNode, folly::dynamic props) { +SharedShadowNode FabricUIManager::cloneNodeWithNewProps(const SharedShadowNode &shadowNode, folly::dynamic props) const { isLoggingEnabled && LOG(INFO) << "FabricUIManager::cloneNodeWithNewProps(shadowNode: " << shadowNode->getDebugDescription(DebugStringConvertibleOptions {.format = false}) << ", props: " << props << ")"; // Assuming semantic: Cloning with same children and specified props. const SharedComponentDescriptor &componentDescriptor = (*componentDescriptorRegistry_)[shadowNode]; @@ -192,7 +192,7 @@ SharedShadowNode FabricUIManager::cloneNodeWithNewProps(const SharedShadowNode & return clonedShadowNode; } -SharedShadowNode FabricUIManager::cloneNodeWithNewChildrenAndProps(const SharedShadowNode &shadowNode, folly::dynamic props) { +SharedShadowNode FabricUIManager::cloneNodeWithNewChildrenAndProps(const SharedShadowNode &shadowNode, folly::dynamic props) const { isLoggingEnabled && LOG(INFO) << "FabricUIManager::cloneNodeWithNewChildrenAndProps(shadowNode: " << shadowNode->getDebugDescription(DebugStringConvertibleOptions {.format = false}) << ", props: " << props << ")"; // Assuming semantic: Cloning with empty children and specified props. const SharedComponentDescriptor &componentDescriptor = (*componentDescriptorRegistry_)[shadowNode]; @@ -211,23 +211,23 @@ SharedShadowNode FabricUIManager::cloneNodeWithNewChildrenAndProps(const SharedS return clonedShadowNode; } -void FabricUIManager::appendChild(const SharedShadowNode &parentShadowNode, const SharedShadowNode &childShadowNode) { +void FabricUIManager::appendChild(const SharedShadowNode &parentShadowNode, const SharedShadowNode &childShadowNode) const { isLoggingEnabled && LOG(INFO) << "FabricUIManager::appendChild(parentShadowNode: " << parentShadowNode->getDebugDescription(DebugStringConvertibleOptions {.format = false}) << ", childShadowNode: " << childShadowNode->getDebugDescription(DebugStringConvertibleOptions {.format = false}) << ")"; const SharedComponentDescriptor &componentDescriptor = (*componentDescriptorRegistry_)[parentShadowNode]; componentDescriptor->appendChild(parentShadowNode, childShadowNode); } -SharedShadowNodeUnsharedList FabricUIManager::createChildSet(int rootTag) { +SharedShadowNodeUnsharedList FabricUIManager::createChildSet(int rootTag) const { isLoggingEnabled && LOG(INFO) << "FabricUIManager::createChildSet(rootTag: " << rootTag << ")"; return std::make_shared(SharedShadowNodeList({})); } -void FabricUIManager::appendChildToSet(const SharedShadowNodeUnsharedList &shadowNodeList, const SharedShadowNode &shadowNode) { +void FabricUIManager::appendChildToSet(const SharedShadowNodeUnsharedList &shadowNodeList, const SharedShadowNode &shadowNode) const { isLoggingEnabled && LOG(INFO) << "FabricUIManager::appendChildToSet(shadowNodeList: " << shadowNodeList << ", shadowNode: " << shadowNode->getDebugDescription(DebugStringConvertibleOptions {.format = false}) << ")"; shadowNodeList->push_back(shadowNode); } -void FabricUIManager::completeRoot(int rootTag, const SharedShadowNodeUnsharedList &children) { +void FabricUIManager::completeRoot(int rootTag, const SharedShadowNodeUnsharedList &children) const { isLoggingEnabled && LOG(INFO) << "FabricUIManager::completeRoot(rootTag: " << rootTag << ", shadowNodeList: " << children << ")"; if (delegate_) { @@ -235,8 +235,11 @@ void FabricUIManager::completeRoot(int rootTag, const SharedShadowNodeUnsharedLi } } -void FabricUIManager::registerEventHandler(UniqueEventHandler eventHandler) { +void FabricUIManager::registerEventHandler(UniqueEventHandler eventHandler) const { isLoggingEnabled && LOG(INFO) << "FabricUIManager::registerEventHandler(eventHandler: " << eventHandler.get() << ")"; + + // Technically, it should be protected by a mutex but regularly it should + // be safe because it used only during initialization process. eventHandler_ = std::move(eventHandler); } diff --git a/ReactCommon/fabric/uimanager/FabricUIManager.h b/ReactCommon/fabric/uimanager/FabricUIManager.h index 61ddbbdfe08..c881ac43784 100644 --- a/ReactCommon/fabric/uimanager/FabricUIManager.h +++ b/ReactCommon/fabric/uimanager/FabricUIManager.h @@ -18,6 +18,9 @@ namespace facebook { namespace react { +class FabricUIManager; +using UIManager = FabricUIManager; + using DispatchEventToEmptyTargetFunction = void (const EventHandler &eventHandler, std::string type, folly::dynamic payload); using DispatchEventToTargetFunction = void (const EventHandler &eventHandler, const EventTarget &eventTarget, std::string type, folly::dynamic payload); using ReleaseEventTargetFunction = void (EventTarget eventTarget); @@ -51,22 +54,28 @@ public: #pragma mark - JavaScript/React-facing Interface - SharedShadowNode createNode(Tag reactTag, std::string viewName, Tag rootTag, folly::dynamic props, SharedEventTarget eventTarget); - SharedShadowNode cloneNode(const SharedShadowNode &node); - SharedShadowNode cloneNodeWithNewChildren(const SharedShadowNode &node); - SharedShadowNode cloneNodeWithNewProps(const SharedShadowNode &node, folly::dynamic props); - SharedShadowNode cloneNodeWithNewChildrenAndProps(const SharedShadowNode &node, folly::dynamic newProps); - void appendChild(const SharedShadowNode &parentNode, const SharedShadowNode &childNode); - SharedShadowNodeUnsharedList createChildSet(Tag rootTag); - void appendChildToSet(const SharedShadowNodeUnsharedList &childSet, const SharedShadowNode &childNode); - void completeRoot(Tag rootTag, const SharedShadowNodeUnsharedList &childSet); - void registerEventHandler(UniqueEventHandler eventHandler); + /* + * All those JavaScript-facing methods call be called from any thread. + * `UIManager` guarantees its own thread-safety, but it does *not* guarantee + * thread-safety of `ShadowNode`s that it operates on. The caller should + * enforce logical correctness and thread-safety of the unsealed `ShadowNode`s. + */ + SharedShadowNode createNode(Tag reactTag, std::string viewName, Tag rootTag, folly::dynamic props, SharedEventTarget eventTarget) const; + SharedShadowNode cloneNode(const SharedShadowNode &node) const; + SharedShadowNode cloneNodeWithNewChildren(const SharedShadowNode &node) const; + SharedShadowNode cloneNodeWithNewProps(const SharedShadowNode &node, folly::dynamic props) const; + SharedShadowNode cloneNodeWithNewChildrenAndProps(const SharedShadowNode &node, folly::dynamic newProps) const; + void appendChild(const SharedShadowNode &parentNode, const SharedShadowNode &childNode) const; + SharedShadowNodeUnsharedList createChildSet(Tag rootTag) const; + void appendChildToSet(const SharedShadowNodeUnsharedList &childSet, const SharedShadowNode &childNode) const; + void completeRoot(Tag rootTag, const SharedShadowNodeUnsharedList &childSet) const; + void registerEventHandler(UniqueEventHandler eventHandler) const; private: SharedComponentDescriptorRegistry componentDescriptorRegistry_; UIManagerDelegate *delegate_; - UniqueEventHandler eventHandler_; + mutable UniqueEventHandler eventHandler_; std::function dispatchEventToEmptyTargetFunction_; std::function dispatchEventToTargetFunction_; };