Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 5 additions & 1 deletion Source/ASCollectionView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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];
Expand All @@ -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++;
Expand Down
6 changes: 5 additions & 1 deletion Source/ASTableView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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];
Expand All @@ -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;
Expand Down
8 changes: 7 additions & 1 deletion Source/Details/ASDataController.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
16 changes: 12 additions & 4 deletions Source/Details/ASDataController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}];
}];
}];
});
Expand Down
8 changes: 7 additions & 1 deletion Source/Details/ASRangeController.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions Source/Details/ASRangeController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down