From 876a2789dba199c5b17514512eb2f42a7366c4cb Mon Sep 17 00:00:00 2001
From: Valentin Shergin <shergin@fb.com>
Date: Wed, 28 Aug 2019 21:08:27 -0700
Subject: [PATCH] Fabric: Preventing an obsolete state to be committed

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
---
 ReactCommon/fabric/core/state/State.h         |  8 ++++++++
 .../fabric/core/state/StateCoordinator.cpp    | 20 +++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/ReactCommon/fabric/core/state/State.h b/ReactCommon/fabric/core/state/State.h
index dbf7973fa0..6ab0c921af 100644
--- a/ReactCommon/fabric/core/state/State.h
+++ b/ReactCommon/fabric/core/state/State.h
@@ -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
diff --git a/ReactCommon/fabric/core/state/StateCoordinator.cpp b/ReactCommon/fabric/core/state/StateCoordinator.cpp
index b8493a44a4..b1853a3243 100644
--- a/ReactCommon/fabric/core/state/StateCoordinator.cpp
+++ b/ReactCommon/fabric/core/state/StateCoordinator.cpp
@@ -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);
 }