Prevent item reload, delete, insert collisions

Summary:
Adds a new test and fixes an item animation collision that `UICollectionView` throws on. Basically we cannot collide reloads with insert+delete.

The example in the test is trivial, but real-world situations do appear b/c of the coalescence of `IGListAdapterUpdater`. It can queue reloads and deletes/inserts at the same index paths while waiting.

- [x] All tests pass. Demo project builds and runs.
- [x] I added tests, an experiment, or detailed why my change isn't tested.
- [x] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md)
Closes https://github.com/Instagram/IGListKit/pull/325

Differential Revision: D4322005

Pulled By: rnystrom

fbshipit-source-id: 2247163c13c0873a74ac485efff2d882ca6568f9
This commit is contained in:
Ryan Nystrom 2016-12-13 13:42:19 -08:00 committed by Facebook Github Bot
parent 695440ec1f
commit 169c0d6c54
3 changed files with 109 additions and 2 deletions

View file

@ -8,8 +8,13 @@ The changelog for `IGListKit`. Also see the [releases](https://github.com/instag
This release closes the [2.1.0 milestone](https://github.com/Instagram/IGListKit/milestone/2).
### Enhancements
- Disables `prefetchEnabled` by default on `IGListCollectionView`. [Sven Bacia (#323)](https://github.com/Instagram/IGListKit/pull/323)
### Fixes
- Avoid `UICollectionView` crashes when queueing a reload and insert/delete on the same item as well as reloading an item in a section that is animating. [Ryan Nystrom](https://github.com/rnystrom) [(#325)](https://github.com/Instagram/IGListKit/pull/325)
2.0.0
-----

View file

@ -893,8 +893,29 @@
return;
}
NSArray *indexPaths = [self indexPathsFromSectionController:sectionController indexes:indexes adjustForUpdateBlock:YES];
[self.updatingDelegate reloadItemsInCollectionView:collectionView indexPaths:indexPaths];
if (self.isInUpdateBlock) {
/**
UICollectionView is not designed to support -reloadSections: or -reloadItemsAtIndexPaths: during batch updates.
Internally it appears to convert these operations to a delete+insert. However the transformation is too simple
in that it doesn't account for the item's section being moved (naturally or explicitly) and can queue animation
collisions.
If you have an object at section 2 with 4 items and attempt to reload item at index 1, you would create an
NSIndexPath at section: 2, item: 1. Within -performBatchUpdates:, UICollectionView converts this to a delete
and insert at the same NSIndexPath.
If a section were inserted at position 2, the original section 2 has naturally shifted to section 3. However,
the insert NSIndexPath is section: 2, item: 1. Now the UICollectionView has a section animation at section 2,
as well as an item insert animation at section: 2, item: 1, and it will throw an exception.
IGListAdapter tracks the before/after mapping of section controllers to make precise NSIndexPath conversions.
*/
[self deleteInSectionController:sectionController atIndexes:indexes];
[self insertInSectionController:sectionController atIndexes:indexes];
} else {
NSArray *indexPaths = [self indexPathsFromSectionController:sectionController indexes:indexes adjustForUpdateBlock:YES];
[self.updatingDelegate reloadItemsInCollectionView:collectionView indexPaths:indexPaths];
}
}
- (void)insertInSectionController:(IGListSectionController<IGListSectionType> *)sectionController atIndexes:(NSIndexSet *)indexes {

View file

@ -1055,4 +1055,85 @@
[self waitForExpectationsWithTimeout:15 handler:nil];
}
- (void)test_whenReloadingItems_withDeleteAndInsertCollision_thatUpdateCanBeApplied {
[self setupWithObjects:@[
genTestObject(@1, @1),
genTestObject(@2, @5),
genTestObject(@3, @1),
]];
IGListSectionController<IGListSectionType> *section = [self.adapter sectionControllerForObject:self.dataSource.objects[1]];
XCTestExpectation *expectation = genExpectation;
[section.collectionContext performBatchAnimated:NO updates:^{
[section.collectionContext deleteInSectionController:section atIndexes:[NSIndexSet indexSetWithIndexesInRange:NSMakeRange(1, 2)]];
[section.collectionContext insertInSectionController:section atIndexes:[NSIndexSet indexSetWithIndexesInRange:NSMakeRange(1, 2)]];
[section.collectionContext reloadInSectionController:section atIndexes:[NSIndexSet indexSetWithIndexesInRange:NSMakeRange(1, 4)]];
} completion:^(BOOL finished) {
[expectation fulfill];
}];
[self waitForExpectationsWithTimeout:15 handler:nil];
}
- (void)test_whenReloadingItems_withSectionInsertedInFront_thatUpdateCanBeApplied {
[self setupWithObjects:@[
genTestObject(@1, @1),
genTestObject(@2, @5),
genTestObject(@3, @1),
]];
IGListSectionController<IGListSectionType> *section = [self.adapter sectionControllerForObject:self.dataSource.objects[1]];
XCTestExpectation *expectation1 = genExpectation;
[section.collectionContext performBatchAnimated:NO updates:^{
[section.collectionContext reloadInSectionController:section atIndexes:[NSIndexSet indexSetWithIndexesInRange:NSMakeRange(1, 4)]];
} completion:^(BOOL finished) {
[expectation1 fulfill];
}];
self.dataSource.objects = @[
genTestObject(@1, @1),
genTestObject(@4, @1), // insert to shift object @2
genTestObject(@2, @5),
genTestObject(@3, @1),
];
XCTestExpectation *expectation2 = genExpectation;
[self.adapter performUpdatesAnimated:YES completion:^(BOOL finished) {
[expectation2 fulfill];
}];
[self waitForExpectationsWithTimeout:15 handler:nil];
}
- (void)test_whenReloadingItems_withSectionDeletedInFront_thatUpdateCanBeApplied {
[self setupWithObjects:@[
genTestObject(@1, @1),
genTestObject(@2, @5),
genTestObject(@3, @1),
]];
IGListSectionController<IGListSectionType> *section = [self.adapter sectionControllerForObject:self.dataSource.objects[1]];
XCTestExpectation *expectation1 = genExpectation;
[section.collectionContext performBatchAnimated:NO updates:^{
[section.collectionContext reloadInSectionController:section atIndexes:[NSIndexSet indexSetWithIndexesInRange:NSMakeRange(1, 4)]];
} completion:^(BOOL finished) {
[expectation1 fulfill];
}];
self.dataSource.objects = @[
genTestObject(@2, @5),
genTestObject(@3, @1),
];
XCTestExpectation *expectation2 = genExpectation;
[self.adapter performUpdatesAnimated:YES completion:^(BOOL finished) {
[expectation2 fulfill];
}];
[self waitForExpectationsWithTimeout:15 handler:nil];
}
@end