check for a valid section NSInterger in IGListAdapter

Summary:
Issue: Invalidating the layout of a `IGListSectionController` that's been removed from the map crashes, because we're passing `NSNotFound` to `[UICollectionView numberOfItemsInSection:...]`. This can easily happen if the section-controller invalidates its layout in a `performBatchAnimated` completion block, but the associated object was deleted during the update or after a full reload.

Fix: Like the rest of `IGListAdapter`, lets check for a valid `NSInteger` and bail early if we don't have it.

Reviewed By: mariajayne

Differential Revision: D22588010

fbshipit-source-id: bf0c26f313795bb10682ef6b660d6ea8a7ce9f1e
This commit is contained in:
Maxime Ollivier 2020-07-17 08:44:01 -07:00 committed by Facebook GitHub Bot
parent d59c266108
commit 56fa344508
2 changed files with 83 additions and 4 deletions

View file

@ -901,10 +901,16 @@
}
- (NSArray<UICollectionViewCell *> *)fullyVisibleCellsForSectionController:(IGListSectionController *)sectionController {
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.
return @[];
}
NSMutableArray *cells = [NSMutableArray new];
UICollectionView *collectionView = self.collectionView;
NSArray *visibleCells = [collectionView visibleCells];
const NSInteger section = [self sectionForSectionController:sectionController];
for (UICollectionViewCell *cell in visibleCells) {
if ([collectionView indexPathForCell:cell].section == section) {
const CGRect cellRect = [cell convertRect:cell.bounds toView:collectionView];
@ -917,10 +923,16 @@
}
- (NSArray<UICollectionViewCell *> *)visibleCellsForSectionController:(IGListSectionController *)sectionController {
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.
return @[];
}
NSMutableArray *cells = [NSMutableArray new];
UICollectionView *collectionView = self.collectionView;
NSArray *visibleCells = [collectionView visibleCells];
const NSInteger section = [self sectionForSectionController:sectionController];
for (UICollectionViewCell *cell in visibleCells) {
if ([collectionView indexPathForCell:cell].section == section) {
[cells addObject:cell];
@ -930,10 +942,16 @@
}
- (NSArray<NSIndexPath *> *)visibleIndexPathsForSectionController:(IGListSectionController *) sectionController {
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.
return @[];
}
NSMutableArray *paths = [NSMutableArray new];
UICollectionView *collectionView = self.collectionView;
NSArray *visiblePaths = [collectionView indexPathsForVisibleItems];
const NSInteger section = [self sectionForSectionController:sectionController];
for (NSIndexPath *path in visiblePaths) {
if (path.section == section) {
[paths addObject:path];
@ -1105,8 +1123,16 @@
}
- (void)invalidateLayoutForSectionController:(IGListSectionController *)sectionController
completion:(void (^)(BOOL finished))completion{
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.
if (completion) {
completion(NO);
}
return;
}
const NSInteger items = [_collectionView numberOfItemsInSection:section];
NSMutableArray<NSIndexPath *> *indexPaths = [NSMutableArray new];

View file

@ -1843,4 +1843,57 @@
[mockDelegate verify];
}
#pragma mark - Deleted Section Controllers
- (void)test_whenSectionControllerRemoved_thatCellForIndexPathIsNil {
self.dataSource.objects = @[@1];
[self.adapter performUpdatesAnimated:NO completion:nil];
IGListSectionController *sectionController = [self.adapter sectionControllerForObject:@1];
self.dataSource.objects = @[@2];
[self.adapter performUpdatesAnimated:NO completion:nil];
XCTAssertNil([sectionController.collectionContext cellForItemAtIndex:0 sectionController:sectionController]);
}
- (void)test_whenSectionControllerRemoved_thatFullyVisibleCellsIsEmpty {
self.dataSource.objects = @[@1];
[self.adapter performUpdatesAnimated:NO completion:nil];
IGListSectionController *sectionController = [self.adapter sectionControllerForObject:@1];
self.dataSource.objects = @[@2];
[self.adapter performUpdatesAnimated:NO completion:nil];
NSArray<UICollectionViewCell *> *cells = [sectionController.collectionContext fullyVisibleCellsForSectionController:sectionController];
XCTAssertEqual(cells.count, 0);
}
- (void)test_whenSectionControllerRemoved_thatVisibleCellsIsEmpty {
self.dataSource.objects = @[@1];
[self.adapter performUpdatesAnimated:NO completion:nil];
IGListSectionController *sectionController = [self.adapter sectionControllerForObject:@1];
self.dataSource.objects = @[@2];
[self.adapter performUpdatesAnimated:NO completion:nil];
NSArray<UICollectionViewCell *> *cells = [sectionController.collectionContext visibleCellsForSectionController:sectionController];
XCTAssertEqual(cells.count, 0);
}
- (void)test_whenSectionControllerRemoved_thatVisibleIndexPathIsEmpty {
self.dataSource.objects = @[@1];
[self.adapter performUpdatesAnimated:NO completion:nil];
IGListSectionController *sectionController = [self.adapter sectionControllerForObject:@1];
self.dataSource.objects = @[@2];
[self.adapter performUpdatesAnimated:NO completion:nil];
NSArray<NSIndexPath *> *cells = [sectionController.collectionContext visibleIndexPathsForSectionController:sectionController];
XCTAssertEqual(cells.count, 0);
}
- (void)test_whenSectionControllerRemoved_thatDoesNotCrashOnInvalidatingLayout {
self.dataSource.objects = @[@1];
[self.adapter performUpdatesAnimated:NO completion:nil];
IGListSectionController *sectionController = [self.adapter sectionControllerForObject:@1];
self.dataSource.objects = @[@2];
[self.adapter performUpdatesAnimated:NO completion:nil];
[sectionController.collectionContext invalidateLayoutForSectionController:sectionController completion:nil];
}
@end