Improve Ancestry Handling, Avoid Assertion Failure (TextureGroup#246)

Adlai-Holler · web-flow · commit 0143e3291aeb · 2017-05-09T18:27:54.000-07:00
* Improve our handling of ancestry

* Increase chungalunga
diff --git a/AsyncDisplayKit.xcodeproj/project.pbxproj b/AsyncDisplayKit.xcodeproj/project.pbxproj
@@ -338,7 +338,7 @@
 		CC57EAF71E3939350034C595 /* ASCollectionView+Undeprecated.h in Headers */ = {isa = PBXBuildFile; fileRef = CC2E317F1DAC353700EEE891 /* ASCollectionView+Undeprecated.h */; settings = {ATTRIBUTES = (Private, ); }; };
 		CC57EAF81E3939450034C595 /* ASTableView+Undeprecated.h in Headers */ = {isa = PBXBuildFile; fileRef = CC512B841DAC45C60054848E /* ASTableView+Undeprecated.h */; settings = {ATTRIBUTES = (Private, ); }; };
 		CC58AA4B1E398E1D002C8CB4 /* ASBlockTypes.h in Headers */ = {isa = PBXBuildFile; fileRef = CC58AA4A1E398E1D002C8CB4 /* ASBlockTypes.h */; settings = {ATTRIBUTES = (Public, ); }; };
-		CC6AA2DA1E9F03B900978E87 /* ASDisplayNode+Ancestry.h in Headers */ = {isa = PBXBuildFile; fileRef = CC6AA2D81E9F03B900978E87 /* ASDisplayNode+Ancestry.h */; };
+		CC6AA2DA1E9F03B900978E87 /* ASDisplayNode+Ancestry.h in Headers */ = {isa = PBXBuildFile; fileRef = CC6AA2D81E9F03B900978E87 /* ASDisplayNode+Ancestry.h */; settings = {ATTRIBUTES = (Public, ); }; };
 		CC6AA2DB1E9F03B900978E87 /* ASDisplayNode+Ancestry.m in Sources */ = {isa = PBXBuildFile; fileRef = CC6AA2D91E9F03B900978E87 /* ASDisplayNode+Ancestry.m */; };
 		CC7FD9E11BB5F750005CCB2B /* ASPhotosFrameworkImageRequestTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CC7FD9E01BB5F750005CCB2B /* ASPhotosFrameworkImageRequestTests.m */; };
 		CC7FD9E21BB603FF005CCB2B /* ASPhotosFrameworkImageRequest.h in Headers */ = {isa = PBXBuildFile; fileRef = CC7FD9DC1BB5E962005CCB2B /* ASPhotosFrameworkImageRequest.h */; settings = {ATTRIBUTES = (Public, ); }; };
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,8 @@
 - Move more properties from ASTableView, ASCollectionView to their respective node classes. [Adlai Holler](https://github.com/Adlai-Holler)
 - Remove finalLayoutElement [Michael Schneider] (https://github.com/maicki)[#96](https://github.com/TextureGroup/Texture/pull/96)
 - Add ASPageTable - A map table for fast retrieval of objects within a certain page [Huy Nguyen](https://github.com/nguyenhuy)
+- Add new public `-supernodes`, `-supernodesIncludingSelf`, and `-supernodeOfClass:includingSelf:` methods. [Adlai Holler](https://github.com/Adlai-Holler)[#246](https://github.com/TextureGroup/Texture/pull/246)
+- Improve our handling supernode traversal to avoid loading layers and fix assertion failures you might hit in debug. [Adlai Holler](https://github.com/Adlai-Holler)[#246](https://github.com/TextureGroup/Texture/pull/246)
 - [ASDisplayNode] Pass drawParameter in rendering context callbacks [Michael Schneider](https://github.com/maicki)[#248](https://github.com/TextureGroup/Texture/pull/248)
 - [ASTextNode] Move to class method of drawRect:withParameters:isCancelled:isRasterizing: for drawing [Michael Schneider] (https://github.com/maicki)[#232](https://github.com/TextureGroup/Texture/pull/232)
 - [ASDisplayNode] Remove instance:-drawRect:withParameters:isCancelled:isRasterizing: (https://github.com/maicki)[#232](https://github.com/TextureGroup/Texture/pull/232)
diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm
@@ -17,6 +17,7 @@
 
 #import <AsyncDisplayKit/ASDisplayNodeInternal.h>
 
+#import <AsyncDisplayKit/ASDisplayNode+Ancestry.h>
 #import <AsyncDisplayKit/ASDisplayNode+FrameworkSubclasses.h>
 #import <AsyncDisplayKit/ASDisplayNode+Beta.h>
 #import <AsyncDisplayKit/ASDisplayNode+Deprecated.h>
@@ -600,9 +601,9 @@ - (CALayer *)_locked_layerToLoad
   return layer;
 }
 
-- (void)_locked_loadViewOrLayerIsLayerBacked:(BOOL)isLayerBacked
+- (void)_locked_loadViewOrLayer
 {
-  if (isLayerBacked) {
+  if (_flags.layerBacked) {
     TIME_SCOPED(_debugTimeToCreateView);
     _layer = [self _locked_layerToLoad];
     static int ASLayerDelegateAssociationKey;
@@ -692,7 +693,7 @@ - (UIView *)view
   
   // Loading a view needs to happen on the main thread
   ASDisplayNodeAssertMainThread();
-  [self _locked_loadViewOrLayerIsLayerBacked:isLayerBacked];
+  [self _locked_loadViewOrLayer];
   
   // FIXME: Ideally we'd call this as soon as the node receives -setNeedsLayout
   // but automatic subnode management would require us to modify the node tree
@@ -727,20 +728,13 @@ - (CALayer *)layer
     return _layer;
   }
   
-  BOOL isLayerBacked = _flags.layerBacked;
-  if (!isLayerBacked) {
-    // No need for the lock and call the view explicitly in case it needs to be loaded first
-    ASDN::MutexUnlocker u(__instanceLock__);
-    return self.view.layer;
-  }
-  
   if (![self _locked_shouldLoadViewOrLayer]) {
     return nil;
   }
   
   // Loading a layer needs to happen on the main thread
   ASDisplayNodeAssertMainThread();
-  [self _locked_loadViewOrLayerIsLayerBacked:isLayerBacked];
+  [self _locked_loadViewOrLayer];
   
   // FIXME: Ideally we'd call this as soon as the node receives -setNeedsLayout
   // but automatic subnode management would require us to modify the node tree
@@ -3909,8 +3903,8 @@ - (ASEventLog *)eventLog
     [result addObject:@{ @"frame" : [NSValue valueWithCGRect:_pendingViewState.frame] }];
   }
   
-  // Check supernode so that if we are cell node we don't find self.
-  ASCellNode *cellNode = ASDisplayNodeFindFirstSupernodeOfClass(self.supernode, [ASCellNode class]);
+  // Check supernode so that if we are a cell node we don't find self.
+  ASCellNode *cellNode = [self supernodeOfClass:[ASCellNode class] includingSelf:NO];
   if (cellNode != nil) {
     [result addObject:@{ @"cellNode" : ASObjectDescriptionMakeTiny(cellNode) }];
   }
diff --git a/Source/ASDisplayNodeExtras.h b/Source/ASDisplayNodeExtras.h
@@ -125,12 +125,12 @@ extern void ASDisplayNodePerformBlockOnEverySubnode(ASDisplayNode *node, BOOL tr
 /**
  Given a display node, traverses up the layer tree hierarchy, returning the first display node that passes block.
  */
-extern ASDisplayNode * _Nullable ASDisplayNodeFindFirstSupernode(ASDisplayNode * _Nullable node, BOOL (^block)(ASDisplayNode *node)) AS_WARN_UNUSED_RESULT;
+extern ASDisplayNode * _Nullable ASDisplayNodeFindFirstSupernode(ASDisplayNode * _Nullable node, BOOL (^block)(ASDisplayNode *node)) AS_WARN_UNUSED_RESULT ASDISPLAYNODE_DEPRECATED_MSG("Use the `supernodes` property instead.");
 
 /**
  Given a display node, traverses up the layer tree hierarchy, returning the first display node of kind class.
  */
-extern __kindof ASDisplayNode * _Nullable ASDisplayNodeFindFirstSupernodeOfClass(ASDisplayNode *start, Class c) AS_WARN_UNUSED_RESULT;
+extern __kindof ASDisplayNode * _Nullable ASDisplayNodeFindFirstSupernodeOfClass(ASDisplayNode *start, Class c) AS_WARN_UNUSED_RESULT  ASDISPLAYNODE_DEPRECATED_MSG("Use the `supernodeOfClass:includingSelf:` method instead.");
 
 /**
  * Given a layer, find the window it lives in, if any.
diff --git a/Source/ASDisplayNodeExtras.mm b/Source/ASDisplayNodeExtras.mm
@@ -18,6 +18,7 @@
 #import <AsyncDisplayKit/ASDisplayNodeExtras.h>
 #import <AsyncDisplayKit/ASDisplayNodeInternal.h>
 #import <AsyncDisplayKit/ASDisplayNode+FrameworkPrivate.h>
+#import <AsyncDisplayKit/ASDisplayNode+Ancestry.h>
 
 #import <queue>
 #import <AsyncDisplayKit/ASRunLoopQueue.h>
@@ -138,24 +139,21 @@ extern void ASDisplayNodePerformBlockOnEverySubnode(ASDisplayNode *node, BOOL tr
 
 ASDisplayNode *ASDisplayNodeFindFirstSupernode(ASDisplayNode *node, BOOL (^block)(ASDisplayNode *node))
 {
-  CALayer *layer = node.layer;
-
-  while (layer) {
-    node = ASLayerToDisplayNode(layer);
-    if (block(node)) {
-      return node;
+  // This function has historically started with `self` but the name suggests
+  // that it wouldn't. Perhaps we should change the behavior.
+  for (ASDisplayNode *ancestor in node.supernodesIncludingSelf) {
+    if (block(ancestor)) {
+      return ancestor;
     }
-    layer = layer.superlayer;
   }
-
   return nil;
 }
 
 __kindof ASDisplayNode *ASDisplayNodeFindFirstSupernodeOfClass(ASDisplayNode *start, Class c)
 {
-  return ASDisplayNodeFindFirstSupernode(start, ^(ASDisplayNode *n) {
-    return [n isKindOfClass:c];
-  });
+  // This function has historically started with `self` but the name suggests
+  // that it wouldn't. Perhaps we should change the behavior.
+  return [start supernodeOfClass:c includingSelf:YES];
 }
 
 static void _ASCollectDisplayNodes(NSMutableArray *array, CALayer *layer)
diff --git a/Source/AsyncDisplayKit.h b/Source/AsyncDisplayKit.h
@@ -17,6 +17,7 @@
 
 #import <AsyncDisplayKit/ASAvailability.h>
 #import <AsyncDisplayKit/ASDisplayNode.h>
+#import <AsyncDisplayKit/ASDisplayNode+Ancestry.h>
 #import <AsyncDisplayKit/ASDisplayNode+Convenience.h>
 #import <AsyncDisplayKit/ASDisplayNodeExtras.h>
 
diff --git a/Source/Base/ASDisplayNode+Ancestry.h b/Source/Base/ASDisplayNode+Ancestry.h
@@ -22,7 +22,36 @@ NS_ASSUME_NONNULL_BEGIN
 
 @interface ASDisplayNode (Ancestry)
 
-- (NSEnumerator *)ancestorEnumeratorWithSelf:(BOOL)includeSelf;
+/**
+ * Returns an object to enumerate the supernode ancestry of this node, starting with its supernode.
+ *
+ * For instance, you could write:
+ *   for (ASDisplayNode *node in self.supernodes) {
+ *     if ([node.backgroundColor isEqual:[UIColor blueColor]]) {
+ *       node.hidden = YES;
+ *     }
+ *   }
+ *
+ * Note: If this property is read on the main thread, the enumeration will attempt to go up
+ *  the layer hierarchy if it finds a break in the display node hierarchy.
+ */
+@property (atomic, readonly) id<NSFastEnumeration> supernodes;
+
+/**
+ * Same as `supernodes` but begins the enumeration with self.
+ */
+@property (atomic, readonly) id<NSFastEnumeration> supernodesIncludingSelf;
+
+/**
+ * Searches the supernodes of this node for one matching the given class.
+ *
+ * @param supernodeClass The class of node you're looking for.
+ * @param includeSelf Whether to include self in the search.
+ * @return A node of the given class that is an ancestor of this node, or nil.
+ *
+ * @note See the documentation on `supernodes` for details about the upward traversal.
+ */
+- (nullable __kindof ASDisplayNode *)supernodeOfClass:(Class)supernodeClass includingSelf:(BOOL)includeSelf;
 
 /**
  * e.g. "(<MYTextNode: 0xFFFF>, <MYTextContainingNode: 0xFFFF>, <MYCellNode: 0xFFFF>)"
diff --git a/Source/Base/ASDisplayNode+Ancestry.m b/Source/Base/ASDisplayNode+Ancestry.m
@@ -16,46 +16,80 @@
 //
 
 #import "ASDisplayNode+Ancestry.h"
+#import <AsyncDisplayKit/ASThread.h>
+#import <AsyncDisplayKit/ASDisplayNodeExtras.h>
 
 AS_SUBCLASSING_RESTRICTED
 @interface ASNodeAncestryEnumerator : NSEnumerator
 @end
 
 @implementation ASNodeAncestryEnumerator {
-  /// Would be nice to use __unsafe_unretained but nodes can be
-  /// deallocated on arbitrary threads so nope.
-  __weak ASDisplayNode * _nextNode;
+  ASDisplayNode *_lastNode; // This needs to be strong because enumeration will not retain the current batch of objects
+  BOOL _initialState;
 }
 
 - (instancetype)initWithNode:(ASDisplayNode *)node
 {
   if (self = [super init]) {
-    _nextNode = node;
+    _initialState = YES;
+    _lastNode = node;
   }
   return self;
 }
 
 - (id)nextObject
 {
-  ASDisplayNode *node = _nextNode;
-  _nextNode = [node supernode];
-  return node;
+  if (_initialState) {
+    _initialState = NO;
+    return _lastNode;
+  }
+
+  ASDisplayNode *nextNode = _lastNode.supernode;
+  if (nextNode == nil && ASDisplayNodeThreadIsMain()) {
+    CALayer *layer = _lastNode.nodeLoaded ? _lastNode.layer.superlayer : nil;
+    while (layer != nil) {
+      nextNode = ASLayerToDisplayNode(layer);
+      if (nextNode != nil) {
+        break;
+      }
+      layer = layer.superlayer;
+    }
+  }
+  _lastNode = nextNode;
+  return nextNode;
 }
 
 @end
 
 @implementation ASDisplayNode (Ancestry)
 
-- (NSEnumerator *)ancestorEnumeratorWithSelf:(BOOL)includeSelf
+- (id<NSFastEnumeration>)supernodes
+{
+  NSEnumerator *result = [[ASNodeAncestryEnumerator alloc] initWithNode:self];
+  [result nextObject]; // discard first object (self)
+  return result;
+}
+
+- (id<NSFastEnumeration>)supernodesIncludingSelf
 {
-  ASDisplayNode *node = includeSelf ? self : self.supernode;
-  return [[ASNodeAncestryEnumerator alloc] initWithNode:node];
+  return [[ASNodeAncestryEnumerator alloc] initWithNode:self];
+}
+
+- (nullable __kindof ASDisplayNode *)supernodeOfClass:(Class)supernodeClass includingSelf:(BOOL)includeSelf
+{
+  id<NSFastEnumeration> chain = includeSelf ? self.supernodesIncludingSelf : self.supernodes;
+  for (ASDisplayNode *ancestor in chain) {
+    if ([ancestor isKindOfClass:supernodeClass]) {
+      return ancestor;
+    }
+  }
+  return nil;
 }
 
 - (NSString *)ancestryDescription
 {
   NSMutableArray *strings = [NSMutableArray array];
-  for (ASDisplayNode *node in [self ancestorEnumeratorWithSelf:YES]) {
+  for (ASDisplayNode *node in self.supernodes) {
     [strings addObject:ASObjectDescriptionMakeTiny(node)];
   }
   return strings.description;
