fix invalidateLayout crash

Summary:
Pretty sure in `-invalidateLayout`, we should calculate the `NSIndexPaths` after the delay. Otherwise, those `NSIndexPaths` might be incorrect after the update.

Before this change, running `test_whenInvalidatingInsideBatchUpdate_andRemoveThatSectionController_thatCollectionViewDoesntCrash` crashes with:
> Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'attempting to invalidate an item at an invalid indexPath: <NSIndexPath: 0x9566e79c2077363c> {length = 2, path = 1 - 0} globalIndex: 1 numItems: 1'

Reviewed By: candance

Differential Revision: D23742819

fbshipit-source-id: 39496d04025ff3554673313df3c7516c33c305d4
This commit is contained in:
Maxime Ollivier 2020-09-16 16:16:40 -07:00 committed by Facebook GitHub Bot
parent a6526ce097
commit cbaa7a8dce
3 changed files with 80 additions and 8 deletions

View file

@ -1271,6 +1271,17 @@
- (void)invalidateLayoutForSectionController:(IGListSectionController *)sectionController
completion:(void (^)(BOOL finished))completion {
__weak __typeof__(self) weakSelf = self;
// do not call -[UICollectionView performBatchUpdates:completion:] while already updating. defer it until completed.
[self _deferBlockBetweenBatchUpdates:^{
// Note that we calculate the `NSIndexPaths` after the batch update, otherwise they're be wrong.
[weakSelf _invalidateLayoutForSectionController:sectionController completion:completion];
}];
}
- (void)_invalidateLayoutForSectionController:(IGListSectionController *)sectionController
completion:(void (^)(BOOL finished))completion {
const NSInteger section = [self sectionForSectionController:sectionController];
if (section == NSNotFound) {
// The section controller is not in the map, which can happen if the associated object was deleted or after a full reload.
@ -1291,14 +1302,9 @@
UICollectionViewLayoutInvalidationContext *context = [[[layout.class invalidationContextClass] alloc] init];
[context invalidateItemsAtIndexPaths:indexPaths];
__weak __typeof__(_collectionView) weakCollectionView = _collectionView;
// do not call -[UICollectionView performBatchUpdates:completion:] while already updating. defer it until completed.
[self _deferBlockBetweenBatchUpdates:^{
[weakCollectionView performBatchUpdates:^{
[layout invalidateLayoutWithContext:context];
} completion:completion];
}];
[_collectionView performBatchUpdates:^{
[layout invalidateLayoutWithContext:context];
} completion:completion];
}
#pragma mark - IGListBatchContext

View file

@ -1597,6 +1597,39 @@
}];
}
- (void)test_whenInvalidatingInsideBatchUpdate_andRemoveThatSectionController_thatCollectionViewDoesntCrash {
IGTestObject *foo = genTestObject(@1, @"Foo");
IGTestObject *bar = genTestObject(@0, @"Bar");
self.dataSource.objects = @[foo, bar];
UICollectionView *collectionView = [[UICollectionView alloc] initWithFrame:self.window.frame collectionViewLayout:[UICollectionViewFlowLayout new]];
[self.window addSubview:collectionView];
IGListAdapterUpdater *updater = [IGListAdapterUpdater new];
IGListAdapter *adapter = [[IGListAdapter alloc] initWithUpdater:updater viewController:nil];
adapter.dataSource = self.dataSource;
adapter.collectionView = collectionView;
[collectionView layoutIfNeeded];
IGTestDelegateController *sectionToRemove = [adapter sectionControllerForObject:bar];
self.dataSource.objects = @[foo];
XCTestExpectation *expectation = genExpectation;
[adapter performUpdatesAnimated:YES completion:^(BOOL finished) {
XCTAssertTrue(finished);
[expectation fulfill];
}];
XCTestExpectation *expectation2 = genExpectation;
[sectionToRemove.collectionContext invalidateLayoutForSectionController:sectionToRemove completion:^(BOOL finished) {
// That section-controller is about to be removed, so this should not finish.
XCTAssertFalse(finished);
[expectation2 fulfill];
}];
[self waitForExpectationsWithTimeout:30 handler:nil];
}
- (void)test_whenAddingMultipleUpdateListeners_withPerformUpdatesAnimated_thatEventsReceived {
[self setupWithObjects:@[
genTestObject(@1, @1)

View file

@ -2005,6 +2005,39 @@
}];
}
- (void)test_whenInvalidatingInsideBatchUpdate_andRemoveThatSectionController_thatCollectionViewDoesntCrash {
IGTestObject *foo = genTestObject(@1, @"Foo");
IGTestObject *bar = genTestObject(@0, @"Bar");
self.dataSource.objects = @[foo, bar];
UICollectionView *collectionView = [[UICollectionView alloc] initWithFrame:self.window.frame collectionViewLayout:[UICollectionViewFlowLayout new]];
[self.window addSubview:collectionView];
IGListExperimentalAdapterUpdater *updater = [IGListExperimentalAdapterUpdater new];
IGListAdapter *adapter = [[IGListAdapter alloc] initWithUpdater:updater viewController:nil];
adapter.dataSource = self.dataSource;
adapter.collectionView = collectionView;
[collectionView layoutIfNeeded];
IGTestDelegateController *sectionToRemove = [adapter sectionControllerForObject:bar];
self.dataSource.objects = @[foo];
XCTestExpectation *expectation = genExpectation;
[adapter performUpdatesAnimated:YES completion:^(BOOL finished) {
XCTAssertTrue(finished);
[expectation fulfill];
}];
XCTestExpectation *expectation2 = genExpectation;
[sectionToRemove.collectionContext invalidateLayoutForSectionController:sectionToRemove completion:^(BOOL finished) {
// That section-controller is about to be removed, so this should not finish.
XCTAssertFalse(finished);
[expectation2 fulfill];
}];
[self waitForExpectationsWithTimeout:30 handler:nil];
}
- (void)test_whenPerformingBatchSectionUpdate_thatTransactionObjectsGetsDeallocated {
__weak IGListUpdateTransactionBuilder *transactionBuilder = nil;
__block __weak IGListUpdateTransactionBuilder *lastTransactionBuilder = nil;