From 5a141094ce93d1e01077b2edad849a888af12e2e Mon Sep 17 00:00:00 2001
From: andyfinnell <andyfinnell@mac.com>
Date: Wed, 21 May 2025 15:56:25 -0400
Subject: [PATCH] Check for batch fetching on scroll

## Summary

For collection views, we do not currently check for batch fetching on scroll. This appears to be a bug, as `scrollViewDidScroll:` does call `_checkForBatchFetching`, and [the commit that added it states it's trying to check on each scroll](https://github.com/TextureGroup/Texture/commit/df497b82c286771658a0ef0826945c716baaa783). Unfortunately, `_checkForBatchFetching` checks for `isTracking` and `isDragging` first and returns if either are `YES`. Since we're in a scroll, they are `YES`. So the call is effectively a no-op.

This bug has been around for 9 years and I'm unsure of the performance implications of turning the batch fetching check on for each scroll tick. Therefore, put the fix behind an experiment feature flag _and_ only call it on the first scroll tick for the scroll session, instead of every scroll tick. Finally, I moved the check after the delegate call, in case the delegate has logic in it to turn on or off the batch fetching.

## Test plan

Ran the app and manually tested batching getting called or not. Also ran `build.sh tests`.
---
 Source/ASCollectionView.mm       | 17 ++++++++++++++++-
 Source/ASExperimentalFeatures.h  |  1 +
 Source/ASExperimentalFeatures.mm |  3 ++-
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/Source/ASCollectionView.mm b/Source/ASCollectionView.mm
index 1ff6062a4..a190abe03 100644
--- a/Source/ASCollectionView.mm
+++ b/Source/ASCollectionView.mm
@@ -142,6 +142,12 @@ @interface ASCollectionView () <ASRangeControllerDataSource, ASRangeControllerDe
    */
   BOOL _hasEverCheckedForBatchFetchingDueToUpdate;
 
+  /**
+   * We want to check for batch fetching on scroll, but every tick would be too much. So check once at the
+   * beginning
+   */
+  BOOL _hasCheckedForBatchFetchingOnScroll;
+
   /**
    * Set during beginInteractiveMovementForItemAtIndexPath and UIGestureRecognizerStateEnded
    * (or UIGestureRecognizerStateFailed, UIGestureRecognizerStateCancelled.
@@ -1620,7 +1626,9 @@ - (void)collectionView:(UICollectionView *)collectionView moveItemAtIndexPath:(N
 - (void)scrollViewDidScroll:(UIScrollView *)scrollView
 {
   ASInterfaceState interfaceState = [self interfaceStateForRangeController:_rangeController];
-  if (ASInterfaceStateIncludesVisible(interfaceState)) {
+  if (ASInterfaceStateIncludesVisible(interfaceState) && !ASActivateExperimentalFeature(ASExperimentalCheckBatchFetchingOnScroll)) {
+    // The following call to _checkForBatchFetching is effectively a no-op, because during scrolling
+    //  isDragging and isTracking are YES.
     [self _checkForBatchFetching];
   }
   for (_ASCollectionViewCell *cell in _cellsForVisibilityUpdates) {
@@ -1630,6 +1638,11 @@ - (void)scrollViewDidScroll:(UIScrollView *)scrollView
   if (_asyncDelegateFlags.scrollViewDidScroll) {
     [_asyncDelegate scrollViewDidScroll:scrollView];
   }
+  if (ASInterfaceStateIncludesVisible(interfaceState) && !_hasCheckedForBatchFetchingOnScroll && ASActivateExperimentalFeature(ASExperimentalCheckBatchFetchingOnScroll)) {
+    // Check after the delegate it give it a chance to turn on/off batch fetching
+    [self _beginBatchFetchingIfNeededWithContentOffset:self.contentOffset velocity:CGPointZero];
+    _hasCheckedForBatchFetchingOnScroll = YES;
+  }
 }
 
 - (void)scrollViewWillEndDragging:(UIScrollView *)scrollView withVelocity:(CGPoint)velocity targetContentOffset:(inout CGPoint *)targetContentOffset
@@ -1645,6 +1658,8 @@ - (void)scrollViewWillEndDragging:(UIScrollView *)scrollView withVelocity:(CGPoi
     [self _beginBatchFetchingIfNeededWithContentOffset:*targetContentOffset velocity:velocity];
   }
   
+  _hasCheckedForBatchFetchingOnScroll = NO; // reset once the scroll is done
+  
   if (_asyncDelegateFlags.scrollViewWillEndDragging) {
     [_asyncDelegate scrollViewWillEndDragging:scrollView withVelocity:velocity targetContentOffset:(targetContentOffset ? : &contentOffset)];
   }
diff --git a/Source/ASExperimentalFeatures.h b/Source/ASExperimentalFeatures.h
index 5a66c9a57..62e7cda26 100644
--- a/Source/ASExperimentalFeatures.h
+++ b/Source/ASExperimentalFeatures.h
@@ -34,6 +34,7 @@ typedef NS_OPTIONS(NSUInteger, ASExperimentalFeatures) {
   ASExperimentalNoTextRendererCache = 1 << 13,                              // exp_no_text_renderer_cache
   ASExperimentalLockTextRendererCache = 1 << 14,                            // exp_lock_text_renderer_cache
   ASExperimentalHierarchyDisplayDidFinishIsRecursive = 1 << 15,             // exp_hierarchy_display_did_finish_is_recursive
+  ASExperimentalCheckBatchFetchingOnScroll = 1 << 16,                       // exp_check_batch_fetching_on_scroll
   ASExperimentalFeatureAll = 0xFFFFFFFF
 };
 
diff --git a/Source/ASExperimentalFeatures.mm b/Source/ASExperimentalFeatures.mm
index 729b1bc19..dd41e0981 100644
--- a/Source/ASExperimentalFeatures.mm
+++ b/Source/ASExperimentalFeatures.mm
@@ -27,7 +27,8 @@
                                       @"exp_range_update_on_changeset_update",
                                       @"exp_no_text_renderer_cache",
                                       @"exp_lock_text_renderer_cache",
-                                      @"exp_hierarchy_display_did_finish_is_recursive"]));
+                                      @"exp_hierarchy_display_did_finish_is_recursive",
+                                      @"exp_check_batch_fetching_on_scroll"]));
 
   if (flags == ASExperimentalFeatureAll) {
     return allNames;