diff --git a/AsyncDisplayKit/ASDisplayNode.mm b/AsyncDisplayKit/ASDisplayNode.mm index bb2b8ae253..ba7a5b402a 100644 --- a/AsyncDisplayKit/ASDisplayNode.mm +++ b/AsyncDisplayKit/ASDisplayNode.mm @@ -1468,7 +1468,7 @@ static inline CATransform3D _calculateTransformFromReferenceToTarget(ASDisplayNo } ASDisplayNodeAssert(_flags.layerBacked, @"We shouldn't get called back here if there is no layer"); - return (id)[NSNull null]; + return (id)kCFNull; } #pragma mark - Managing the Node Hierarchy diff --git a/AsyncDisplayKit/Details/_ASDisplayView.mm b/AsyncDisplayKit/Details/_ASDisplayView.mm index b0d1c3a8ab..3a38a674a3 100644 --- a/AsyncDisplayKit/Details/_ASDisplayView.mm +++ b/AsyncDisplayKit/Details/_ASDisplayView.mm @@ -78,16 +78,7 @@ UIView *currentSuperview = self.superview; if (!currentSuperview && newSuperview) { self.keepalive_node = _node; - } else if (currentSuperview && !newSuperview) { - // Clearing keepalive_node may cause deallocation of the node. In this case, __exitHierarchy may not have an opportunity (e.g. _node will be cleared - // by the time -didMoveToWindow occurs after this) to clear the Visible interfaceState, which we need to do before deallocation to meet an API guarantee. - if (_node.inHierarchy) { - [_node __exitHierarchy]; - } - self.keepalive_node = nil; } - - ASDisplayNodeAssert(self.keepalive_node == nil || newSuperview != nil, @"Keepalive reference should not exist if there is no superview."); if (newSuperview) { ASDisplayNode *supernode = _node.supernode; @@ -122,12 +113,21 @@ - (void)didMoveToSuperview { + UIView *superview = self.superview; + if (superview == nil) { + // Clearing keepalive_node may cause deallocation of the node. In this case, __exitHierarchy may not have an opportunity (e.g. _node will be cleared + // by the time -didMoveToWindow occurs after this) to clear the Visible interfaceState, which we need to do before deallocation to meet an API guarantee. + if (_node.inHierarchy) { + [_node __exitHierarchy]; + } + self.keepalive_node = nil; + } + ASDisplayNode *supernode = _node.supernode; ASDisplayNodeAssert(!supernode.isLayerBacked, @"Shouldn't be possible for superview's node to be layer-backed."); if (supernode) { ASDisplayNodeAssertTrue(_node.nodeLoaded); - UIView *superview = self.superview; BOOL supernodeLoaded = supernode.nodeLoaded; BOOL needsSupernodeRemoval = NO; diff --git a/AsyncDisplayKitTests/ASCollectionViewTests.mm b/AsyncDisplayKitTests/ASCollectionViewTests.mm index 760cd4d166..ac73aaeff3 100644 --- a/AsyncDisplayKitTests/ASCollectionViewTests.mm +++ b/AsyncDisplayKitTests/ASCollectionViewTests.mm @@ -348,6 +348,55 @@ } completion:nil]); } +/** + * https://github.com/facebook/AsyncDisplayKit/issues/2011 + * + * If this ever becomes a pain to maintain, drop it. The underlying issue is tested by testThatLayerBackedSubnodesAreMarkedInvisibleBeforeDeallocWhenSupernodesViewIsRemovedFromHierarchyWhileBeingRetained + */ +- (void)testThatDisappearingSupplementariesWithLayerBackedNodesDontFailAssert +{ + UIWindow *window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds]; + UICollectionViewLayout *layout = [[UICollectionViewFlowLayout alloc] init]; + ASCollectionView *cv = [[ASCollectionView alloc] initWithFrame:window.bounds collectionViewLayout:layout]; + + + __unused NSMutableSet *keepaliveNodes = [NSMutableSet set]; + id dataSource = [OCMockObject niceMockForProtocol:@protocol(ASCollectionDataSource)]; + static int nodeIdx = 0; + [[[dataSource stub] andDo:^(NSInvocation *invocation) { + __autoreleasing ASCellNode *suppNode = [[ASCellNode alloc] init]; + int thisNodeIdx = nodeIdx++; + suppNode.name = [NSString stringWithFormat:@"Cell #%d", thisNodeIdx]; + [keepaliveNodes addObject:suppNode]; + + ASDisplayNode *layerBacked = [[ASDisplayNode alloc] init]; + layerBacked.layerBacked = YES; + layerBacked.name = [NSString stringWithFormat:@"Subnode #%d", thisNodeIdx]; + [suppNode addSubnode:layerBacked]; + [invocation setReturnValue:&suppNode]; + }] collectionView:cv nodeForSupplementaryElementOfKind:UICollectionElementKindSectionHeader atIndexPath:OCMOCK_ANY]; + [[[dataSource stub] andReturnValue:[NSNumber numberWithInteger:1]] numberOfSectionsInCollectionView:cv]; + cv.asyncDataSource = dataSource; + + id delegate = [OCMockObject niceMockForProtocol:@protocol(UICollectionViewDelegateFlowLayout)]; + [[[delegate stub] andReturnValue:[NSValue valueWithCGSize:CGSizeMake(100, 100)]] collectionView:cv layout:OCMOCK_ANY referenceSizeForHeaderInSection:0]; + cv.asyncDelegate = delegate; + + [cv registerSupplementaryNodeOfKind:UICollectionElementKindSectionHeader]; + [window addSubview:cv]; + + [window makeKeyAndVisible]; + + for (NSInteger i = 0; i < 2; i++) { + // NOTE: waitUntilAllUpdatesAreCommitted or reloadDataImmediately is not sufficient here!! + XCTestExpectation *done = [self expectationWithDescription:[NSString stringWithFormat:@"Reload #%td complete", i]]; + [cv reloadDataWithCompletion:^{ + [done fulfill]; + }]; + [self waitForExpectationsWithTimeout:1 handler:nil]; + } + +} - (void)testThatNodeCalculatedSizesAreUpdatedBeforeFirstPrepareLayoutAfterRotation { diff --git a/AsyncDisplayKitTests/ASDisplayNodeTests.m b/AsyncDisplayKitTests/ASDisplayNodeTests.m index 1c07633bbe..0e148e2908 100644 --- a/AsyncDisplayKitTests/ASDisplayNodeTests.m +++ b/AsyncDisplayKitTests/ASDisplayNodeTests.m @@ -1941,4 +1941,27 @@ static bool stringContainsPointer(NSString *description, id p) { XCTAssertEqual(contentsAfterRedisplay, node.contents); } +// Underlying issue for: https://github.com/facebook/AsyncDisplayKit/issues/2011 +- (void)testThatLayerBackedSubnodesAreMarkedInvisibleBeforeDeallocWhenSupernodesViewIsRemovedFromHierarchyWhileBeingRetained +{ + UIWindow *window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds]; + + NS_VALID_UNTIL_END_OF_SCOPE UIView *nodeView = nil; + { + NS_VALID_UNTIL_END_OF_SCOPE ASDisplayNode *node = [[ASDisplayNode alloc] init]; + nodeView = node.view; + node.name = @"Node"; + + NS_VALID_UNTIL_END_OF_SCOPE ASDisplayNode *subnode = [[ASDisplayNode alloc] init]; + subnode.layerBacked = YES; + [node addSubnode:subnode]; + subnode.name = @"Subnode"; + + [window addSubview:nodeView]; + } + + // nodeView must continue to be retained across this call, but the nodes must not. + XCTAssertNoThrow([nodeView removeFromSuperview]); +} + @end