From 7e84cadc9c933fce19d2f9cc580f76b1e0d54089 Mon Sep 17 00:00:00 2001
From: Valentin Shergin <shergin@fb.com>
Date: Tue, 10 Apr 2018 12:45:35 -0700
Subject: [PATCH] Fabric: Refined conception and usage of Sealable

Summary: Slightly new approach: Some non-const methods might not always mutate objects, so sometimes we should call `ensureUnsealed()` only inside conditional branches where we actually mutate an instance.

Reviewed By: fkgozali

Differential Revision: D7467793

fbshipit-source-id: 1b9f229cf6816e54e0df36699a571fdb612d3c3c
---
 ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp | 7 ++++---
 ReactCommon/fabric/core/primitives/Sealable.h           | 8 ++++++--
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp b/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp
index 8148f00dff..0e2652efc8 100644
--- a/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp
+++ b/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp
@@ -23,6 +23,8 @@ bool LayoutableShadowNode::setLayoutMetrics(LayoutMetrics layoutMetrics) {
     return false;
   }
 
+  ensureUnsealed();
+
   layoutMetrics_ = layoutMetrics;
   return true;
 }
@@ -60,8 +62,6 @@ Float LayoutableShadowNode::lastBaseline(Size size) const {
 }
 
 void LayoutableShadowNode::layout(LayoutContext layoutContext) {
-  ensureUnsealed();
-
   layoutChildren(layoutContext);
 
   for (auto child : getChildren()) {
@@ -69,6 +69,8 @@ void LayoutableShadowNode::layout(LayoutContext layoutContext) {
       continue;
     }
 
+    ensureUnsealed();
+
     // The assumption:
     // All `sealed` children were replaced with not-yet-sealed clones
     // somewhere in `layoutChildren`.
@@ -89,7 +91,6 @@ void LayoutableShadowNode::layout(LayoutContext layoutContext) {
 }
 
 void LayoutableShadowNode::layoutChildren(LayoutContext layoutContext) {
-  ensureUnsealed();
   // Default implementation does nothing.
 }
 
diff --git a/ReactCommon/fabric/core/primitives/Sealable.h b/ReactCommon/fabric/core/primitives/Sealable.h
index e0d168e44e..7f07819a1f 100644
--- a/ReactCommon/fabric/core/primitives/Sealable.h
+++ b/ReactCommon/fabric/core/primitives/Sealable.h
@@ -31,7 +31,12 @@ namespace react {
  *
  * How to use:
  *   1. Inherit your class from `Sealable`.
- *   2. Call `ensureUnsealed()` from all non-const methods.
+ *   2. Call `ensureUnsealed()` in all cases where the object might be mutated:
+ *      a. At the beginning of all *always* mutating `non-const` methods;
+ *      b. Right before the place where actual mutation happens in all *possible*
+ *         mutating `non-const` methods;
+ *      c. Right after performing `const_cast`. (Optionally. This is not strictly
+ *         necessary but might help detect problems earlier.)
  *   3. Call `seal()` at some point from which any modifications
  *      must be prevented.
  */
@@ -55,7 +60,6 @@ public:
    */
   bool getSealed() const;
 
-protected:
   /*
    * Throws an exception if the object is sealed.
    * Call this from all non-`const` methods.