Skip to content

Commit

Permalink
Fabric: Clarifying life-time concerns of ShadowTree::delegate_
Browse files Browse the repository at this point in the history
Summary:
This diff makes it clear from the code that ShadowTree delegate must be around for the whole life-time of a ShadowTree instance and there is no way to revoke/reset the delegate. This makes reasoning about the lifetime much simpler.

We didn't call `setDelegate(nullptr)` before, so nothing really changes here.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: mdvacca

Differential Revision: D18542502

fbshipit-source-id: f57ee21e0bb533fb82cb6f8ba7723e40ffb25a38
  • Loading branch information
shergin authored and facebook-github-bot committed Nov 18, 2019
1 parent b346970 commit 9349ee2
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 31 deletions.
23 changes: 6 additions & 17 deletions ReactCommon/fabric/mounting/ShadowTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,11 @@ static void updateMountedFlag(

ShadowTree::ShadowTree(
SurfaceId surfaceId,
const LayoutConstraints &layoutConstraints,
const LayoutContext &layoutContext,
const RootComponentDescriptor &rootComponentDescriptor)
: surfaceId_(surfaceId) {
LayoutConstraints const &layoutConstraints,
LayoutContext const &layoutContext,
RootComponentDescriptor const &rootComponentDescriptor,
ShadowTreeDelegate const &delegate)
: surfaceId_(surfaceId), delegate_(delegate) {
const auto noopEventEmitter = std::make_shared<const ViewEventEmitter>(
nullptr, -1, std::shared_ptr<const EventDispatcher>());

Expand Down Expand Up @@ -197,9 +198,7 @@ bool ShadowTree::tryCommit(ShadowTreeCommitTransaction transaction) const {
mountingCoordinator_->push(
ShadowTreeRevision{newRootShadowNode, revisionNumber, telemetry});

if (delegate_) {
delegate_->shadowTreeDidFinishTransaction(*this, mountingCoordinator_);
}
delegate_.shadowTreeDidFinishTransaction(*this, mountingCoordinator_);

return true;
}
Expand Down Expand Up @@ -244,15 +243,5 @@ void ShadowTree::emitLayoutEvents(
}
}

#pragma mark - Delegate

void ShadowTree::setDelegate(ShadowTreeDelegate const *delegate) {
delegate_ = delegate;
}

ShadowTreeDelegate const *ShadowTree::getDelegate() const {
return delegate_;
}

} // namespace react
} // namespace facebook
15 changes: 3 additions & 12 deletions ReactCommon/fabric/mounting/ShadowTree.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ class ShadowTree final {
SurfaceId surfaceId,
LayoutConstraints const &layoutConstraints,
LayoutContext const &layoutContext,
RootComponentDescriptor const &rootComponentDescriptor);
RootComponentDescriptor const &rootComponentDescriptor,
ShadowTreeDelegate const &delegate);

~ShadowTree();

Expand All @@ -65,16 +66,6 @@ class ShadowTree final {
*/
void commitEmptyTree() const;

#pragma mark - Delegate

/*
* Sets and gets the delegate.
* The delegate is stored as a raw pointer, so the owner must null
* the pointer before being destroyed.
*/
void setDelegate(ShadowTreeDelegate const *delegate);
ShadowTreeDelegate const *getDelegate() const;

private:
RootShadowNode::Unshared cloneRootShadowNode(
RootShadowNode::Shared const &oldRootShadowNode,
Expand All @@ -85,12 +76,12 @@ class ShadowTree final {
std::vector<LayoutableShadowNode const *> &affectedLayoutableNodes) const;

SurfaceId const surfaceId_;
ShadowTreeDelegate const &delegate_;
mutable better::shared_mutex commitMutex_;
mutable RootShadowNode::Shared
rootShadowNode_; // Protected by `commitMutex_`.
mutable ShadowTreeRevision::Number revisionNumber_{
0}; // Protected by `commitMutex_`.
ShadowTreeDelegate const *delegate_;
MountingCoordinator::Shared mountingCoordinator_;
};

Expand Down
7 changes: 5 additions & 2 deletions ReactCommon/fabric/uimanager/Scheduler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,11 @@ void Scheduler::startSurface(
SystraceSection s("Scheduler::startSurface");

auto shadowTree = std::make_unique<ShadowTree>(
surfaceId, layoutConstraints, layoutContext, *rootComponentDescriptor_);
shadowTree->setDelegate(uiManager_.get());
surfaceId,
layoutConstraints,
layoutContext,
*rootComponentDescriptor_,
*uiManager_);

auto uiManager = uiManager_;

Expand Down

0 comments on commit 9349ee2

Please sign in to comment.