Skip to content

Commit

Permalink
Fabric: Preventing an obsolete state to be committed
Browse files Browse the repository at this point in the history
Summary:
It's okay in Fabric model to commit some subtrees which are kinda obsolete (that's strange but technically it's not against the model), but it's not okay to commit some states that were already committed before.
This diff prevents that.

Reviewed By: JoshuaGross

Differential Revision: D17053428

fbshipit-source-id: fb3536312163b7b57011647b4ba7b931c2179d89
  • Loading branch information
shergin authored and facebook-github-bot committed Aug 29, 2019
1 parent f8e5093 commit 876a278
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 0 deletions.
8 changes: 8 additions & 0 deletions ReactCommon/fabric/core/state/State.h
Expand Up @@ -49,6 +49,14 @@ class State {
* Must be used by `ShadowNode` *only*.
*/
State::Shared getCommitedState() const;

/*
* Indicates that the state was committed once and then was replaced by a
* newer one.
* To be used by `StateCoordinator` only.
* Protected by mutex inside `StateCoordinator`.
*/
mutable bool isObsolete_{false};
};

} // namespace react
Expand Down
20 changes: 20 additions & 0 deletions ReactCommon/fabric/core/state/StateCoordinator.cpp
Expand Up @@ -24,6 +24,26 @@ const StateTarget &StateCoordinator::getTarget() const {

void StateCoordinator::setTarget(StateTarget &&target) const {
std::unique_lock<better::shared_mutex> lock(mutex_);

assert(target && "`StateTarget` must not be empty.");

if (target_) {
auto &previousState = target_.getShadowNode().getState();
auto &nextState = target.getShadowNode().getState();

/*
* Checking and setting `isObsolete_` prevents old states to be recommitted
* on top of fresher states. It's okay to commit a tree with "older" Shadow
* Nodes (the evolution of nodes is not linear), however, we never back out
* states (they progress linearly).
*/
if (nextState->isObsolete_) {
return;
}

previousState->isObsolete_ = true;
}

target_ = std::move(target);
}

Expand Down

0 comments on commit 876a278

Please sign in to comment.