Merge pull request #2715 from facebook/AHMoreBatchFetching

Ensure Batch Fetching Includes Leading Screens, Even if Not Scrolling

AsyncDisplayKit/ASCollectionView.mm

@@ -1708,6 +1708,12 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell";
     [_rangeController setNeedsUpdate];
     [_rangeController updateIfNeeded];
   }
+
+  // When we aren't visible, we will only fetch up to the visible area. Now that we are visible,
+  // we will fetch visible area + leading screens, so we need to check.
+  if (visible) {
+    [self _checkForBatchFetching];
+  }
 }
 
 #pragma mark ASCALayerExtendedDelegate

AsyncDisplayKit/ASTableView.mm

@@ -1725,6 +1725,12 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell";
     [_rangeController setNeedsUpdate];
     [_rangeController updateIfNeeded];
   }
+
+  // When we aren't visible, we will only fetch up to the visible area. Now that we are visible,
+  // we will fetch visible area + leading screens, so we need to check.
+  if (visible) {
+    [self _checkForBatchFetching];
+  }
 }
 
 @end

AsyncDisplayKit/Private/ASBatchFetching.h

@@ -45,6 +45,7 @@ BOOL ASDisplayShouldFetchBatchForScrollView(UIScrollView<ASBatchFetchingScrollVi
  @param contentSize The content size of the scrollview.
  @param targetOffset The offset that the scrollview will scroll to.
  @param leadingScreens How many screens in the remaining distance will trigger batch fetching.
+ @param visible Whether the view is visible or not.
  @return Whether or not the current state should proceed with batch fetching.
  @discussion This method is broken into a category for unit testing purposes and should be used with the ASTableView and
  * ASCollectionView batch fetching API.
@@ -55,6 +56,7 @@ extern BOOL ASDisplayShouldFetchBatchForContext(ASBatchContext *context,
                                                 CGRect bounds,
                                                 CGSize contentSize,
                                                 CGPoint targetOffset,
-                                                CGFloat leadingScreens);
+                                                CGFloat leadingScreens,
+                                                BOOL visible);
 
 ASDISPLAYNODE_EXTERN_C_END

AsyncDisplayKit/Private/ASBatchFetching.m

@@ -22,7 +22,8 @@ BOOL ASDisplayShouldFetchBatchForScrollView(UIScrollView<ASBatchFetchingScrollVi
   CGRect bounds = scrollView.bounds;
   CGSize contentSize = scrollView.contentSize;
   CGFloat leadingScreens = scrollView.leadingScreensForBatching;
-  return ASDisplayShouldFetchBatchForContext(context, scrollDirection, scrollableDirections, bounds, contentSize, contentOffset, leadingScreens);
+  BOOL visible = (scrollView.window != nil);
+  return ASDisplayShouldFetchBatchForContext(context, scrollDirection, scrollableDirections, bounds, contentSize, contentOffset, leadingScreens, visible);
 }
 
 BOOL ASDisplayShouldFetchBatchForContext(ASBatchContext *context,
@@ -31,7 +32,8 @@ BOOL ASDisplayShouldFetchBatchForContext(ASBatchContext *context,
                                          CGRect bounds,
                                          CGSize contentSize,
                                          CGPoint targetOffset,
-                                         CGFloat leadingScreens)
+                                         CGFloat leadingScreens,
+                                         BOOL visible)
 {
   // Do not allow fetching if a batch is already in-flight and hasn't been completed or cancelled
   if ([context isFetching]) {
@@ -55,16 +57,25 @@ BOOL ASDisplayShouldFetchBatchForContext(ASBatchContext *context,
     contentLength = contentSize.width;
   }
 
-  // target offset will always be 0 if the content size is smaller than the viewport
-  BOOL hasSmallContent = offset == 0.0 && contentLength < viewLength;
+  BOOL hasSmallContent = contentLength < viewLength;
   if (hasSmallContent) {
     return YES;
   }
 
-  BOOL isScrollingTowardEnd = (ASScrollDirectionContainsDown(scrollDirection) || ASScrollDirectionContainsRight(scrollDirection));
+  // If we are not visible, but we do have enough content to fill visible area,
+  // don't batch fetch.
+  if (visible == NO) {
+    return NO;
+  }
+
+  // If they are scrolling toward the head of content, don't batch fetch.
+  BOOL isScrollingTowardHead = (ASScrollDirectionContainsUp(scrollDirection) || ASScrollDirectionContainsLeft(scrollDirection));
+  if (isScrollingTowardHead) {
+    return NO;
+  }
 
   CGFloat triggerDistance = viewLength * leadingScreens;
   CGFloat remainingDistance = contentLength - viewLength - offset;
 
-  return isScrollingTowardEnd && remainingDistance <= triggerDistance;
+  return remainingDistance <= triggerDistance;
 }

AsyncDisplayKitTests/ASBatchFetchingTests.m

@@ -30,26 +30,26 @@
 
 - (void)testBatchNullState {
   ASBatchContext *context = [[ASBatchContext alloc] init];
-  BOOL shouldFetch = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionDown, ASScrollDirectionVerticalDirections, CGRectZero, CGSizeZero, CGPointZero, 0.0);
+  BOOL shouldFetch = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionDown, ASScrollDirectionVerticalDirections, CGRectZero, CGSizeZero, CGPointZero, 0.0, YES);
   XCTAssert(shouldFetch == NO, @"Should not fetch in the null state");
 }
 
 - (void)testBatchAlreadyFetching {
   ASBatchContext *context = [[ASBatchContext alloc] init];
   [context beginBatchFetching];
-  BOOL shouldFetch = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionDown, ASScrollDirectionVerticalDirections, PASSING_RECT, PASSING_SIZE, PASSING_POINT, 1.0);
+  BOOL shouldFetch = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionDown, ASScrollDirectionVerticalDirections, PASSING_RECT, PASSING_SIZE, PASSING_POINT, 1.0, YES);
   XCTAssert(shouldFetch == NO, @"Should not fetch when context is already fetching");
 }
 
 - (void)testUnsupportedScrollDirections {
   ASBatchContext *context = [[ASBatchContext alloc] init];
-  BOOL fetchRight = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionRight, ASScrollDirectionHorizontalDirections, PASSING_RECT, PASSING_SIZE, PASSING_POINT, 1.0);
+  BOOL fetchRight = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionRight, ASScrollDirectionHorizontalDirections, PASSING_RECT, PASSING_SIZE, PASSING_POINT, 1.0, YES);
   XCTAssert(fetchRight == YES, @"Should fetch for scrolling right");
-  BOOL fetchDown = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionDown, ASScrollDirectionVerticalDirections, PASSING_RECT, PASSING_SIZE, PASSING_POINT, 1.0);
+  BOOL fetchDown = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionDown, ASScrollDirectionVerticalDirections, PASSING_RECT, PASSING_SIZE, PASSING_POINT, 1.0, YES);
   XCTAssert(fetchDown == YES, @"Should fetch for scrolling down");
-  BOOL fetchUp = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionUp, ASScrollDirectionVerticalDirections, PASSING_RECT, PASSING_SIZE, PASSING_POINT, 1.0);
+  BOOL fetchUp = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionUp, ASScrollDirectionVerticalDirections, PASSING_RECT, PASSING_SIZE, PASSING_POINT, 1.0, YES);
   XCTAssert(fetchUp == NO, @"Should not fetch for scrolling up");
-  BOOL fetchLeft = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionLeft, ASScrollDirectionHorizontalDirections, PASSING_RECT, PASSING_SIZE, PASSING_POINT, 1.0);
+  BOOL fetchLeft = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionLeft, ASScrollDirectionHorizontalDirections, PASSING_RECT, PASSING_SIZE, PASSING_POINT, 1.0, YES);
   XCTAssert(fetchLeft == NO, @"Should not fetch for scrolling left");
 }
 
@@ -57,7 +57,7 @@
   CGFloat screen = 1.0;
   ASBatchContext *context = [[ASBatchContext alloc] init];
   // scroll to 1-screen top offset, height is 1 screen, so bottom is 1 screen away from end of content
-  BOOL shouldFetch = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionDown, ASScrollDirectionVerticalDirections, VERTICAL_RECT(screen), VERTICAL_SIZE(screen * 3.0), VERTICAL_OFFSET(screen * 1.0), 1.0);
+  BOOL shouldFetch = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionDown, ASScrollDirectionVerticalDirections, VERTICAL_RECT(screen), VERTICAL_SIZE(screen * 3.0), VERTICAL_OFFSET(screen * 1.0), 1.0, YES);
   XCTAssert(shouldFetch == YES, @"Fetch should begin when vertically scrolling to exactly 1 leading screen away");
 }
 
@@ -65,7 +65,7 @@
   CGFloat screen = 1.0;
   ASBatchContext *context = [[ASBatchContext alloc] init];
   // 3 screens of content, scroll only 1/2 of one screen
-  BOOL shouldFetch = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionDown, ASScrollDirectionVerticalDirections, VERTICAL_RECT(screen), VERTICAL_SIZE(screen * 3.0), VERTICAL_OFFSET(screen * 0.5), 1.0);
+  BOOL shouldFetch = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionDown, ASScrollDirectionVerticalDirections, VERTICAL_RECT(screen), VERTICAL_SIZE(screen * 3.0), VERTICAL_OFFSET(screen * 0.5), 1.0, YES);
   XCTAssert(shouldFetch == NO, @"Fetch should not begin when vertically scrolling less than the leading distance away");
 }
 
@@ -73,7 +73,7 @@
   CGFloat screen = 1.0;
   ASBatchContext *context = [[ASBatchContext alloc] init];
   // 3 screens of content, top offset to 3-screens, height 1 screen, so its 1 screen past the leading
-  BOOL shouldFetch = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionDown, ASScrollDirectionVerticalDirections, VERTICAL_RECT(screen), VERTICAL_SIZE(screen * 3.0), VERTICAL_OFFSET(screen * 3.0), 1.0);
+  BOOL shouldFetch = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionDown, ASScrollDirectionVerticalDirections, VERTICAL_RECT(screen), VERTICAL_SIZE(screen * 3.0), VERTICAL_OFFSET(screen * 3.0), 1.0, YES);
   XCTAssert(shouldFetch == YES, @"Fetch should begin when vertically scrolling past the content size");
 }
 
@@ -81,7 +81,7 @@
   CGFloat screen = 1.0;
   ASBatchContext *context = [[ASBatchContext alloc] init];
   // scroll to 1-screen left offset, width is 1 screen, so right is 1 screen away from end of content
-  BOOL shouldFetch = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionRight, ASScrollDirectionVerticalDirections, HORIZONTAL_RECT(screen), HORIZONTAL_SIZE(screen * 3.0), HORIZONTAL_OFFSET(screen * 1.0), 1.0);
+  BOOL shouldFetch = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionRight, ASScrollDirectionVerticalDirections, HORIZONTAL_RECT(screen), HORIZONTAL_SIZE(screen * 3.0), HORIZONTAL_OFFSET(screen * 1.0), 1.0, YES);
   XCTAssert(shouldFetch == YES, @"Fetch should begin when horizontally scrolling to exactly 1 leading screen away");
 }
 
@@ -89,7 +89,7 @@
   CGFloat screen = 1.0;
   ASBatchContext *context = [[ASBatchContext alloc] init];
   // 3 screens of content, scroll only 1/2 of one screen
-  BOOL shouldFetch = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionLeft, ASScrollDirectionHorizontalDirections, HORIZONTAL_RECT(screen), HORIZONTAL_SIZE(screen * 3.0), HORIZONTAL_OFFSET(screen * 0.5), 1.0);
+  BOOL shouldFetch = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionLeft, ASScrollDirectionHorizontalDirections, HORIZONTAL_RECT(screen), HORIZONTAL_SIZE(screen * 3.0), HORIZONTAL_OFFSET(screen * 0.5), 1.0, YES);
   XCTAssert(shouldFetch == NO, @"Fetch should not begin when horizontally scrolling less than the leading distance away");
 }
 
@@ -97,7 +97,7 @@
   CGFloat screen = 1.0;
   ASBatchContext *context = [[ASBatchContext alloc] init];
   // 3 screens of content, left offset to 3-screens, width 1 screen, so its 1 screen past the leading
-  BOOL shouldFetch = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionDown, ASScrollDirectionHorizontalDirections, HORIZONTAL_RECT(screen), HORIZONTAL_SIZE(screen * 3.0), HORIZONTAL_OFFSET(screen * 3.0), 1.0);
+  BOOL shouldFetch = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionDown, ASScrollDirectionHorizontalDirections, HORIZONTAL_RECT(screen), HORIZONTAL_SIZE(screen * 3.0), HORIZONTAL_OFFSET(screen * 3.0), 1.0, YES);
   XCTAssert(shouldFetch == YES, @"Fetch should begin when vertically scrolling past the content size");
 }
 
@@ -105,7 +105,7 @@
   CGFloat screen = 1.0;
   ASBatchContext *context = [[ASBatchContext alloc] init];
   // when the content size is < screen size, the target offset will always be 0
-  BOOL shouldFetch = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionDown, ASScrollDirectionVerticalDirections, VERTICAL_RECT(screen), VERTICAL_SIZE(screen * 0.5), VERTICAL_OFFSET(0.0), 1.0);
+  BOOL shouldFetch = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionDown, ASScrollDirectionVerticalDirections, VERTICAL_RECT(screen), VERTICAL_SIZE(screen * 0.5), VERTICAL_OFFSET(0.0), 1.0, YES);
   XCTAssert(shouldFetch == YES, @"Fetch should begin when the target is 0 and the content size is smaller than the scree");
 }
 
@@ -113,7 +113,7 @@
   CGFloat screen = 1.0;
   ASBatchContext *context = [[ASBatchContext alloc] init];
   // when the content size is < screen size, the target offset will always be 0
-  BOOL shouldFetch = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionRight, ASScrollDirectionHorizontalDirections, HORIZONTAL_RECT(screen), HORIZONTAL_SIZE(screen * 0.5), HORIZONTAL_OFFSET(0.0), 1.0);
+  BOOL shouldFetch = ASDisplayShouldFetchBatchForContext(context, ASScrollDirectionRight, ASScrollDirectionHorizontalDirections, HORIZONTAL_RECT(screen), HORIZONTAL_SIZE(screen * 0.5), HORIZONTAL_OFFSET(0.0), 1.0, YES);
   XCTAssert(shouldFetch == YES, @"Fetch should begin when the target is 0 and the content size is smaller than the scree");
 }
 

AsyncDisplayKitTests/ASCollectionViewTests.mm

@@ -891,4 +891,94 @@
   [self waitForExpectationsWithTimeout:3 handler:nil];
 }
 
+- (void)testThatWeBatchFetchUntilContentRequirementIsMet_Animated
+{
+  [self _primitiveBatchFetchingFillTestAnimated:YES visible:YES controller:nil];
+}
+
+- (void)testThatWeBatchFetchUntilContentRequirementIsMet_Nonanimated
+{
+  [self _primitiveBatchFetchingFillTestAnimated:NO visible:YES controller:nil];
+}
+
+- (void)testThatWeBatchFetchUntilContentRequirementIsMet_Invisible
+{
+  [self _primitiveBatchFetchingFillTestAnimated:NO visible:NO controller:nil];
+}
+
+- (void)testThatWhenWeBecomeVisibleWeWillFetchAdditionalContent
+{
+  ASCollectionViewTestController *ctrl = [[ASCollectionViewTestController alloc] initWithNibName:nil bundle:nil];
+  // Start with 1 empty section
+  ctrl.asyncDelegate->_itemCounts = {0};
+  [self _primitiveBatchFetchingFillTestAnimated:NO visible:NO controller:ctrl];
+  XCTAssertGreaterThan([ctrl.collectionNode numberOfItemsInSection:0], 0);
+  [self _primitiveBatchFetchingFillTestAnimated:NO visible:YES controller:ctrl];
+}
+
+- (void)_primitiveBatchFetchingFillTestAnimated:(BOOL)animated visible:(BOOL)visible controller:(nullable ASCollectionViewTestController *)testController
+{
+  if (testController == nil) {
+    testController = [[ASCollectionViewTestController alloc] initWithNibName:nil bundle:nil];
+    // Start with 1 empty section
+    testController.asyncDelegate->_itemCounts = {0};
+  }
+  ASCollectionNode *cn = testController.collectionNode;
+
+  UIWindow *window = nil;
+  UIView *view = nil;
+  if (visible) {
+    window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds];
+    view = window;
+  } else {
+    view = cn.view;
+    view.frame = [UIScreen mainScreen].bounds;
+  }
+
+  XCTestExpectation *expectation = [self expectationWithDescription:@"Completed all batch fetches"];
+  __weak ASCollectionViewTestController *weakController = testController;
+  __block NSInteger batchFetchCount = 0;
+  testController.asyncDelegate.willBeginBatchFetch = ^(ASBatchContext *context) {
+    dispatch_async(dispatch_get_main_queue(), ^{
+      NSInteger fetchIndex = batchFetchCount++;
+
+      NSInteger itemCount = weakController.asyncDelegate->_itemCounts[0];
+      weakController.asyncDelegate->_itemCounts[0] = (itemCount + 1);
+      if (animated) {
+        [cn insertItemsAtIndexPaths:@[ [NSIndexPath indexPathForItem:itemCount inSection:0] ]];
+      } else {
+        [cn performBatchAnimated:NO updates:^{
+          [cn insertItemsAtIndexPaths:@[ [NSIndexPath indexPathForItem:itemCount inSection:0] ]];
+        } completion:nil];
+      }
+
+      [context completeBatchFetching:YES];
+
+      // If no more batch fetches have happened in 1 second, assume we're done.
+      dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(1 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
+        if (fetchIndex == batchFetchCount - 1) {
+          [expectation fulfill];
+        }
+      });
+    });
+  };
+  window.rootViewController = testController;
+
+  [window makeKeyAndVisible];
+  [view layoutIfNeeded];
+
+  [self waitForExpectationsWithTimeout:60 handler:nil];
+  CGFloat contentHeight = cn.view.contentSize.height;
+  CGFloat requiredContentHeight;
+  CGFloat itemHeight = [cn.view layoutAttributesForItemAtIndexPath:[NSIndexPath indexPathForItem:0 inSection:0]].size.height;
+  if (visible) {
+    requiredContentHeight = CGRectGetMaxY(cn.bounds) + CGRectGetHeight(cn.bounds) * cn.view.leadingScreensForBatching;
+  } else {
+    requiredContentHeight = CGRectGetMaxY(cn.bounds);
+  }
+  XCTAssertGreaterThan(batchFetchCount, 2);
+  XCTAssertGreaterThanOrEqual(contentHeight, requiredContentHeight, @"Loaded too little content.");
+  XCTAssertLessThanOrEqual(contentHeight, requiredContentHeight + 2 * itemHeight, @"Loaded too much content.");
+}
+
 @end