Initial commit to cleanup modifying subnode hierarchy (#2535)

AsyncDisplayKit/ASDisplayNode.mm

@@ -1641,8 +1641,7 @@ static inline CATransform3D _calculateTransformFromReferenceToTarget(ASDisplayNo
 
 #pragma mark - Managing the Node Hierarchy
 
-static bool disableNotificationsForMovingBetweenParents(ASDisplayNode *from, ASDisplayNode *to)
+ASDISPLAYNODE_INLINE bool shouldDisableNotificationsForMovingBetweenParents(ASDisplayNode *from, ASDisplayNode *to) {
-{
   if (!from || !to) return NO;
   if (from->_flags.synchronous) return NO;
   if (to->_flags.synchronous) return NO;
@@ -1650,6 +1649,114 @@ static bool disableNotificationsForMovingBetweenParents(ASDisplayNode *from, ASD
   return YES;
 }
 
+/// Returns incremented value of i if i is not NSNotFound
+ASDISPLAYNODE_INLINE NSInteger incrementIfFound(NSInteger i) {
+  return i == NSNotFound ? NSNotFound : i + 1;
+}
+
+/// Returns if a node is a member of a rasterized tree
+ASDISPLAYNODE_INLINE BOOL canUseViewAPI(ASDisplayNode *node, ASDisplayNode *subnode) {
+  return (subnode.isLayerBacked == NO && node.isLayerBacked == NO);
+}
+
+/// Returns if node is a member of a rasterized tree
+ASDISPLAYNODE_INLINE BOOL nodeIsInRasterizedTree(ASDisplayNode *node) {
+  return (node->_flags.shouldRasterizeDescendants || (node->_hierarchyState & ASHierarchyStateRasterized));
+}
+
+/*
+ * Central private helper method that should eventually be called if submethods add, insert or replace subnodes
+ * You must hold __instanceLock__ to call this.
+ *
+ * @param subnode       The subnode to insert
+ * @param subnodeIndex  The index in _subnodes to insert it
+ * @param viewSublayerIndex The index in layer.sublayers (not view.subviews) at which to insert the view (use if we can use the view API) otherwise pass NSNotFound
+ * @param sublayerIndex The index in layer.sublayers at which to insert the layer (use if either parent or subnode is layer-backed) otherwise pass NSNotFound
+ * @param oldSubnode Remove this subnode before inserting; ok to be nil if no removal is desired
+ */
+- (void)_insertSubnode:(ASDisplayNode *)subnode atSubnodeIndex:(NSInteger)subnodeIndex sublayerIndex:(NSInteger)sublayerIndex andRemoveSubnode:(ASDisplayNode *)oldSubnode
+{
+  if (subnode == nil || subnode == self) {
+    ASDisplayNodeFailAssert(@"Cannot insert a nil subnode or self as subnode");
+    return;
+  }
+  
+  if (subnodeIndex == NSNotFound) {
+    ASDisplayNodeFailAssert(@"Try to insert node on an index that was not found");
+    return;
+  }
+  
+  if (subnodeIndex > _subnodes.count || subnodeIndex < 0) {
+    ASDisplayNodeFailAssert(@"Cannot insert a subnode at index %zd. Count is %zd", subnodeIndex, _subnodes.count);
+    return;
+  }
+  
+  // Disable appearance methods during move between supernodes, but make sure we restore their state after we do our thing
+  ASDisplayNode *oldParent = subnode.supernode;
+  BOOL disableNotifications = shouldDisableNotificationsForMovingBetweenParents(oldParent, self);
+  if (disableNotifications) {
+    [subnode __incrementVisibilityNotificationsDisabled];
+  }
+  
+  [subnode _removeFromSupernode];
+  [oldSubnode _removeFromSupernode];
+  
+  if (_subnodes == nil) {
+    _subnodes = [[NSMutableArray alloc] init];
+  }
+  
+  ASDisplayNodeLogEvent(self, @"%@: %@", NSStringFromSelector(_cmd), subnode);
+    
+  [_subnodes insertObject:subnode atIndex:subnodeIndex];
+  
+  // This call will apply our .hierarchyState to the new subnode.
+  // If we are a managed hierarchy, as in ASCellNode trees, it will also apply our .interfaceState.
+  [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 (nodeIsInRasterizedTree(self) == NO && self.nodeLoaded) {
+    // If node is loaded insert the subnode otherwise wait until the node get's loaded
+    ASPerformBlockOnMainThread(^{
+      [self _insertSubnodeSubviewOrSublayer:subnode atIndex:sublayerIndex];
+    });
+  }
+
+  ASDisplayNodeAssert(disableNotifications == shouldDisableNotificationsForMovingBetweenParents(oldParent, self), @"Invariant violated");
+  if (disableNotifications) {
+    [subnode __decrementVisibilityNotificationsDisabled];
+  }
+}
+
+/*
+ * Inserts the view or layer of the given node at the given index
+ * You must hold __instanceLock__ to call this.
+ *
+ * @param subnode       The subnode to insert
+ * @param idx           The index in _view.subviews or _layer.sublayers at which to insert the subnode.view or
+ *                      subnode.layer of the subnode
+ */
+- (void)_insertSubnodeSubviewOrSublayer:(ASDisplayNode *)subnode atIndex:(NSInteger)idx
+{
+  ASDisplayNodeAssertMainThread();
+  ASDisplayNodeAssert(self.nodeLoaded, @"_insertSubnodeSubviewOrSublayer:atIndex: should never be called before our own view is created");
+
+  ASDisplayNodeAssert(idx != NSNotFound, @"Try to insert node on an index that was not found");
+  if (idx == NSNotFound) {
+    return;
+  }
+  
+  // If we can use view API, do. Due to an apple bug, -insertSubview:atIndex: actually wants a LAYER index, which we pass in
+  if (canUseViewAPI(self, subnode)) {
+    [_view insertSubview:subnode.view atIndex:idx];
+  } else {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wconversion"
+    [_layer insertSublayer:subnode.layer atIndex:idx];
+#pragma clang diagnostic pop
+  }
+}
+
 - (void)addSubnode:(ASDisplayNode *)subnode
 {
   // TODO: 2.0 Conversion: Reenable and fix within product code
@@ -1661,110 +1768,31 @@ static bool disableNotificationsForMovingBetweenParents(ASDisplayNode *from, ASD
 {
   ASDisplayNodeAssertThreadAffinity(self);
   ASDN::MutexLocker l(__instanceLock__);
-
+  
   ASDisplayNodeAssert(subnode, @"Cannot insert a nil subnode");
+    
+  // Don't add subnode if it's already if it's already a subnodes
   ASDisplayNode *oldParent = subnode.supernode;
   if (!subnode || subnode == self || oldParent == self) {
     return;
   }
 
-  // Disable appearance methods during move between supernodes, but make sure we restore their state after we do our thing
+  [self _insertSubnode:subnode atSubnodeIndex:_subnodes.count sublayerIndex:_layer.sublayers.count andRemoveSubnode:nil];
-  BOOL isMovingEquivalentParents = disableNotificationsForMovingBetweenParents(oldParent, self);
+}
-  if (isMovingEquivalentParents) {
-    [subnode __incrementVisibilityNotificationsDisabled];
-  }
-  [subnode _removeFromSupernode];
 
-  if (!_subnodes) {
+- (void)_addSubnodeViewsAndLayers
-    _subnodes = [[NSMutableArray alloc] init];
+{
-  }
+  for (ASDisplayNode *node in [_subnodes copy]) {
-
+    [self _addSubnodeSubviewOrSublayer:node];
-  ASDisplayNodeLogEvent(self, @"%@ %@", NSStringFromSelector(_cmd), subnode);
-  [_subnodes addObject:subnode];
-  
-  // This call will apply our .hierarchyState to the new subnode.
-  // If we are a managed hierarchy, as in ASCellNode trees, it will also apply our .interfaceState.
-  [subnode __setSupernode:self];
-  
-  if (self.nodeLoaded) {
-    // If this node has a view or layer, force the subnode to also create its view or layer and add it to the hierarchy here.
-    // Otherwise there is no way for the subnode's view or layer to enter the hierarchy, except recursing down all
-    // subnodes on the main thread after the node tree has been created but before the first display (which
-    // could introduce performance problems).
-    ASPerformBlockOnMainThread(^{
-      [self _addSubnodeSubviewOrSublayer:subnode];
-    });
-  }
-
-  ASDisplayNodeAssert(isMovingEquivalentParents == disableNotificationsForMovingBetweenParents(oldParent, self), @"Invariant violated");
-  if (isMovingEquivalentParents) {
-    [subnode __decrementVisibilityNotificationsDisabled];
   }
 }
 
-/*
+- (void)_addSubnodeSubviewOrSublayer:(ASDisplayNode *)subnode
- Private helper function.
- You must hold __instanceLock__ to call this.
-
- @param subnode       The subnode to insert
- @param subnodeIndex  The index in _subnodes to insert it
- @param viewSublayerIndex The index in layer.sublayers (not view.subviews) at which to insert the view (use if we can use the view API) otherwise pass NSNotFound
- @param sublayerIndex The index in layer.sublayers at which to insert the layer (use if either parent or subnode is layer-backed) otherwise pass NSNotFound
- @param oldSubnode Remove this subnode before inserting; ok to be nil if no removal is desired
- */
-- (void)_insertSubnode:(ASDisplayNode *)subnode atSubnodeIndex:(NSInteger)subnodeIndex sublayerIndex:(NSInteger)sublayerIndex andRemoveSubnode:(ASDisplayNode *)oldSubnode
 {
-  if (subnodeIndex == NSNotFound) {
+    // Due to a bug in Apple's framework we have to use the layer index to insert a subview
-    return;
+    // so just use th ecount of the sublayers to add the subnode
-  }
+  NSInteger idx = _layer.sublayers.count;
-  
+  [self _insertSubnodeSubviewOrSublayer:subnode atIndex:idx];
-  ASDisplayNodeAssert(subnode, @"Cannot insert a nil subnode");
-  if (!subnode) {
-    return;
-  }
-
-  ASDisplayNode *oldParent = [subnode _deallocSafeSupernode];
-  // Disable appearance methods during move between supernodes, but make sure we restore their state after we do our thing
-  BOOL isMovingEquivalentParents = disableNotificationsForMovingBetweenParents(oldParent, self);
-  if (isMovingEquivalentParents) {
-    [subnode __incrementVisibilityNotificationsDisabled];
-  }
-  
-  [subnode _removeFromSupernode];
-  [oldSubnode _removeFromSupernode];
-  
-  if (!_subnodes)
-    _subnodes = [[NSMutableArray alloc] init];
-  ASDisplayNodeLogEvent(self, @"%@: %@", NSStringFromSelector(_cmd), subnode);
-  [_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]) {
-    if (_layer) {
-      ASDisplayNodeCAssertMainThread();
-
-      ASDisplayNodeAssert(sublayerIndex != NSNotFound, @"Should pass either a valid sublayerIndex");
-
-      if (sublayerIndex != NSNotFound) {
-        BOOL canUseViewAPI = !subnode.isLayerBacked && !self.isLayerBacked;
-        // If we can use view API, do. Due to an apple bug, -insertSubview:atIndex: actually wants a LAYER index, which we pass in
-        if (canUseViewAPI && sublayerIndex != NSNotFound) {
-          [_view insertSubview:subnode.view atIndex:sublayerIndex];
-        } else if (sublayerIndex != NSNotFound) {
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wconversion"
-          [_layer insertSublayer:subnode.layer atIndex:sublayerIndex];
-#pragma clang diagnostic pop
-        }
-      }
-    }
-  }
-
-  ASDisplayNodeAssert(isMovingEquivalentParents == disableNotificationsForMovingBetweenParents(oldParent, self), @"Invariant violated");
-  if (isMovingEquivalentParents) {
-    [subnode __decrementVisibilityNotificationsDisabled];
-  }
 }
 
 - (void)replaceSubnode:(ASDisplayNode *)oldSubnode withSubnode:(ASDisplayNode *)replacementSubnode
@@ -1779,31 +1807,37 @@ static bool disableNotificationsForMovingBetweenParents(ASDisplayNode *from, ASD
   ASDisplayNodeAssertThreadAffinity(self);
   ASDN::MutexLocker l(__instanceLock__);
 
-  if (!replacementSubnode || [oldSubnode _deallocSafeSupernode] != self) {
+  if (replacementSubnode == nil) {
-    ASDisplayNodeAssert(0, @"Bad use of api. Invalid subnode to replace async.");
+    ASDisplayNodeFailAssert(@"Invalid subnode to replace");
+    return;
+  }
+  
+  if ([oldSubnode _deallocSafeSupernode] != self) {
+    ASDisplayNodeFailAssert(@"Old Subnode to replace must be a subnode");
     return;
   }
 
-  ASDisplayNodeAssert(!(self.nodeLoaded && !oldSubnode.nodeLoaded), @"ASDisplayNode corruption bug. We have view loaded, but child node does not.");
+  ASDisplayNodeAssert(!(self.nodeLoaded && !oldSubnode.nodeLoaded), @"We have view loaded, but child node does not.");
   ASDisplayNodeAssert(_subnodes, @"You should have subnodes if you have a subnode");
 
   NSInteger subnodeIndex = [_subnodes indexOfObjectIdenticalTo:oldSubnode];
   NSInteger sublayerIndex = NSNotFound;
 
-  if (_layer) {
+  // Don't bother figuring out the sublayerIndex if in a rasterized subtree, because there are no layers in the
-    sublayerIndex = [_layer.sublayers indexOfObjectIdenticalTo:oldSubnode.layer];
+  // hierarchy and none of this could possibly work.
-    ASDisplayNodeAssert(sublayerIndex != NSNotFound, @"Somehow oldSubnode's supernode is self, yet we could not find it in our layers to replace");
+  if (nodeIsInRasterizedTree(self) == NO) {
-    if (sublayerIndex == NSNotFound) return;
+    if (_layer) {
+      sublayerIndex = [_layer.sublayers indexOfObjectIdenticalTo:oldSubnode.layer];
+      ASDisplayNodeAssert(sublayerIndex != NSNotFound, @"Somehow oldSubnode's supernode is self, yet we could not find it in our layers to replace");
+      if (sublayerIndex == NSNotFound) {
+        return;
+      }
+    }
   }
 
   [self _insertSubnode:replacementSubnode atSubnodeIndex:subnodeIndex sublayerIndex:sublayerIndex andRemoveSubnode:oldSubnode];
 }
 
-// This is just a convenience to avoid a bunch of conditionals
-static NSInteger incrementIfFound(NSInteger i) {
-  return i == NSNotFound ? NSNotFound : i + 1;
-}
-
 - (void)insertSubnode:(ASDisplayNode *)subnode belowSubnode:(ASDisplayNode *)below
 {
   // TODO: 2.0 Conversion: Reenable and fix within product code
@@ -1816,13 +1850,13 @@ static NSInteger incrementIfFound(NSInteger i) {
   ASDisplayNodeAssertThreadAffinity(self);
   ASDN::MutexLocker l(__instanceLock__);
 
-  ASDisplayNodeAssert(subnode, @"Cannot insert a nil subnode");
+  if (subnode == nil) {
-  if (!subnode) {
+    ASDisplayNodeFailAssert(@"Cannot insert a nil subnode");
     return;
   }
 
-  ASDisplayNodeAssert([below _deallocSafeSupernode] == self, @"Node to insert below must be a subnode");
   if ([below _deallocSafeSupernode] != self) {
+    ASDisplayNodeFailAssert(@"Node to insert below must be a subnode");
     return;
   }
 
@@ -1831,22 +1865,31 @@ static NSInteger incrementIfFound(NSInteger i) {
   NSInteger belowSubnodeIndex = [_subnodes indexOfObjectIdenticalTo:below];
   NSInteger belowSublayerIndex = NSNotFound;
 
-  if (_layer) {
+  
-    belowSublayerIndex = [_layer.sublayers indexOfObjectIdenticalTo:below.layer];
+  // Don't bother figuring out the sublayerIndex if in a rasterized subtree, because there are no layers in the
-    ASDisplayNodeAssert(belowSublayerIndex != NSNotFound, @"Somehow below's supernode is self, yet we could not find it in our layers to reference");
+  // hierarchy and none of this could possibly work.
-    if (belowSublayerIndex == NSNotFound)
+  if (nodeIsInRasterizedTree(self) == NO) {
-      return;
-  }
-  // If the subnode is already in the subnodes array / sublayers and it's before the below node, removing it to insert it will mess up our calculation
-  if ([subnode _deallocSafeSupernode] == self) {
-    NSInteger currentIndexInSubnodes = [_subnodes indexOfObjectIdenticalTo:subnode];
-    if (currentIndexInSubnodes < belowSubnodeIndex) {
-      belowSubnodeIndex--;
-    }
     if (_layer) {
-      NSInteger currentIndexInSublayers = [_layer.sublayers indexOfObjectIdenticalTo:subnode.layer];
+      belowSublayerIndex = [_layer.sublayers indexOfObjectIdenticalTo:below.layer];
-      if (currentIndexInSublayers < belowSublayerIndex) {
+      ASDisplayNodeAssert(belowSublayerIndex != NSNotFound, @"Somehow below's supernode is self, yet we could not find it in our layers to reference");
-        belowSublayerIndex--;
+      if (belowSublayerIndex == NSNotFound)
+        return;
+    }
+    
+    ASDisplayNodeAssert(belowSubnodeIndex != NSNotFound, @"Couldn't find above in subnodes");
+    
+    // If the subnode is already in the subnodes array / sublayers and it's before the below node, removing it to
+    // insert it will mess up our calculation
+    if ([subnode _deallocSafeSupernode] == self) {
+      NSInteger currentIndexInSubnodes = [_subnodes indexOfObjectIdenticalTo:subnode];
+      if (currentIndexInSubnodes < belowSubnodeIndex) {
+        belowSubnodeIndex--;
+      }
+      if (_layer) {
+        NSInteger currentIndexInSublayers = [_layer.sublayers indexOfObjectIdenticalTo:subnode.layer];
+        if (currentIndexInSublayers < belowSublayerIndex) {
+          belowSublayerIndex--;
+        }
       }
     }
   }
@@ -1868,13 +1911,13 @@ static NSInteger incrementIfFound(NSInteger i) {
   ASDisplayNodeAssertThreadAffinity(self);
   ASDN::MutexLocker l(__instanceLock__);
 
-  ASDisplayNodeAssert(subnode, @"Cannot insert a nil subnode");
+  if (subnode == nil) {
-  if (!subnode) {
+    ASDisplayNodeFailAssert(@"Cannot insert a nil subnode");
     return;
   }
 
-  ASDisplayNodeAssert([above _deallocSafeSupernode] == self, @"Node to insert above must be a subnode");
   if ([above _deallocSafeSupernode] != self) {
+    ASDisplayNodeFailAssert(@"Node to insert above must be a subnode");
     return;
   }
 
@@ -1883,17 +1926,20 @@ static NSInteger incrementIfFound(NSInteger i) {
   NSInteger aboveSubnodeIndex = [_subnodes indexOfObjectIdenticalTo:above];
   NSInteger aboveSublayerIndex = NSNotFound;
 
-  // Don't bother figuring out the sublayerIndex if in a rasterized subtree, because there are no layers in the hierarchy and none of this could possibly work.
+  // Don't bother figuring out the sublayerIndex if in a rasterized subtree, because there are no layers in the
-  if (!_flags.shouldRasterizeDescendants && [self __shouldLoadViewOrLayer]) {
+  // hierarchy and none of this could possibly work.
+  if (nodeIsInRasterizedTree(self) == NO) {
     if (_layer) {
       aboveSublayerIndex = [_layer.sublayers indexOfObjectIdenticalTo:above.layer];
       ASDisplayNodeAssert(aboveSublayerIndex != NSNotFound, @"Somehow above's supernode is self, yet we could not find it in our layers to replace");
       if (aboveSublayerIndex == NSNotFound)
         return;
     }
+    
     ASDisplayNodeAssert(aboveSubnodeIndex != NSNotFound, @"Couldn't find above in subnodes");
 
-    // If the subnode is already in the subnodes array / sublayers and it's before the below node, removing it to insert it will mess up our calculation
+    // If the subnode is already in the subnodes array / sublayers and it's before the below node, removing it to
+    // insert it will mess up our calculation
     if ([subnode _deallocSafeSupernode] == self) {
       NSInteger currentIndexInSubnodes = [_subnodes indexOfObjectIdenticalTo:subnode];
       if (currentIndexInSubnodes <= aboveSubnodeIndex) {
@@ -1922,57 +1968,36 @@ static NSInteger incrementIfFound(NSInteger i) {
 {
   ASDisplayNodeAssertThreadAffinity(self);
   ASDN::MutexLocker l(__instanceLock__);
+  
+  if (subnode == nil) {
+    ASDisplayNodeFailAssert(@"Cannot insert a nil subnode");
+    return;
+  }
 
   if (idx > _subnodes.count || idx < 0) {
     ASDisplayNodeFailAssert(@"Cannot insert a subnode at index %zd. Count is %zd", idx, _subnodes.count);
     return;
   }
-
-  if (subnode == nil) {
-    ASDisplayNodeFailAssert(@"Attempt to insert a nil subnode into node %@", self);
-    return;
-  }
   
   NSInteger sublayerIndex = NSNotFound;
 
-  // Account for potentially having other subviews
+  // Don't bother figuring out the sublayerIndex if in a rasterized subtree, because there are no layers in the
-  if (_layer && idx == 0) {
+  // hierarchy and none of this could possibly work.
-    sublayerIndex = 0;
+  if (nodeIsInRasterizedTree(self) == NO) {
-  } else if (_layer) {
+    // Account for potentially having other subviews
-    ASDisplayNode *positionInRelationTo = (_subnodes.count > 0 && idx > 0) ? _subnodes[idx - 1] : nil;
+    if (_layer && idx == 0) {
-    if (positionInRelationTo) {
+      sublayerIndex = 0;
-      sublayerIndex = incrementIfFound([_layer.sublayers indexOfObjectIdenticalTo:positionInRelationTo.layer]);
+    } else if (_layer) {
+      ASDisplayNode *positionInRelationTo = (_subnodes.count > 0 && idx > 0) ? _subnodes[idx - 1] : nil;
+      if (positionInRelationTo) {
+        sublayerIndex = incrementIfFound([_layer.sublayers indexOfObjectIdenticalTo:positionInRelationTo.layer]);
+      }
     }
   }
 
   [self _insertSubnode:subnode atSubnodeIndex:idx sublayerIndex:sublayerIndex andRemoveSubnode:nil];
 }
 
-
-- (void)_addSubnodeSubviewOrSublayer:(ASDisplayNode *)subnode
-{
-  ASDisplayNodeAssertMainThread();
-  ASDisplayNodeAssert(self.nodeLoaded, @"_addSubnodeSubview: should never be called before our own view is created");
-
-  BOOL canUseViewAPI = !self.isLayerBacked && !subnode.isLayerBacked;
-  if (canUseViewAPI) {
-    [_view addSubview:subnode.view];
-  } else {
-    // Disallow subviews in a layer-backed node
-    ASDisplayNodeAssert(subnode.isLayerBacked, @"Cannot add a subview to a layer-backed node; only sublayers permitted.");
-    [_layer addSublayer:subnode.layer];
-  }
-}
-
-- (void)_addSubnodeViewsAndLayers
-{
-  ASDisplayNodeAssertMainThread();
-
-  for (ASDisplayNode *node in [_subnodes copy]) {
-    [self _addSubnodeSubviewOrSublayer:node];
-  }
-}
-
 - (void)_removeSubnode:(ASDisplayNode *)subnode
 {
   ASDisplayNodeAssertThreadAffinity(self);