Merge pull request #1256 from Adlai-Holler/AHAffinityRevisions

[ASDisplayNode] Post-Review Refinements of Recent No-Affinity UIKit/CA Property Bridging

AsyncDisplayKit/ASDisplayNode.mm

@@ -79,6 +79,17 @@ BOOL ASDisplayNodeSubclassOverridesSelector(Class subclass, SEL selector)
   return ASSubclassOverridesSelector([ASDisplayNode class], subclass, selector);
 }
 
+_ASPendingState *ASDisplayNodeGetPendingState(ASDisplayNode *node)
+{
+  ASDN::MutexLocker l(node->_propertyLock);
+  _ASPendingState *result = node->_pendingViewState;
+  if (result == nil) {
+    result = [[_ASPendingState alloc] init];
+    node->_pendingViewState = result;
+  }
+  return result;
+}
+
 /**
  *  Returns ASDisplayNodeFlags for the givern class/instance. instance MAY BE NIL.
  *
@@ -257,7 +268,6 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c)
   _contentsScaleForDisplay = ASScreenScale();
   _displaySentinel = [[ASSentinel alloc] init];
   _preferredFrameSize = CGSizeZero;
-  _pendingViewState = [_ASPendingState new];
 }
 
 - (id)init
@@ -1037,11 +1047,6 @@ static inline void filterNodesInLayoutAtIndexesWithIntersectingNodes(
   }
 }
 
-// If not rasterized (and therefore we certainly have a view or layer),
-// Send the message to the view/layer first, as scheduleNodeForDisplay may call -displayIfNeeded.
-// Wrapped / synchronous nodes created with initWithView/LayerBlock: do not need scheduleNodeForDisplay,
-// as they don't need to display in the working range at all - since at all times onscreen, one
-// -setNeedsDisplay to the CALayer will result in a synchronous display in the next frame.
 - (void)__setNeedsDisplay
 {
   BOOL nowDisplay = ASInterfaceStateIncludesDisplay(_interfaceState);
@@ -1894,6 +1899,7 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock)
 
 - (ASSizeRange)constrainedSizeForCalculatedLayout
 {
+  ASDN::MutexLocker l(_propertyLock);
   return _constrainedSize;
 }
 

AsyncDisplayKit/ASImageNode.mm

@@ -123,6 +123,7 @@
   if (!ASObjectIsEqual(_image, image)) {
     _image = image;
 
+    ASDN::MutexUnlocker u(_imageLock);
     [self invalidateCalculatedLayout];
     [self setNeedsDisplay];
   }

AsyncDisplayKit/Private/ASDisplayNode+UIViewBridge.mm

@@ -37,7 +37,7 @@
 
 #define DISPLAYNODE_USE_LOCKS 1
 
-#define __loaded (_layer != nil)
+#define __loaded(node) (node->_view != nil || (node->_layer != nil && node->_flags.layerBacked))
 
 #if DISPLAYNODE_USE_LOCKS
 #define _bridge_prologue_read ASDN::MutexLocker l(_propertyLock); ASDisplayNodeAssertThreadAffinity(self)
@@ -52,37 +52,36 @@
 /// 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).
 ASDISPLAYNODE_INLINE BOOL ASDisplayNodeShouldApplyBridgedWriteToView(ASDisplayNode *node) {
+  BOOL loaded = __loaded(node);
   if (ASDisplayNodeThreadIsMain()) {
-    return node.nodeLoaded;
+    return loaded;
   } else {
-    if (node.nodeLoaded && !node->_pendingViewState.hasChanges) {
+    if (loaded && !node->_pendingViewState.hasChanges) {
-      [ASPendingStateController.sharedInstance registerNode:node];
+      [[ASPendingStateController sharedInstance] registerNode:node];
     }
     return NO;
   }
 };
 
-#define _getFromViewOrLayer(layerProperty, viewAndPendingViewStateProperty) __loaded ? \
+#define _getFromViewOrLayer(layerProperty, viewAndPendingViewStateProperty) __loaded(self) ? \
   (_view ? _view.viewAndPendingViewStateProperty : _layer.layerProperty )\
- : self.pendingViewState.viewAndPendingViewStateProperty
+ : ASDisplayNodeGetPendingState(self).viewAndPendingViewStateProperty
 
 #define _setToViewOrLayer(layerProperty, layerValueExpr, viewAndPendingViewStateProperty, viewAndPendingViewStateExpr) BOOL shouldApply = ASDisplayNodeShouldApplyBridgedWriteToView(self); \
-  if (shouldApply) { (_view ? _view.viewAndPendingViewStateProperty = (viewAndPendingViewStateExpr) : _layer.layerProperty = (layerValueExpr)); } else { _pendingViewState.viewAndPendingViewStateProperty = (viewAndPendingViewStateExpr); }
+  if (shouldApply) { (_view ? _view.viewAndPendingViewStateProperty = (viewAndPendingViewStateExpr) : _layer.layerProperty = (layerValueExpr)); } else { ASDisplayNodeGetPendingState(self).viewAndPendingViewStateProperty = (viewAndPendingViewStateExpr); }
 
 #define _setToViewOnly(viewAndPendingViewStateProperty, viewAndPendingViewStateExpr) BOOL shouldApply = ASDisplayNodeShouldApplyBridgedWriteToView(self); \
-if (shouldApply) { _view.viewAndPendingViewStateProperty = (viewAndPendingViewStateExpr); } else { _pendingViewState.viewAndPendingViewStateProperty = (viewAndPendingViewStateExpr); }
+if (shouldApply) { _view.viewAndPendingViewStateProperty = (viewAndPendingViewStateExpr); } else { ASDisplayNodeGetPendingState(self).viewAndPendingViewStateProperty = (viewAndPendingViewStateExpr); }
 
-#define _getFromViewOnly(viewAndPendingViewStateProperty) __loaded ? _view.viewAndPendingViewStateProperty : self.pendingViewState.viewAndPendingViewStateProperty
+#define _getFromViewOnly(viewAndPendingViewStateProperty) __loaded(self) ? _view.viewAndPendingViewStateProperty : ASDisplayNodeGetPendingState(self).viewAndPendingViewStateProperty
 
-#define _getFromLayer(layerProperty) __loaded ? _layer.layerProperty : self.pendingViewState.layerProperty
+#define _getFromLayer(layerProperty) __loaded(self) ? _layer.layerProperty : ASDisplayNodeGetPendingState(self).layerProperty
 
 #define _setToLayer(layerProperty, layerValueExpr) BOOL shouldApply = ASDisplayNodeShouldApplyBridgedWriteToView(self); \
-if (shouldApply) { _layer.layerProperty = (layerValueExpr); } else { _pendingViewState.layerProperty = (layerValueExpr); }
+if (shouldApply) { _layer.layerProperty = (layerValueExpr); } else { ASDisplayNodeGetPendingState(self).layerProperty = (layerValueExpr); }
 
 #define _messageToViewOrLayer(viewAndLayerSelector) (_view ? [_view viewAndLayerSelector] : [_layer viewAndLayerSelector])
 
-#define _messageToLayer(layerSelector) __loaded ? [_layer layerSelector] : [self.pendingViewState layerSelector]
-
 /**
  * This category implements certain frequently-used properties and methods of UIView and CALayer so that ASDisplayNode clients can just call the view/layer methods on the node,
  * with minimal loss in performance.  Unlike UIView and CALayer methods, these can be called from a non-main thread until the view or layer is created.
@@ -235,36 +234,63 @@ if (shouldApply) { _layer.layerProperty = (layerValueExpr); } else { _pendingVie
 - (void)setFrame:(CGRect)rect
 {
   _bridge_prologue_write;
-  BOOL setFrameDirectly = _flags.synchronous && !_flags.layerBacked;
-  BOOL isMainThread = ASDisplayNodeThreadIsMain();
-  BOOL nodeLoaded = self.nodeLoaded;
-  if (nodeLoaded && isMainThread && setFrameDirectly) {
-    // For classes like ASTableNode, ASCollectionNode, ASScrollNode and similar - make sure UIView gets setFrame:
-    
-    // Frame is only defined when transform is identity because we explicitly diverge from CALayer behavior and define frame without transform
-#if DEBUG
-    // Checking if the transform is identity is expensive, so disable when unnecessary. We have assertions on in Release, so DEBUG is the only way I know of.
-    ASDisplayNodeAssert(CATransform3DIsIdentity(self.transform), @"-[ASDisplayNode setFrame:] - self.transform must be identity in order to set the frame property.  (From Apple's UIView documentation: If the transform property is not the identity transform, the value of this property is undefined and therefore should be ignored.)");
-#endif
-    _view.frame = rect;
-  } else if (!nodeLoaded || (isMainThread && !setFrameDirectly)) {
-    /**
-     * Sets a new frame to this node by changing its bounds and position. This method can be safely called even if
-     * the transform is a non-identity transform, because bounds and position can be set instead of frame.
-     * This is NOT called for synchronous nodes (wrapping regular views), which may rely on a [UIView setFrame:] call.
-     * A notable example of the latter is UITableView, which won't resize its internal container if only layer bounds are set.
-     */
-    CGRect bounds;
-    CGPoint position;
-    ASBoundsAndPositionForFrame(rect, self.bounds.origin, self.anchorPoint, &bounds, &position);
 
-    self.bounds = bounds;
+  // For classes like ASTableNode, ASCollectionNode, ASScrollNode and similar - make sure UIView gets setFrame:
-    self.position = position;
+  struct ASDisplayNodeFlags flags = _flags;
-  } else if (nodeLoaded && !isMainThread) {
+  BOOL setFrameDirectly = flags.synchronous && !flags.layerBacked;
-    if (!_pendingViewState.hasChanges) {
+
-      [ASPendingStateController.sharedInstance registerNode:self];
+  BOOL nodeLoaded = __loaded(self);
+  BOOL isMainThread = ASDisplayNodeThreadIsMain();
+  if (!setFrameDirectly) {
+    BOOL canReadProperties = isMainThread || !nodeLoaded;
+    if (canReadProperties) {
+      // We don't have to set frame directly, and we can read current properties.
+      // Compute a new bounds and position and set them on self.
+      CALayer *layer = _layer;
+      BOOL useLayer = (layer != nil);
+      CGPoint origin = (useLayer ? layer.bounds.origin : self.bounds.origin);
+      CGPoint anchorPoint = (useLayer ? layer.anchorPoint : self.anchorPoint);
+
+      CGRect newBounds = CGRectZero;
+      CGPoint newPosition = CGPointZero;
+      ASBoundsAndPositionForFrame(rect, origin, anchorPoint, &newBounds, &newPosition);
+
+      if (useLayer) {
+        layer.bounds = newBounds;
+        layer.position = newPosition;
+      } else {
+        self.bounds = newBounds;
+        self.position = newPosition;
+      }
+    } else {
+      // We don't have to set frame directly, but we can't read properties.
+      // Store the frame in our pending state, and it'll get decomposed into
+      // bounds and position when the pending state is applied.
+      _ASPendingState *pendingState = ASDisplayNodeGetPendingState(self);
+      if (nodeLoaded && !pendingState.hasChanges) {
+        [[ASPendingStateController sharedInstance] registerNode:self];
+      }
+      pendingState.frame = rect;
+    }
+  } else {
+    if (nodeLoaded && isMainThread) {
+      // We do have to set frame directly, and we're on main thread with a loaded node.
+      // Just set the frame on the view.
+      // NOTE: Frame is only defined when transform is identity because we explicitly diverge from CALayer behavior and define frame without transform.
+#if DEBUG
+      // Checking if the transform is identity is expensive, so disable when unnecessary. We have assertions on in Release, so DEBUG is the only way I know of.
+      ASDisplayNodeAssert(CATransform3DIsIdentity(self.transform), @"-[ASDisplayNode setFrame:] - self.transform must be identity in order to set the frame property.  (From Apple's UIView documentation: If the transform property is not the identity transform, the value of this property is undefined and therefore should be ignored.)");
+#endif
+      _view.frame = rect;
+    } else {
+      // We do have to set frame directly, but either the node isn't loaded or we're on a non-main thread.
+      // Set the frame on the pending state, and it'll call setFrame: when applied.
+      _ASPendingState *pendingState = ASDisplayNodeGetPendingState(self);
+      if (nodeLoaded && !pendingState.hasChanges) {
+        [[ASPendingStateController sharedInstance] registerNode:self];
+      }
+      pendingState.frame = rect;
     }
-    _pendingViewState.frame = rect;
   }
 }
 
@@ -288,39 +314,37 @@ if (shouldApply) { _layer.layerProperty = (layerValueExpr); } else { _pendingVie
       [rasterizedContainerNode setNeedsDisplay];
     });
   } else {
-    if (self.nodeLoaded) {
+    BOOL shouldApply = ASDisplayNodeShouldApplyBridgedWriteToView(self);
-      if (ASDisplayNodeThreadIsMain()) {
+    if (shouldApply) {
-        _messageToViewOrLayer(setNeedsDisplay);
+      // If not rasterized, and the node is loaded (meaning we certainly have a view or layer), send a
-      } else {
+      // message to the view/layer first. This is because __setNeedsDisplay calls as scheduleNodeForDisplay,
-        if (!_pendingViewState.hasChanges) {
+      // which may call -displayIfNeeded. We want to ensure the needsDisplay flag is set now, and then cleared.
-          [ASPendingStateController.sharedInstance registerNode:self];
+      _messageToViewOrLayer(setNeedsDisplay);
-        }
-        [self __setNeedsDisplay];
-        [_pendingViewState setNeedsDisplay];
-      }
     } else {
-      [self __setNeedsDisplay];
+      [ASDisplayNodeGetPendingState(self) setNeedsDisplay];
     }
+    [self __setNeedsDisplay];
   }
 }
 
 - (void)setNeedsLayout
 {
   _bridge_prologue_write;
-  if (self.nodeLoaded) {
+  BOOL shouldApply = ASDisplayNodeShouldApplyBridgedWriteToView(self);
-    if (ASDisplayNodeThreadIsMain()) {
+  if (shouldApply) {
-       [self __setNeedsLayout];
+    // The node is loaded and we're on main.
-        _messageToViewOrLayer(setNeedsLayout);
+    // Quite the opposite of setNeedsDisplay, we must call __setNeedsLayout before messaging
-    } else {
+    // the view or layer to ensure that measurement and implicitly added subnodes have been handled.
-      if (!_pendingViewState.hasChanges) {
+    [self __setNeedsLayout];
-        [ASPendingStateController.sharedInstance registerNode:self];
+    _messageToViewOrLayer(setNeedsLayout);
-      }
+  } else if (__loaded(self)) {
-      // NOTE: We will call [self __setNeedsLayout] just before we apply
+    // The node is loaded but we're not on main.
-      // the pending state. We need to call it on main if the node is loaded
+    // We will call [self __setNeedsLayout] when we apply
-      // to support implicit hierarchy management.
+    // the pending state. We need to call it on main if the node is loaded
-      [_pendingViewState setNeedsLayout];
+    // to support implicit hierarchy management.
-    }
+    [ASDisplayNodeGetPendingState(self) setNeedsLayout];
   } else {
+    // The node is not loaded and we're not on main.
     [self __setNeedsLayout];
   }
 }
@@ -504,28 +528,29 @@ if (shouldApply) { _layer.layerProperty = (layerValueExpr); } else { _pendingVie
 - (UIViewContentMode)contentMode
 {
   _bridge_prologue_read;
-  if (__loaded) {
+  if (__loaded(self)) {
     if (_flags.layerBacked) {
       return ASDisplayNodeUIContentModeFromCAContentsGravity(_layer.contentsGravity);
     } else {
       return _view.contentMode;
     }
   } else {
-    return self.pendingViewState.contentMode;
+    return ASDisplayNodeGetPendingState(self).contentMode;
   }
 }
 
 - (void)setContentMode:(UIViewContentMode)contentMode
 {
   _bridge_prologue_write;
-  if (__loaded) {
+  BOOL shouldApply = ASDisplayNodeShouldApplyBridgedWriteToView(self);
+  if (shouldApply) {
     if (_flags.layerBacked) {
       _layer.contentsGravity = ASDisplayNodeCAContentsGravityFromUIContentMode(contentMode);
     } else {
       _view.contentMode = contentMode;
     }
   } else {
-    self.pendingViewState.contentMode = contentMode;
+    ASDisplayNodeGetPendingState(self).contentMode = contentMode;
   }
 }
 

AsyncDisplayKit/Private/ASDisplayNodeInternal.h

@@ -23,9 +23,13 @@
 
 @protocol _ASDisplayLayerDelegate;
 @class _ASDisplayLayer;
+@class _ASPendingState;
 
 BOOL ASDisplayNodeSubclassOverridesSelector(Class subclass, SEL selector);
 
+/// Get the pending view state for the node, creating one if needed.
+_ASPendingState *ASDisplayNodeGetPendingState(ASDisplayNode *node);
+
 typedef NS_OPTIONS(NSUInteger, ASDisplayNodeMethodOverrides)
 {
   ASDisplayNodeMethodOverrideNone               = 0,
@@ -36,7 +40,6 @@ typedef NS_OPTIONS(NSUInteger, ASDisplayNodeMethodOverrides)
   ASDisplayNodeMethodOverrideLayoutSpecThatFits = 1 << 4
 };
 
-@class _ASPendingState;
 @class _ASDisplayNodePosition;
 
 FOUNDATION_EXPORT NSString * const ASRenderingEngineDidDisplayScheduledNodesNotification;
@@ -52,10 +55,37 @@ FOUNDATION_EXPORT NSString * const ASRenderingEngineDidDisplayNodesScheduledBefo
 @package
   _ASPendingState *_pendingViewState;
 
-@protected
   // Protects access to _view, _layer, _pendingViewState, _subnodes, _supernode, and other properties which are accessed from multiple threads.
   ASDN::RecursiveMutex _propertyLock;
+  UIView *_view;
+  CALayer *_layer;
 
+  struct ASDisplayNodeFlags {
+    // public properties
+    unsigned synchronous:1;
+    unsigned layerBacked:1;
+    unsigned displaysAsynchronously:1;
+    unsigned shouldRasterizeDescendants:1;
+    unsigned shouldBypassEnsureDisplay:1;
+    unsigned displaySuspended:1;
+    unsigned hasCustomDrawingPriority:1;
+
+    // whether custom drawing is enabled
+    unsigned implementsInstanceDrawRect:1;
+    unsigned implementsDrawRect:1;
+    unsigned implementsInstanceImageDisplay:1;
+    unsigned implementsImageDisplay:1;
+    unsigned implementsDrawParameters:1;
+
+    // internal state
+    unsigned isMeasured:1;
+    unsigned isEnteringHierarchy:1;
+    unsigned isExitingHierarchy:1;
+    unsigned isInHierarchy:1;
+    unsigned visibilityNotificationsDisabled:VISIBILITY_NOTIFICATIONS_DISABLED_BITS;
+  } _flags;
+  
+@protected
   ASDisplayNode * __weak _supernode;
 
   ASSentinel *_displaySentinel;
@@ -86,39 +116,12 @@ FOUNDATION_EXPORT NSString * const ASRenderingEngineDidDisplayNodesScheduledBefo
   ASDisplayNodeDidLoadBlock _nodeLoadedBlock;
   Class _viewClass;
   Class _layerClass;
-  UIView *_view;
-  CALayer *_layer;
 
   UIImage *_placeholderImage;
   CALayer *_placeholderLayer;
 
   // keeps track of nodes/subnodes that have not finished display, used with placeholders
   NSMutableSet *_pendingDisplayNodes;
-  
-  struct ASDisplayNodeFlags {
-    // public properties
-    unsigned synchronous:1;
-    unsigned layerBacked:1;
-    unsigned displaysAsynchronously:1;
-    unsigned shouldRasterizeDescendants:1;
-    unsigned shouldBypassEnsureDisplay:1;
-    unsigned displaySuspended:1;
-    unsigned hasCustomDrawingPriority:1;
-
-    // whether custom drawing is enabled
-    unsigned implementsInstanceDrawRect:1;
-    unsigned implementsDrawRect:1;
-    unsigned implementsInstanceImageDisplay:1;
-    unsigned implementsImageDisplay:1;
-    unsigned implementsDrawParameters:1;
-
-    // internal state
-    unsigned isMeasured:1;
-    unsigned isEnteringHierarchy:1;
-    unsigned isExitingHierarchy:1;
-    unsigned isInHierarchy:1;
-    unsigned visibilityNotificationsDisabled:VISIBILITY_NOTIFICATIONS_DISABLED_BITS;
-  } _flags;
 
   ASDisplayNodeExtraIvars _extra;
   
@@ -139,9 +142,6 @@ FOUNDATION_EXPORT NSString * const ASRenderingEngineDidDisplayNodesScheduledBefo
 // The _ASDisplayLayer backing the node, if any.
 @property (nonatomic, readonly, retain) _ASDisplayLayer *asyncLayer;
 
-// Creates a pendingViewState if one doesn't exist. Allows setting view properties on a bg thread before there is a view.
-@property (atomic, retain, readonly) _ASPendingState *pendingViewState;
-
 // Bitmask to check which methods an object overrides.
 @property (nonatomic, assign, readonly) ASDisplayNodeMethodOverrides methodOverrides;
 

AsyncDisplayKit/Private/ASInternalHelpers.h

@@ -51,9 +51,9 @@ ASDISPLAYNODE_INLINE void ASPerformBlockWithoutAnimation(BOOL withoutAnimation,
 
 ASDISPLAYNODE_INLINE void ASBoundsAndPositionForFrame(CGRect rect, CGPoint origin, CGPoint anchorPoint, CGRect *bounds, CGPoint *position)
 {
-  *bounds      = (CGRect){ origin, rect.size };
+  *bounds   = (CGRect){ origin, rect.size };
-  *position    = CGPointMake(rect.origin.x + rect.size.width * anchorPoint.x,
+  *position = CGPointMake(rect.origin.x + rect.size.width * anchorPoint.x,
-                            rect.origin.y + rect.size.height * anchorPoint.y);
+                          rect.origin.y + rect.size.height * anchorPoint.y);
 }
 
 @interface NSIndexPath (ASInverseComparison)

AsyncDisplayKit/Private/ASPendingStateController.mm

@@ -32,7 +32,7 @@
 {
   self = [super init];
   if (self) {
-    _dirtyNodes = [ASWeakSet new];
+    _dirtyNodes = [[ASWeakSet alloc] init];
   }
   return self;
 }
@@ -40,21 +40,15 @@
 + (ASPendingStateController *)sharedInstance
 {
   static dispatch_once_t onceToken;
-  static ASPendingStateController *controller;
+  static ASPendingStateController *controller = nil;
   dispatch_once(&onceToken, ^{
-    controller = [ASPendingStateController new];
+    controller = [[ASPendingStateController alloc] init];
   });
   return controller;
 }
 
 #pragma mark External API
 
-- (void)flush
-{
-  ASDisplayNodeAssertMainThread();
-  [self flushNow];
-}
-
 - (void)registerNode:(ASDisplayNode *)node
 {
   ASDisplayNodeAssert(node.nodeLoaded, @"Expected display node to be loaded before it was registered with ASPendingStateController. Node: %@", node);
@@ -64,6 +58,25 @@
   [self scheduleFlushIfNeeded];
 }
 
+/**
+ * NOTE: There is a small re-entrancy hazard here.
+ * If the user gives us a subclass of UIView/CALayer that
+ * adds side-effects to property sets, and one side effect
+ * waits on a background thread that sets a view/layer property
+ * on a loaded node, then we've got a deadlock.
+ */
+- (void)flush
+{
+  ASDisplayNodeAssertMainThread();
+  ASDN::MutexLocker l(_lock);
+  for (ASDisplayNode *node in _dirtyNodes) {
+    [node applyPendingViewState];
+  }
+  [_dirtyNodes removeAllObjects];
+  _flags.pendingFlush = NO;
+}
+
+
 #pragma mark Private Methods
 
 /**
@@ -77,27 +90,10 @@
 
   _flags.pendingFlush = YES;
   dispatch_async(dispatch_get_main_queue(), ^{
-    [self flushNow];
+    [self flush];
   });
 }
 
-/**
- * NOTE: There is a small re-entrancy hazard here.
- * If the user gives us a subclass of UIView/CALayer that
- * adds side-effects to property sets, and one side effect
- * waits on a background thread that sets a view/layer property
- * on a loaded node, then we've got a deadlock.
- */
-- (void)flushNow
-{
-  ASDN::MutexLocker l(_lock);
-  for (ASDisplayNode *node in _dirtyNodes) {
-    [node applyPendingViewState];
-  }
-  [_dirtyNodes removeAllObjects];
-  _flags.pendingFlush = NO;
-}
-
 @end
 
 @implementation ASPendingStateController (Testing)

AsyncDisplayKit/Private/_ASPendingState.mm

@@ -14,6 +14,10 @@
 #import "ASInternalHelpers.h"
 #import "ASDisplayNodeInternal.h"
 
+#define __shouldSetNeedsDisplay(layer) (flags.needsDisplay \
+  || (flags.setOpaque && opaque != (layer).opaque)\
+  || (flags.setBackgroundColor && !CGColorEqualToColor(backgroundColor, (layer).backgroundColor)))
+
 typedef struct {
   // Properties
   int needsDisplay:1;
@@ -106,6 +110,30 @@ typedef struct {
   ASPendingStateFlags _flags;
 }
 
+/**
+ * Apply the state's frame, bounds, and position to layer. This will not
+ * be called on synchronous view-backed nodes which require we directly
+ * call [view setFrame:].
+ *
+ * FIXME: How should we reconcile order-of-operations between setting frame, bounds, position?
+ * Note we can't read bounds and position in the background, so we have to keep the frame
+ * value intact until application time (now).
+ */
+ASDISPLAYNODE_INLINE void ASPendingStateApplyMetricsToLayer(_ASPendingState *state, CALayer *layer) {
+  ASPendingStateFlags flags = state->_flags;
+  if (flags.setFrame) {
+    CGRect _bounds = CGRectZero;
+    CGPoint _position = CGPointZero;
+    ASBoundsAndPositionForFrame(state->frame, layer.bounds.origin, layer.anchorPoint, &_bounds, &_position);
+    layer.bounds = _bounds;
+    layer.position = _position;
+  } else {
+    if (flags.setBounds)
+      layer.bounds = state->bounds;
+    if (flags.setPosition)
+      layer.position = state->position;
+  }
+}
 
 @synthesize clipsToBounds=clipsToBounds;
 @synthesize opaque=opaque;
@@ -560,9 +588,7 @@ static UIColor *defaultTintColor = nil;
 {
   ASPendingStateFlags flags = _flags;
 
-  if (flags.needsDisplay
+  if (__shouldSetNeedsDisplay(layer)) {
-      || (flags.setOpaque && opaque != layer.opaque)
-      || (flags.setBackgroundColor && !CGColorEqualToColor(backgroundColor, layer.backgroundColor))) {
     [layer setNeedsDisplay];
   }
 
@@ -641,18 +667,7 @@ static UIColor *defaultTintColor = nil;
   if (flags.setOpaque)
     ASDisplayNodeAssert(layer.opaque == opaque, @"Didn't set opaque as desired");
 
-  if (flags.setFrame) {
+  ASPendingStateApplyMetricsToLayer(self, layer);
-    CGRect _bounds;
-    CGPoint _position;
-    ASBoundsAndPositionForFrame(self.frame, layer.bounds.origin, layer.anchorPoint, &_bounds, &_position);
-    layer.bounds = _bounds;
-    layer.position = _position;
-  } else {
-    if (flags.setBounds)
-      layer.bounds = bounds;
-    if (flags.setPosition)
-      layer.position = position;
-  }
 }
 
 - (void)applyToView:(UIView *)view setFrameDirectly:(BOOL)setFrameDirectly
@@ -668,9 +683,7 @@ static UIColor *defaultTintColor = nil;
   CALayer *layer = view.layer;
 
   ASPendingStateFlags flags = _flags;
-  if (flags.needsDisplay
+  if (__shouldSetNeedsDisplay(layer)) {
-      || (flags.setOpaque && opaque != view.opaque)
-      || (flags.setBackgroundColor && !CGColorEqualToColor(backgroundColor, layer.backgroundColor))) {
     [view setNeedsDisplay];
   }
 
@@ -683,11 +696,6 @@ static UIColor *defaultTintColor = nil;
   if (flags.setZPosition)
     layer.zPosition = zPosition;
 
-  // This should only be used for synchronous views wrapped by nodes.
-  if (flags.setFrame && !(flags.setBounds && flags.setPosition)) {
-    view.frame = frame;
-  }
-  
   if (flags.setBounds)
     view.bounds = bounds;
 
@@ -810,31 +818,16 @@ static UIColor *defaultTintColor = nil;
   if (flags.setAccessibilityIdentifier)
     view.accessibilityIdentifier = accessibilityIdentifier;
 
-  // FIXME: How should we reconcile order-of-operations between setting frame, bounds, position?
+  // For classes like ASTableNode, ASCollectionNode, ASScrollNode and similar - make sure UIView gets setFrame:
-  // Note we can't read bounds and position in the background, so we have to keep the frame
+  if (flags.setFrame && setFrameDirectly) {
-  // value intact until application time (now).
+    // Frame is only defined when transform is identity because we explicitly diverge from CALayer behavior and define frame without transform
-  if (flags.setFrame) {
-    if (setFrameDirectly) {
-      // For classes like ASTableNode, ASCollectionNode, ASScrollNode and similar - make sure UIView gets setFrame:
-
-      // Frame is only defined when transform is identity because we explicitly diverge from CALayer behavior and define frame without transform
 #if DEBUG
-      // Checking if the transform is identity is expensive, so disable when unnecessary. We have assertions on in Release, so DEBUG is the only way I know of.
+    // Checking if the transform is identity is expensive, so disable when unnecessary. We have assertions on in Release, so DEBUG is the only way I know of.
-      ASDisplayNodeAssert(CATransform3DIsIdentity(layer.transform), @"-[ASDisplayNode setFrame:] - self.transform must be identity in order to set the frame property.  (From Apple's UIView documentation: If the transform property is not the identity transform, the value of this property is undefined and therefore should be ignored.)");
+    ASDisplayNodeAssert(CATransform3DIsIdentity(layer.transform), @"-[ASDisplayNode setFrame:] - self.transform must be identity in order to set the frame property.  (From Apple's UIView documentation: If the transform property is not the identity transform, the value of this property is undefined and therefore should be ignored.)");
 #endif
-      view.frame = frame;
+    view.frame = frame;
-    } else {
-      CGRect _bounds;
-      CGPoint _position;
-      ASBoundsAndPositionForFrame(self.frame, layer.bounds.origin, layer.anchorPoint, &_bounds, &_position);
-      layer.bounds = _bounds;
-      layer.position = _position;
-    }
   } else {
-    if (flags.setBounds)
+    ASPendingStateApplyMetricsToLayer(self, layer);
-      layer.bounds = bounds;
-    if (flags.setPosition)
-      layer.position = position;
   }
 }
 

AsyncDisplayKitTests/ASBridgedPropertiesTests.mm

@@ -168,13 +168,13 @@ static inline void ASDispatchSyncOnOtherThread(dispatch_block_t block) {
 {
   ASPendingStateController *ctrl = [ASPendingStateController sharedInstance];
   ASDisplayNode *node = [ASDisplayNode new];
-  XCTAssertFalse(node.pendingViewState.hasChanges);
+  XCTAssertFalse(ASDisplayNodeGetPendingState(node).hasChanges);
   [node view];
   XCTAssertEqual(node.alpha, 1);
   node.alpha = 0;
   XCTAssertEqual(node.view.alpha, 0);
   XCTAssertEqual(node.alpha, 0);
-  XCTAssertFalse(node.pendingViewState.hasChanges);
+  XCTAssertFalse(ASDisplayNodeGetPendingState(node).hasChanges);
   XCTAssertFalse(ctrl.test_isFlushScheduled);
 }