Merge pull request #1020 from facebook/RangeDeletions

appleguy · appleguy · commit 5a7772a0ae67 · 2015-12-30T23:07:32.000-08:00
Ensure that ASRangeController immediately removes any deleted nodes from its range state.
diff --git a/AsyncDisplayKit/ASCollectionView.mm b/AsyncDisplayKit/ASCollectionView.mm
@@ -927,4 +927,23 @@ - (void)clearFetchedData
   }
 }
 
+#pragma mark - UICollectionView dead-end intercepts
+
+#if ASDISPLAYNODE_ASSERTIONS_ENABLED // Remove implementations entirely for efficiency if not asserting.
+
+// intercepted due to not being supported by ASCollectionView (prevent bugs caused by usage)
+
+- (BOOL)collectionView:(UICollectionView *)collectionView canMoveItemAtIndexPath:(NSIndexPath *)indexPath NS_AVAILABLE_IOS(9_0)
+{
+  ASDisplayNodeAssert(![self.asyncDataSource respondsToSelector:_cmd], @"%@ is not supported by ASCollectionView - please remove or disable this data source method.", NSStringFromSelector(_cmd));
+  return NO;
+}
+
+- (void)collectionView:(UICollectionView *)collectionView moveItemAtIndexPath:(NSIndexPath *)sourceIndexPath toIndexPath:(NSIndexPath*)destinationIndexPath NS_AVAILABLE_IOS(9_0)
+{
+  ASDisplayNodeAssert(![self.asyncDataSource respondsToSelector:_cmd], @"%@ is not supported by ASCollectionView - please remove or disable this data source method.", NSStringFromSelector(_cmd));
+}
+
+#endif
+
 @end
diff --git a/AsyncDisplayKit/ASPagerNode.h b/AsyncDisplayKit/ASPagerNode.h
@@ -17,27 +17,24 @@
 - (ASCellNode *)pagerNode:(ASPagerNode *)pagerNode nodeAtIndex:(NSInteger)index;
 @end
 
-// WARNING: ASPagerNode is new in AsyncDisplayKit 1.9.4 and not yet widely tested.
-// Details of its API or behavior may change in future releases
 @interface ASPagerNode : ASCollectionNode
 
 // Configures a default horizontal, paging flow layout with 0 inter-item spacing.
 - (instancetype)init;
 
 // Initializer with custom-configured flow layout properties.
-- (instancetype)initWithFlowLayout:(UICollectionViewFlowLayout *)flowLayout;
+- (instancetype)initWithCollectionViewLayout:(UICollectionViewFlowLayout *)flowLayout;
 
-// The underlying ASCollectionView object.
-@property (nonatomic, readonly) ASCollectionView *view;
+// Data Source is required, and uses a different protocol from ASCollectionNode.
+- (void)setDataSource:(id <ASPagerNodeDataSource>)dataSource;
+- (id <ASPagerNodeDataSource>)dataSource;
 
 // Delegate is optional, and uses the same protocol as ASCollectionNode.
 // This includes UIScrollViewDelegate as well as most methods from UICollectionViewDelegate, like willDisplay...
 @property (nonatomic, weak) id <ASCollectionDelegate> delegate;
 
-// Data Source is required, and uses a different protocol from ASCollectionNode.
-//@property (nonatomic, weak) id <ASPagerNodeDataSource> dataSource;
-- (void)setDataSource:(id <ASPagerNodeDataSource>)dataSource;
-- (id <ASPagerNodeDataSource>)dataSource;
+// The underlying ASCollectionView object.
+@property (nonatomic, readonly) ASCollectionView *view;
 
 - (void)scrollToPageAtIndex:(NSInteger)index animated:(BOOL)animated;
 
diff --git a/AsyncDisplayKit/ASPagerNode.m b/AsyncDisplayKit/ASPagerNode.m
@@ -29,17 +29,12 @@ - (instancetype)init
   flowLayout.minimumInteritemSpacing = 0;
   flowLayout.minimumLineSpacing = 0;
   
-  return [self initWithFlowLayout:flowLayout];
+  return [self initWithCollectionViewLayout:flowLayout];
 }
 
-- (instancetype)initWithCollectionViewLayout:(UICollectionViewLayout *)layout
-{
-  ASDisplayNodeAssert([layout isKindOfClass:[UICollectionViewFlowLayout class]], @"ASPagerNode requires a flow layout.");
-  return [self initWithFlowLayout:(UICollectionViewFlowLayout *)layout];
-}
-
-- (instancetype)initWithFlowLayout:(UICollectionViewFlowLayout *)flowLayout
+- (instancetype)initWithCollectionViewLayout:(UICollectionViewFlowLayout *)flowLayout;
 {
+  ASDisplayNodeAssert([flowLayout isKindOfClass:[UICollectionViewFlowLayout class]], @"ASPagerNode requires a flow layout.");
   self = [super initWithCollectionViewLayout:flowLayout];
   if (self != nil) {
     _flowLayout = flowLayout;
diff --git a/AsyncDisplayKit/Details/ASDelegateProxy.h b/AsyncDisplayKit/Details/ASDelegateProxy.h
@@ -9,7 +9,7 @@
 #import <Foundation/Foundation.h>
 
 @class ASDelegateProxy;
-@protocol ASDelegateProxyInterceptor
+@protocol ASDelegateProxyInterceptor <NSObject>
 @required
 // Called if the target object is discovered to be nil if it had been non-nil at init time.
 // This happens if the object is deallocated, because the proxy must maintain a weak reference to avoid cycles.
@@ -25,7 +25,7 @@
 
 @interface ASDelegateProxy : NSProxy
 
-- (instancetype)initWithTarget:(id<NSObject>)target interceptor:(id <ASDelegateProxyInterceptor>)interceptor;
+- (instancetype)initWithTarget:(id <NSObject>)target interceptor:(id <ASDelegateProxyInterceptor>)interceptor;
 
 // This method must be overridden by a subclass.
 - (BOOL)interceptsSelector:(SEL)selector;
diff --git a/AsyncDisplayKit/Details/ASDelegateProxy.m b/AsyncDisplayKit/Details/ASDelegateProxy.m
@@ -54,7 +54,11 @@ - (BOOL)interceptsSelector:(SEL)selector
           selector == @selector(collectionView:didEndDisplayingCell:forItemAtIndexPath:) ||
           
           // used for batch fetching API
-          selector == @selector(scrollViewWillEndDragging:withVelocity:targetContentOffset:)
+          selector == @selector(scrollViewWillEndDragging:withVelocity:targetContentOffset:) ||
+          
+          // intercepted due to not being supported by ASCollectionView (prevent bugs caused by usage)
+          selector == @selector(collectionView:canMoveItemAtIndexPath:) ||
+          selector == @selector(collectionView:moveItemAtIndexPath:toIndexPath:)
           );
 }
 
@@ -97,7 +101,7 @@ - (instancetype)initWithTarget:(id <NSObject>)target interceptor:(id <ASDelegate
 - (BOOL)respondsToSelector:(SEL)aSelector
 {
   if ([self interceptsSelector:aSelector]) {
-    return (_interceptor != nil);
+    return [_interceptor respondsToSelector:aSelector];
   } else {
     // Also return NO if _target has become nil due to zeroing weak reference (or placeholder initialization).
     return [_target respondsToSelector:aSelector];
diff --git a/AsyncDisplayKit/Details/ASLayoutRangeType.h b/AsyncDisplayKit/Details/ASLayoutRangeType.h
@@ -9,7 +9,7 @@
 #import <Foundation/Foundation.h>
 
 typedef NS_ENUM(NSInteger, ASLayoutRangeType) {
-  ASLayoutRangeTypeVisible,
+  ASLayoutRangeTypeVisible = 0,
   ASLayoutRangeTypeRender,
   ASLayoutRangeTypePreload,
   ASLayoutRangeTypeCount
diff --git a/AsyncDisplayKit/Details/ASRangeController.mm b/AsyncDisplayKit/Details/ASRangeController.mm
@@ -130,7 +130,8 @@ - (void)_updateVisibleNodeIndexPaths
                                                           rangeType:rangeType];
 
       // Notify to remove indexpaths that are leftover that are not visible or included in the _layoutController calculated paths
-      NSMutableSet *removedIndexPaths = _rangeIsValid ? [_rangeTypeIndexPaths[rangeKey] mutableCopy] : [NSMutableSet set];
+      // This value may be nil for the first call of this method.
+      NSMutableSet *removedIndexPaths = [_rangeTypeIndexPaths[rangeKey] mutableCopy];
       [removedIndexPaths minusSet:indexPaths];
       [removedIndexPaths minusSet:visibleNodePathsSet];
 
@@ -176,61 +177,76 @@ - (BOOL)shouldSkipVisibleNodesForRangeType:(ASLayoutRangeType)rangeType
 
 #pragma mark - ASDataControllerDelegete
 
-- (void)dataControllerBeginUpdates:(ASDataController *)dataController {
+- (void)dataControllerBeginUpdates:(ASDataController *)dataController
+{
   ASPerformBlockOnMainThread(^{
     [_delegate didBeginUpdatesInRangeController:self];
   });
 }
 
-- (void)dataController:(ASDataController *)dataController endUpdatesAnimated:(BOOL)animated completion:(void (^)(BOOL))completion {
+- (void)dataController:(ASDataController *)dataController endUpdatesAnimated:(BOOL)animated completion:(void (^)(BOOL))completion
+{
   ASPerformBlockOnMainThread(^{
     [_delegate rangeController:self didEndUpdatesAnimated:animated completion:completion];
   });
 }
 
-- (void)dataController:(ASDataController *)dataController didInsertNodes:(NSArray *)nodes atIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions {
+- (void)dataController:(ASDataController *)dataController didInsertNodes:(NSArray *)nodes atIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions
+{
   ASDisplayNodeAssert(nodes.count == indexPaths.count, @"Invalid index path");
-
-  NSMutableArray *nodeSizes = [NSMutableArray arrayWithCapacity:nodes.count];
-  [nodes enumerateObjectsUsingBlock:^(ASCellNode *node, NSUInteger idx, BOOL *stop) {
-    [nodeSizes addObject:[NSValue valueWithCGSize:node.calculatedSize]];
-  }];
-
   ASPerformBlockOnMainThread(^{
     _rangeIsValid = NO;
     [_delegate rangeController:self didInsertNodes:nodes atIndexPaths:indexPaths withAnimationOptions:animationOptions];
   });
 }
 
-- (void)dataController:(ASDataController *)dataController didDeleteNodes:(NSArray *)nodes atIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions {
+- (void)dataController:(ASDataController *)dataController didDeleteNodes:(NSArray *)nodes atIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions
+{
   ASPerformBlockOnMainThread(^{
     _rangeIsValid = NO;
+    
+    // When removing nodes we need to make sure that removed indexPaths are not left in _rangeTypeIndexPaths,
+    // otherwise _updateVisibleNodeIndexPaths may try to retrieve nodes from dataSource that aren't there anymore
+    for (NSInteger i = 0; i < ASLayoutRangeTypeCount; i++) {
+      id rangeKey = @((ASLayoutRangeType)i);
+      NSMutableSet *rangePaths = [_rangeTypeIndexPaths[rangeKey] mutableCopy];
+      for (NSIndexPath *path in indexPaths) {
+        [rangePaths removeObject:path];
+      }
+      _rangeTypeIndexPaths[rangeKey] = rangePaths;
+    }
+    
     [_delegate rangeController:self didDeleteNodes:nodes atIndexPaths:indexPaths withAnimationOptions:animationOptions];
   });
 }
 
-- (void)dataController:(ASDataController *)dataController didInsertSections:(NSArray *)sections atIndexSet:(NSIndexSet *)indexSet withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions {
+- (void)dataController:(ASDataController *)dataController didInsertSections:(NSArray *)sections atIndexSet:(NSIndexSet *)indexSet withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions
+{
   ASDisplayNodeAssert(sections.count == indexSet.count, @"Invalid sections");
-
-  NSMutableArray *sectionNodeSizes = [NSMutableArray arrayWithCapacity:sections.count];
-
-  [sections enumerateObjectsUsingBlock:^(NSArray *nodes, NSUInteger idx, BOOL *stop) {
-    NSMutableArray *nodeSizes = [NSMutableArray arrayWithCapacity:nodes.count];
-    [nodes enumerateObjectsUsingBlock:^(ASCellNode *node, NSUInteger idx2, BOOL *stop2) {
-      [nodeSizes addObject:[NSValue valueWithCGSize:node.calculatedSize]];
-    }];
-    [sectionNodeSizes addObject:nodeSizes];
-  }];
-
   ASPerformBlockOnMainThread(^{
     _rangeIsValid = NO;
     [_delegate rangeController:self didInsertSectionsAtIndexSet:indexSet withAnimationOptions:animationOptions];
   });
 }
 
-- (void)dataController:(ASDataController *)dataController didDeleteSectionsAtIndexSet:(NSIndexSet *)indexSet withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions {
+- (void)dataController:(ASDataController *)dataController didDeleteSectionsAtIndexSet:(NSIndexSet *)indexSet withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions
+{
   ASPerformBlockOnMainThread(^{
     _rangeIsValid = NO;
+    
+    // When removing nodes we need to make sure that removed indexPaths are not left in _rangeTypeIndexPaths,
+    // otherwise _updateVisibleNodeIndexPaths may try to retrieve nodes from dataSource that aren't there anymore
+    for (NSInteger i = 0; i < ASLayoutRangeTypeCount; i++) {
+      id rangeKey = @((ASLayoutRangeType)i);
+      NSMutableSet *rangePaths = [_rangeTypeIndexPaths[rangeKey] mutableCopy];
+      for (NSIndexPath *path in _rangeTypeIndexPaths[rangeKey]) {
+        if ([indexSet containsIndex:path.section]) {
+          [rangePaths removeObject:path];
+        }
+      }
+      _rangeTypeIndexPaths[rangeKey] = rangePaths;
+    }
+    
     [_delegate rangeController:self didDeleteSectionsAtIndexSet:indexSet withAnimationOptions:animationOptions];
   });
 }
diff --git a/examples/VerticalWithinHorizontalScrolling/Sample/GradientTableNode.h b/examples/VerticalWithinHorizontalScrolling/Sample/GradientTableNode.h
@@ -19,4 +19,6 @@
 
 - (instancetype)initWithElementSize:(CGSize)size;
 
+@property (nonatomic) NSInteger pageNumber;
+
 @end
diff --git a/examples/VerticalWithinHorizontalScrolling/Sample/GradientTableNode.mm b/examples/VerticalWithinHorizontalScrolling/Sample/GradientTableNode.mm
@@ -63,9 +63,16 @@ - (ASCellNode *)tableView:(ASTableView *)tableView nodeForRowAtIndexPath:(NSInde
 {
   RandomCoreGraphicsNode *elementNode = [[RandomCoreGraphicsNode alloc] init];
   elementNode.preferredFrameSize = _elementSize;
+  elementNode.indexPath = [NSIndexPath indexPathForRow:indexPath.row inSection:_pageNumber];
   return elementNode;
 }
 
+- (void)tableView:(UITableView *)tableView didSelectRowAtIndexPath:(NSIndexPath *)indexPath
+{
+  [tableView deselectRowAtIndexPath:indexPath animated:NO];
+  [_tableNode.view reloadRowsAtIndexPaths:@[indexPath] withRowAnimation:UITableViewRowAnimationNone];
+}
+
 - (void)layout
 {
   [super layout];
diff --git a/examples/VerticalWithinHorizontalScrolling/Sample/RandomCoreGraphicsNode.h b/examples/VerticalWithinHorizontalScrolling/Sample/RandomCoreGraphicsNode.h
@@ -10,4 +10,6 @@
 
 @interface RandomCoreGraphicsNode : ASCellNode
 
+@property (nonatomic) NSIndexPath *indexPath;
+
 @end
diff --git a/examples/VerticalWithinHorizontalScrolling/Sample/RandomCoreGraphicsNode.m b/examples/VerticalWithinHorizontalScrolling/Sample/RandomCoreGraphicsNode.m
@@ -42,4 +42,24 @@ + (void)drawRect:(CGRect)bounds withParameters:(id<NSObject>)parameters isCancel
   CGColorSpaceRelease(colorSpace);
 }
 
+#if 0
+- (void)fetchData
+{
+  NSLog(@"fetchData - %@, %@", self, self.indexPath);
+  [super fetchData];
+}
+
+- (void)clearFetchedData
+{
+  NSLog(@"clearFetchedData - %@, %@", self, self.indexPath);
+  [super clearFetchedData];
+}
+
+- (void)visibilityDidChange:(BOOL)isVisible
+{
+  NSLog(@"visibilityDidChange:%d - %@, %@", isVisible, self, self.indexPath);
+  [super visibilityDidChange:isVisible];
+}
+#endif
+
 @end
diff --git a/examples/VerticalWithinHorizontalScrolling/Sample/ViewController.m b/examples/VerticalWithinHorizontalScrolling/Sample/ViewController.m
@@ -69,12 +69,13 @@ - (BOOL)prefersStatusBarHidden
 #pragma mark -
 #pragma mark ASPagerNode.
 
-- (ASCellNode *)pagerNode:(ASPagerNode *)pagerNode nodeAtIndex:(NSInteger)index;
+- (ASCellNode *)pagerNode:(ASPagerNode *)pagerNode nodeAtIndex:(NSInteger)index
 {
   CGSize boundsSize = pagerNode.bounds.size;
   CGSize gradientRowSize = CGSizeMake(boundsSize.width, 100);
   GradientTableNode *node = [[GradientTableNode alloc] initWithElementSize:gradientRowSize];
   node.preferredFrameSize = boundsSize;
+  node.pageNumber = index;
   return node;
 }
 

Original file line number	Diff line number	Diff line change
`@@ -63,9 +63,16 @@ - (ASCellNode )tableView:(ASTableView )tableView nodeForRowAtIndexPath:(NSInde`
`63`	`63`	`{`
`64`	`64`	`RandomCoreGraphicsNode *elementNode = [[RandomCoreGraphicsNode alloc] init];`
`65`	`65`	`elementNode.preferredFrameSize = _elementSize;`
	`66`	`+ elementNode.indexPath = [NSIndexPath indexPathForRow:indexPath.row inSection:_pageNumber];`
`66`	`67`	`return elementNode;`
`67`	`68`	`}`
`68`	`69`
	`70`	`+- (void)tableView:(UITableView )tableView didSelectRowAtIndexPath:(NSIndexPath )indexPath`
	`71`	`+{`
	`72`	`+ [tableView deselectRowAtIndexPath:indexPath animated:NO];`
	`73`	`+ [_tableNode.view reloadRowsAtIndexPaths:@[indexPath] withRowAnimation:UITableViewRowAnimationNone];`
	`74`	`+}`
	`75`	`+`
`69`	`76`	`- (void)layout`
`70`	`77`	`{`
`71`	`78`	`[super layout];`