Fix non layout (#309)

* Lock released between add to pend controller and modifying pend state

The existing design is pretty fraught with error. We should probably
rethink this but in the meantime, this fixes a bug where calling
setNeedsLayout can start failing for nodes.

Essentially the method ASDisplayNodeShouldApplyBridgedWriteToView has
a side effect of registering a node to apply it's pending state *if*
it doesn't currently need the pending state applied. My guess is this
was to avoid continually registering the node and this behavior actually
helped expose this bug.

The bug: after the node is registered for flushing it's state, several
code paths released the lock before applying that state to the pending
state object. Before it could re-obtain the lock to apply it to the pending
state, the pending state controller flushed it on the main thread.

On subsequent calls to setNeedsLayout, the pending state had pending state
already (from previous calls which missed the flush) and thus wasn't
registered for future flushing.

* Add changelog

CHANGELOG.md

@@ -1,6 +1,7 @@
 ## master
 
 * Add your own contributions to the next release on the line below this with your name.
+- Fixed an issue where calls to setNeedsDisplay and setNeedsLayout would stop working on loaded nodes. [Garrett Moon](https://github.com/garrettmoon)
 - [ASTextKitFontSizeAdjuster] [Ricky Cancro] Replace use of NSAttributedString's boundingRectWithSize:options:context: with NSLayoutManager's boundingRectForGlyphRange:inTextContainer:
 - Add support for IGListKit post-removal-of-IGListSectionType, in preparation for IGListKit 3.0.0 release. [Adlai Holler](https://github.com/Adlai-Holler) [#49](https://github.com/TextureGroup/Texture/pull/49)
 - Fix `__has_include` check in ASLog.h [Philipp Smorygo]([email protected])

Source/Private/ASDisplayNode+UIViewBridge.mm

@@ -56,6 +56,9 @@
 /// Side Effect: Registers the node with the shared ASPendingStateController if
 /// the property cannot be immediately applied and the node does not already have pending changes.
 /// This function must be called with the node's lock already held (after _bridge_prologue_write).
+/// *warning* the lock should *not* be released until the pending state is updated if this method
+/// returns NO. Otherwise, the pending state can be scheduled and flushed *before* you get a chance
+/// to apply it.
 ASDISPLAYNODE_INLINE BOOL ASDisplayNodeShouldApplyBridgedWriteToView(ASDisplayNode *node) {
   BOOL loaded = __loaded(node);
   if (ASDisplayNodeThreadIsMain()) {
@@ -313,6 +316,11 @@ - (void)setNeedsDisplay
     isRasterized = _hierarchyState & ASHierarchyStateRasterized;
     shouldApply = ASDisplayNodeShouldApplyBridgedWriteToView(self);
     viewOrLayer = _view ?: _layer;
+
+    if (isRasterized == NO && shouldApply == NO) {
+      // We can't release the lock before applying to pending state, or it may be flushed before it can be applied.
+      [ASDisplayNodeGetPendingState(self) setNeedsDisplay];
+    }
   }
 
   if (isRasterized) {
@@ -337,9 +345,6 @@ - (void)setNeedsDisplay
       // message to the view/layer first. This is because __setNeedsDisplay calls as scheduleNodeForDisplay,
       // which may call -displayIfNeeded. We want to ensure the needsDisplay flag is set now, and then cleared.
       [viewOrLayer setNeedsDisplay];
-    } else {
-      _bridge_prologue_write;
-      [ASDisplayNodeGetPendingState(self) setNeedsDisplay];
     }
     [self __setNeedsDisplay];
   }
@@ -355,6 +360,13 @@ - (void)setNeedsLayout
     shouldApply = ASDisplayNodeShouldApplyBridgedWriteToView(self);
     loaded = __loaded(self);
     viewOrLayer = _view ?: _layer;
+    if (shouldApply == NO && loaded) {
+      // The node is loaded but we're not on main.
+      // We will call [self __setNeedsLayout] when we apply the pending state.
+      // We need to call it on main if the node is loaded to support automatic subnode management.
+      // We can't release the lock before applying to pending state, or it may be flushed before it can be applied.
+      [ASDisplayNodeGetPendingState(self) setNeedsLayout];
+    }
   }
 
   if (shouldApply) {
@@ -363,13 +375,7 @@ - (void)setNeedsLayout
     // the view or layer to ensure that measurement and implicitly added subnodes have been handled.
     [self __setNeedsLayout];
     [viewOrLayer setNeedsLayout];
-  } else if (loaded) {
-    // The node is loaded but we're not on main.
-    // We will call [self __setNeedsLayout] when we apply the pending state.
-    // We need to call it on main if the node is loaded to support automatic subnode management.
-    _bridge_prologue_write;
-    [ASDisplayNodeGetPendingState(self) setNeedsLayout];
-  } else {
+  } else if (loaded == NO) {
     // The node is not loaded and we're not on main.
     [self __setNeedsLayout];
   }
@@ -385,19 +391,20 @@ - (void)layoutIfNeeded
     shouldApply = ASDisplayNodeShouldApplyBridgedWriteToView(self);
     loaded = __loaded(self);
     viewOrLayer = _view ?: _layer;
+    if (shouldApply == NO && loaded) {
+      // The node is loaded but we're not on main.
+      // We will call layoutIfNeeded on the view or layer when we apply the pending state. __layout will in turn be called on us (see -[_ASDisplayLayer layoutSublayers]).
+      // We need to call it on main if the node is loaded to support automatic subnode management.
+      // We can't release the lock before applying to pending state, or it may be flushed before it can be applied.
+      [ASDisplayNodeGetPendingState(self) layoutIfNeeded];
+    }
   }
 
   if (shouldApply) {
     // The node is loaded and we're on main.
     // Message the view or layer which in turn will call __layout on us (see -[_ASDisplayLayer layoutSublayers]).
     [viewOrLayer layoutIfNeeded];
-  } else if (loaded) {
-    // The node is loaded but we're not on main.
-    // We will call layoutIfNeeded on the view or layer when we apply the pending state. __layout will in turn be called on us (see -[_ASDisplayLayer layoutSublayers]).
-    // We need to call it on main if the node is loaded to support automatic subnode management.
-    _bridge_prologue_write;
-    [ASDisplayNodeGetPendingState(self) layoutIfNeeded];
-  } else {
+  } else if (loaded == NO) {
     // The node is not loaded and we're not on main.
     [self __layout];
   }