Improve UIView & CALayer handling of -addSubnode:, and ensure node hierarchies are hooked up even when addSubview: is used directly.

AsyncDisplayKit/ASDisplayNode.mm

@@ -1225,17 +1225,24 @@ static NSInteger incrementIfFound(NSInteger i) {
   if (!_supernode)
     return;
 
+  // Check to ensure that our view or layer is actually inside of our supernode; otherwise, don't remove it.
+  // Though _ASDisplayView decouples the supernode if it is inserted inside another view hierarchy, this is
+  // more difficult to guarantee with _ASDisplayLayer because CoreAnimation doesn't have a -didMoveToSuperlayer.
+  BOOL shouldRemoveFromSuperviewOrSuperlayer = NO;
+  
+  if (self.nodeLoaded && _supernode.nodeLoaded) {
+    if (_flags.layerBacked) {
+      shouldRemoveFromSuperviewOrSuperlayer = (_layer.superlayer == _supernode.layer);
+    } else {
+      shouldRemoveFromSuperviewOrSuperlayer = (_view.superview == _supernode.view);
+    }
+  }
+
   // Do this before removing the view from the hierarchy, as the node will clear its supernode pointer when its view is removed from the hierarchy.
   [_supernode _removeSubnode:self];
 
-  if (ASDisplayNodeThreadIsMain()) {
+  if (shouldRemoveFromSuperviewOrSuperlayer) {
-    if (_flags.layerBacked) {
+    ASPerformBlockOnMainThread(^{
-      [_layer removeFromSuperlayer];
-    } else {
-      [_view removeFromSuperview];
-    }
-  } else {
-    dispatch_async(dispatch_get_main_queue(), ^{
       if (_flags.layerBacked) {
         [_layer removeFromSuperlayer];
       } else {
@@ -1301,7 +1308,7 @@ static NSInteger incrementIfFound(NSInteger i) {
     _flags.isEnteringHierarchy = NO;
 
     CALayer *layer = self.layer;
-    if (!self.layer.contents) {
+    if (!layer.contents) {
       [layer setNeedsDisplay];
     }
   }
@@ -2317,12 +2324,18 @@ static const char *ASDisplayNodeAssociatedNodeKey = "ASAssociatedNode";
 
 @implementation UIView (AsyncDisplayKit)
 
-- (void)addSubnode:(ASDisplayNode *)node
+- (void)addSubnode:(ASDisplayNode *)subnode
 {
-  if (node.layerBacked) {
+  if (subnode.layerBacked) {
-    [self.layer addSublayer:node.layer];
+    // Call -addSubnode: so that we use the asyncdisplaykit_node path if possible.
+    [self.layer addSubnode:subnode];
   } else {
-    [self addSubview:node.view];
+    ASDisplayNode *selfNode = self.asyncdisplaykit_node;
+    if (selfNode) {
+      [selfNode addSubnode:subnode];
+    } else {
+      [self addSubview:subnode.view];
+    }
   }
 }
 
@@ -2330,9 +2343,14 @@ static const char *ASDisplayNodeAssociatedNodeKey = "ASAssociatedNode";
 
 @implementation CALayer (AsyncDisplayKit)
 
-- (void)addSubnode:(ASDisplayNode *)node
+- (void)addSubnode:(ASDisplayNode *)subnode
 {
-  [self addSublayer:node.layer];
+  ASDisplayNode *selfNode = self.asyncdisplaykit_node;
+  if (selfNode) {
+    [selfNode addSubnode:subnode];
+  } else {
+    [self addSublayer:subnode.layer];
+  }
 }
 
 @end

AsyncDisplayKit/Details/_ASDisplayView.mm

@@ -63,6 +63,22 @@
   return self;
 }
 
+- (void)willMoveToWindow:(UIWindow *)newWindow
+{
+  BOOL visible = (newWindow != nil);
+  if (visible && !_node.inHierarchy) {
+    [_node __enterHierarchy];
+  }
+}
+
+- (void)didMoveToWindow
+{
+  BOOL visible = (self.window != nil);
+  if (!visible && _node.inHierarchy) {
+    [_node __exitHierarchy];
+  }
+}
+
 - (void)willMoveToSuperview:(UIView *)newSuperview
 {
   // Keep the node alive while the view is in a view hierarchy.  This helps ensure that async-drawing views can always
@@ -76,28 +92,53 @@
   else if (currentSuperview && !newSuperview) {
     self.keepalive_node = nil;
   }
+  
+  if (newSuperview) {
+    ASDisplayNode *supernode = _node.supernode;
+    BOOL supernodeLoaded = supernode.nodeLoaded;
+    ASDisplayNodeAssert(!supernode.isLayerBacked, @"Shouldn't be possible for _ASDisplayView's supernode to be layer-backed.");
+    
+    BOOL needsSupernodeUpdate = NO;
+
+    if (supernode) {
+      // If we have a supernode, compensate for users directly messing with views by updating to any new supernode.
+      needsSupernodeUpdate = (!supernodeLoaded || supernode.view != newSuperview);
+    } else {
+      // If we have no supernode and we are now in a view hierarchy, check to see if we can hook up to a supernode.
+      needsSupernodeUpdate = (newSuperview != nil);
     }
 
-- (void)willMoveToWindow:(UIWindow *)newWindow
+    if (needsSupernodeUpdate) {
-{
+      // -removeFromSupernode is called by -addSubnode:, if it is needed.
-  BOOL visible = newWindow != nil;
+      [newSuperview.asyncdisplaykit_node addSubnode:_node];
-  if (visible && !_node.inHierarchy) {
-    [_node __enterHierarchy];
-  } else if (!visible && _node.inHierarchy) {
-    [_node __exitHierarchy];
     }
   }
 
+}
+
 - (void)didMoveToSuperview
 {
-  // FIXME maybe move this logic into ASDisplayNode addSubnode/removeFromSupernode
+  ASDisplayNode *supernode = _node.supernode;
-  UIView *superview = self.superview;
+  ASDisplayNodeAssert(!supernode.isLayerBacked, @"Shouldn't be possible for superview's node to be layer-backed.");
   
-  // If superview's node is different from supernode's view, fix it by setting supernode to the new superview's node.  Got that?
+  if (supernode) {
-  if (!superview)
+    ASDisplayNodeAssertTrue(_node.nodeLoaded);
-    [_node __setSupernode:nil];
+    UIView *superview = self.superview;
-  else if (superview != _node.supernode.view)
+    BOOL supernodeLoaded = supernode.nodeLoaded;
-    [_node __setSupernode:superview.asyncdisplaykit_node];
+    BOOL needsSupernodeRemoval = NO;
+    
+    if (superview) {
+      // If our new superview is not the same as the supernode's view, or the supernode has no view, disconnect.
+      needsSupernodeRemoval = (!supernodeLoaded || supernode.view != superview);
+    } else {
+      // If supernode is loaded but our superview is nil, the user manually removed us, so disconnect supernode.
+      needsSupernodeRemoval = supernodeLoaded;
+    }
+    if (needsSupernodeRemoval) {
+      // The node will only disconnect from its supernode, not removeFromSuperview, in this condition.
+      [_node removeFromSupernode];
+    }
+  }
 }
 
 - (void)setNeedsDisplay