[ASDataController] Apply new visible map inside batch updates block (#…

…420)

* ASDataController to apply new visible map inside batch updates block

* Update CHANGELOG

* Sorry, put up a PR that doesn't even build LOL

CHANGELOG.md

@@ -8,6 +8,7 @@
 - Fix an issue where inserting/deleting sections could lead to inconsistent supplementary element behavior. [Adlai Holler](https://github.com/Adlai-Holler)
 - Overhaul logging and add activity tracing support. [Adlai Holler](https://github.com/Adlai-Holler)
 - Fix a crash where scrolling a table view after entering editing mode could lead to bad internal states in the table. [Huy Nguyen](https://github.com/nguyenhuy) [#416](https://github.com/TextureGroup/Texture/pull/416/)
+- Fix a crash in collection view that occurs if batch updates are performed while scrolling [Huy Nguyen](https://github.com/nguyenhuy) [#378](https://github.com/TextureGroup/Texture/issues/378)
 
 ##2.3.4
 - [Yoga] Rewrite YOGA_TREE_CONTIGUOUS mode with improved behavior and cleaner integration [Scott Goodson](https://github.com/appleguy)

Source/ASCollectionView.mm

@@ -1895,10 +1895,11 @@ - (void)rangeController:(ASRangeController *)rangeController willUpdateWithChang
   }
 }
 
-- (void)rangeController:(ASRangeController *)rangeController didUpdateWithChangeSet:(_ASHierarchyChangeSet *)changeSet
+- (void)rangeController:(ASRangeController *)rangeController didUpdateWithChangeSet:(_ASHierarchyChangeSet *)changeSet updates:(dispatch_block_t)updates
 {
   ASDisplayNodeAssertMainThread();
   if (!self.asyncDataSource || _superIsPendingDataLoad) {
+    updates();
     [changeSet executeCompletionHandlerWithFinished:NO];
     return; // if the asyncDataSource has become invalid while we are processing, ignore this request to avoid crashes
   }
@@ -1907,6 +1908,7 @@ - (void)rangeController:(ASRangeController *)rangeController didUpdateWithChange
     as_activity_scope(as_activity_create("Commit collection update", changeSet.rootActivity, OS_ACTIVITY_FLAG_DEFAULT));
     if (changeSet.includesReloadData) {
       _superIsPendingDataLoad = YES;
+      updates();
       [super reloadData];
       as_log_debug(ASCollectionLog(), "Did reloadData %@", self.collectionNode);
       [changeSet executeCompletionHandlerWithFinished:YES];
@@ -1915,6 +1917,8 @@ - (void)rangeController:(ASRangeController *)rangeController didUpdateWithChange
 
       __block NSUInteger numberOfUpdates = 0;
       [self _superPerformBatchUpdates:^{
+        updates();
+
         for (_ASHierarchyItemChange *change in [changeSet itemChangesOfType:_ASHierarchyChangeTypeReload]) {
           [super reloadItemsAtIndexPaths:change.indexPaths];
           numberOfUpdates++;

Source/ASTableView.mm

@@ -1486,10 +1486,11 @@ - (void)rangeController:(ASRangeController *)rangeController willUpdateWithChang
   }
 }
 
-- (void)rangeController:(ASRangeController *)rangeController didUpdateWithChangeSet:(_ASHierarchyChangeSet *)changeSet
+- (void)rangeController:(ASRangeController *)rangeController didUpdateWithChangeSet:(_ASHierarchyChangeSet *)changeSet updates:(dispatch_block_t)updates
 {
   ASDisplayNodeAssertMainThread();
   if (!self.asyncDataSource || _updatingInResponseToInteractiveMove) {
+    updates();
     [changeSet executeCompletionHandlerWithFinished:NO];
     return; // if the asyncDataSource has become invalid while we are processing, ignore this request to avoid crashes
   }
@@ -1500,6 +1501,7 @@ - (void)rangeController:(ASRangeController *)rangeController didUpdateWithChange
       if (self.test_enableSuperUpdateCallLogging) {
         NSLog(@"-[super reloadData]");
       }
+      updates();
       [super reloadData];
       // Flush any range changes that happened as part of submitting the reload.
       [_rangeController updateIfNeeded];
@@ -1513,6 +1515,8 @@ - (void)rangeController:(ASRangeController *)rangeController didUpdateWithChange
 
   LOG(@"--- UITableView beginUpdates");
   [super beginUpdates];
+
+  updates();
 
   for (_ASHierarchyItemChange *change in [changeSet itemChangesOfType:_ASHierarchyChangeTypeReload]) {
     NSArray<NSIndexPath *> *indexPaths = change.indexPaths;

Source/Details/ASDataController.h

@@ -117,8 +117,14 @@ extern NSString * const ASCollectionInvalidUpdateException;
  * Called for change set updates.
  *
  * @param changeSet The change set that includes all updates
+ *
+ * @param updates The block that performs relevant data updates.
+ *
+ * @discussion The updates block must always be executed or the data controller will get into a bad state.
+ * It should be called at the time the backing view is ready to process the updates,
+ * i.e inside the updates block of `-[UICollectionView performBatchUpdates:completion:] or after calling `-[UITableView beginUpdates]`.
  */
-- (void)dataController:(ASDataController *)dataController didUpdateWithChangeSet:(_ASHierarchyChangeSet *)changeSet;
+- (void)dataController:(ASDataController *)dataController didUpdateWithChangeSet:(_ASHierarchyChangeSet *)changeSet updates:(dispatch_block_t)updates;
 
 @end
 

Source/Details/ASDataController.mm

@@ -610,12 +610,20 @@ - (void)updateWithChangeSet:(_ASHierarchyChangeSet *)changeSet
 
       [_mainSerialQueue performBlockOnMainThread:^{
         as_activity_scope_leave(&preparationScope);
+        // TODO Merge the two delegate methods below
         [_delegate dataController:self willUpdateWithChangeSet:changeSet];
 
-        // Step 4: Deploy the new data as "completed" and inform delegate
-        self.visibleMap = newMap;
-
-        [_delegate dataController:self didUpdateWithChangeSet:changeSet];
+        // Step 4: Inform the delegate
+        [_delegate dataController:self didUpdateWithChangeSet:changeSet updates:^{
+          // Step 5: Deploy the new data as "completed"
+          //
+          // Note that since the backing collection view might be busy responding to user events (e.g scrolling),
+          // it will not consume the batch update blocks immediately.
+          // As a result, in a short intermidate time, the view will still be relying on the old data source state.
+          // Thus, we can't just swap the new map immediately before step 4, but until this update block is executed.
+          // (https://github.com/TextureGroup/Texture/issues/378)
+          self.visibleMap = newMap;
+        }];
       }];
     }];
   });

Source/Details/ASRangeController.h

@@ -156,8 +156,14 @@ AS_SUBCLASSING_RESTRICTED
  * Called after updating with given change set.
  *
  * @param changeSet The change set that includes all updates
+ *
+ * @param updates The block that performs relevant data updates.
+ *
+ * @discussion The updates block must always be executed or the data controller will get into a bad state.
+ * It should be called at the time the backing view is ready to process the updates,
+ * i.e inside the updates block of `-[UICollectionView performBatchUpdates:completion:] or after calling `-[UITableView beginUpdates]`.
  */
-- (void)rangeController:(ASRangeController *)rangeController didUpdateWithChangeSet:(_ASHierarchyChangeSet *)changeSet;
+- (void)rangeController:(ASRangeController *)rangeController didUpdateWithChangeSet:(_ASHierarchyChangeSet *)changeSet updates:(dispatch_block_t)updates;
 
 @end
 

Source/Details/ASRangeController.mm

@@ -502,11 +502,11 @@ - (void)dataController:(ASDataController *)dataController willUpdateWithChangeSe
   [_delegate rangeController:self willUpdateWithChangeSet:changeSet];
 }
 
-- (void)dataController:(ASDataController *)dataController didUpdateWithChangeSet:(_ASHierarchyChangeSet *)changeSet
+- (void)dataController:(ASDataController *)dataController didUpdateWithChangeSet:(_ASHierarchyChangeSet *)changeSet updates:(dispatch_block_t)updates
 {
   ASDisplayNodeAssertMainThread();
   _rangeIsValid = NO;
-  [_delegate rangeController:self didUpdateWithChangeSet:changeSet];
+  [_delegate rangeController:self didUpdateWithChangeSet:changeSet updates:updates];
 }
 
 #pragma mark - Memory Management