Skip to content

Commit

Permalink
Fabric: Managing an EventHandler as a unique_pointer
Browse files Browse the repository at this point in the history
Summary:
I realized that instead of using shared_ptr's type-erasure feature, we can make the EventHandler's destructor virtual and this itself will allow safe deallocation by a pointer to a base class.
We cannot use the same technic for EventTarget thought because having a weak_ptr to this is another feature of shared_ptr that we need.

Reviewed By: mdvacca

Differential Revision: D9775742

fbshipit-source-id: 3c23a163827e8aa9ec731c89ce87051a93afe4ca
  • Loading branch information
shergin authored and facebook-github-bot committed Sep 14, 2018
1 parent 93dd790 commit e05acf1
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 6 deletions.
14 changes: 11 additions & 3 deletions ReactCommon/fabric/events/primitives.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,18 @@ enum class EventPriority: int {
/*
* We need this types only to ensure type-safety when we deal with them. Conceptually,
* they are opaque pointers to some types that derived from those classes.
*
* `EventHandler` is managed as a `unique_ptr`, so it must have a *virtual*
* destructor to allow proper deallocation having only a pointer
* to the base (`EventHandler`) class.
*
* `EventTarget` is managed as a `shared_ptr`, so it does not need to have a virtual
* destructor because `shared_ptr` stores a pointer to destructor inside.
*/
class EventHandler {};
class EventTarget {};
using SharedEventHandler = std::shared_ptr<const EventHandler>;
struct EventHandler { virtual ~EventHandler() = default; };
using UniqueEventHandler = std::unique_ptr<const EventHandler>;

struct EventTarget {};
using SharedEventTarget = std::shared_ptr<const EventTarget>;
using WeakEventTarget = std::weak_ptr<const EventTarget>;

Expand Down
2 changes: 1 addition & 1 deletion ReactCommon/fabric/uimanager/FabricUIManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ void FabricUIManager::completeRoot(int rootTag, const SharedShadowNodeUnsharedLi
}
}

void FabricUIManager::registerEventHandler(std::shared_ptr<EventHandler> eventHandler) {
void FabricUIManager::registerEventHandler(UniqueEventHandler eventHandler) {
isLoggingEnabled && LOG(INFO) << "FabricUIManager::registerEventHandler(eventHandler: " << eventHandler.get() << ")";
eventHandler_ = std::move(eventHandler);
}
Expand Down
4 changes: 2 additions & 2 deletions ReactCommon/fabric/uimanager/FabricUIManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,13 @@ class FabricUIManager {
SharedShadowNodeUnsharedList createChildSet(Tag rootTag);
void appendChildToSet(const SharedShadowNodeUnsharedList &childSet, const SharedShadowNode &childNode);
void completeRoot(Tag rootTag, const SharedShadowNodeUnsharedList &childSet);
void registerEventHandler(std::shared_ptr<EventHandler> eventHandler);
void registerEventHandler(UniqueEventHandler eventHandler);

private:

SharedComponentDescriptorRegistry componentDescriptorRegistry_;
UIManagerDelegate *delegate_;
std::shared_ptr<EventHandler> eventHandler_;
UniqueEventHandler eventHandler_;
std::function<DispatchEventToEmptyTargetFunction> dispatchEventToEmptyTargetFunction_;
std::function<DispatchEventToTargetFunction> dispatchEventToTargetFunction_;
};
Expand Down

0 comments on commit e05acf1

Please sign in to comment.