Support surprisingly nice automatic behavior for the unsupported case of view-backed nodes as children of layer-backed ones.

Author: Scott Goodson

AsyncDisplayKit/ASDisplayNode.mm

@@ -996,13 +996,14 @@ - (void)_insertSubnode:(ASDisplayNode *)subnode atSubnodeIndex:(NSInteger)subnod
   if (isMovingEquivalentParents) {
     [subnode __incrementVisibilityNotificationsDisabled];
   }
+  
   [subnode removeFromSupernode];
-
+  [oldSubnode removeFromSupernode];
+  
   if (!_subnodes)
     _subnodes = [[NSMutableArray alloc] init];
-
-  [oldSubnode removeFromSupernode];
   [_subnodes insertObject:subnode atIndex:subnodeIndex];
+  [subnode __setSupernode:self];
 
   // Don't bother inserting the view/layer if in a rasterized subtree, because there are no layers in the hierarchy and none of this could possibly work.
   if (!_flags.shouldRasterizeDescendants && [self __shouldLoadViewOrLayer]) {
@@ -1030,8 +1031,6 @@ - (void)_insertSubnode:(ASDisplayNode *)subnode atSubnodeIndex:(NSInteger)subnod
   if (isMovingEquivalentParents) {
     [subnode __decrementVisibilityNotificationsDisabled];
   }
-
-  [subnode __setSupernode:self];
 }
 
 - (void)replaceSubnode:(ASDisplayNode *)oldSubnode withSubnode:(ASDisplayNode *)replacementSubnode
@@ -1231,7 +1230,7 @@ - (void)removeFromSupernode
   BOOL shouldRemoveFromSuperviewOrSuperlayer = NO;
 
   if (self.nodeLoaded && _supernode.nodeLoaded) {
-    if (_flags.layerBacked) {
+    if (_flags.layerBacked || _supernode.layerBacked) {
       shouldRemoveFromSuperviewOrSuperlayer = (_layer.superlayer == _supernode.layer);
     } else {
       shouldRemoveFromSuperviewOrSuperlayer = (_view.superview == _supernode.view);

AsyncDisplayKit/Details/_ASDisplayView.mm

@@ -101,8 +101,17 @@ - (void)willMoveToSuperview:(UIView *)newSuperview
     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);
+      if (supernodeLoaded) {
+        if (supernode.layerBacked) {
+          // See comment in -didMoveToSuperview.  This case should be avoided, but is possible with app-level coding errors.
+          needsSupernodeUpdate = (supernode.layer != newSuperview.layer);
+        } else {
+          // If we have a supernode, compensate for users directly messing with views by hitching up to any new supernode.
+          needsSupernodeUpdate = (supernode.view != newSuperview);
+        }
+      } else {
+        needsSupernodeUpdate = YES;
+      }
     } 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);
@@ -129,11 +138,23 @@ - (void)didMoveToSuperview
 
     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);
+      if (supernodeLoaded) {
+        if (supernode.layerBacked) {
+          // As asserted at the top, this shouldn't be possible, but in production with assertions disabled it can happen.
+          // We try to make such code behave as well as feasible because it's not that hard of an error to make if some deep
+          // child node of a layer-backed node happens to be view-backed, but it is not supported and should be avoided.
+          needsSupernodeRemoval = (supernode.layer != superview.layer);
+        } else {
+          needsSupernodeRemoval = (supernode.view != superview);
+        }
+      } else {
+        needsSupernodeRemoval = YES;
+      }
     } 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];

AsyncDisplayKitTests/ASDisplayNodeTests.m

@@ -1342,16 +1342,20 @@ - (void)testInsertSubviewAtIndexWithMeddlingViewsAndLayersViewBacked
   [parent insertSubnode:c belowSubnode:b];
   XCTAssertEqualObjects(orderStringFromSublayers(parent.layer), @"a,e,d,c,b", @"Didn't match");
 
-  XCTAssertEqual(3u, parent.subnodes.count, @"Should have the right subnode count");
+  XCTAssertEqual(4u, parent.subnodes.count, @"Should have the right subnode count");
   XCTAssertEqual(4u, parent.view.subviews.count, @"Should have the right subview count");
   XCTAssertEqual(5u, parent.layer.sublayers.count, @"Should have the right sublayer count");
+  
+  [e removeFromSuperlayer];
+  XCTAssertEqual(4u, parent.layer.sublayers.count, @"Should have the right sublayer count");
 
   //TODO: assert that things deallocate immediately and don't have latent autoreleases in here
   [parent release];
   [a release];
   [b release];
   [c release];
   [d release];
+  [e release];
 }
 
 - (void)testAppleBugInsertSubview
@@ -1415,11 +1419,11 @@ - (void)testInsertSubviewAtIndexWithMeddlingView
   [parent.view insertSubview:d aboveSubview:a.view];
   XCTAssertEqualObjects(orderStringFromSublayers(parent.layer), @"a,d,b", @"Didn't match");
 
-  // (a,e,d,b) => (a,d,>c<,b)
+  // (a,d,b) => (a,d,>c<,b)
   [parent insertSubnode:c belowSubnode:b];
   XCTAssertEqualObjects(orderStringFromSublayers(parent.layer), @"a,d,c,b", @"Didn't match");
 
-  XCTAssertEqual(3u, parent.subnodes.count, @"Should have the right subnode count");
+  XCTAssertEqual(4u, parent.subnodes.count, @"Should have the right subnode count");
   XCTAssertEqual(4u, parent.view.subviews.count, @"Should have the right subview count");
   XCTAssertEqual(4u, parent.layer.sublayers.count, @"Should have the right sublayer count");