Clean listAdapterUpdater state if collectionView becomes nil during update

Summary:
If an `IGListAdapterUpdater` was unable to retrieve a collection view in `[IGListAdapterUpdater performBatchUpdatesWithCollectionViewBlock:]`, it would return early without cleaning state. This would sometimes cause future updates to crash, as this `IGListAdapterUpdater`'s state would now be out-of-sync.

This change also ensures that the `IGListAdapterUpdater`'s completion blocks run when `[IGListAdapterUpdater performBatchUpdatesWithCollectionViewBlock:]` and `[IGListAdapterUpdater performReloadDataWithCollectionViewBlock:]` return early due to having a nil `UICollectionView`.

Reviewed By: rnystrom

Differential Revision: D8056539

fbshipit-source-id: 1af7b675ec6805c2d8031f32d8a4c8e8929564e6
This commit is contained in:
Brandon Darin 2018-05-23 14:54:21 -07:00 committed by Facebook Github Bot
parent b4c8ea180f
commit 290d592983
3 changed files with 133 additions and 30 deletions

View file

@ -13,6 +13,9 @@ The changelog for `IGListKit`. Also see the [releases](https://github.com/instag
- Experimental fix to get the `UICollectionView` for batch updating immediately before applying the update. [Ryan Nystrom](https://github.com/rnystrom) (tbd)
- `[IGListAdapterUpdater performBatchUpdatesWithCollectionViewBlock:]` and `[IGListAdapterUpdater performReloadDataWithCollectionViewBlock:]` clean state and run completion blocks if their `UICollectionView` is nil. [Brandon Darin](https://github.com/jbd1030) (tbd)
3.4.0
-----

View file

@ -44,13 +44,7 @@
- (void)performReloadDataWithCollectionViewBlock:(IGListCollectionViewBlock)collectionViewBlock {
IGAssertMainThread();
// bail early if the collection view has been deallocated in the time since the update was queued
UICollectionView *collectionView = collectionViewBlock();
if (collectionView == nil) {
return;
}
id<IGListAdapterUpdaterDelegate> delegate = self.delegate;
void (^reloadUpdates)(void) = self.reloadUpdates;
IGListBatchUpdates *batchUpdates = self.batchUpdates;
@ -58,6 +52,22 @@
[self cleanStateBeforeUpdates];
void (^executeCompletionBlocks)(BOOL) = ^(BOOL finished) {
for (IGListUpdatingCompletion block in completionBlocks) {
block(finished);
}
self.state = IGListBatchUpdateStateIdle;
};
// bail early if the collection view has been deallocated in the time since the update was queued
UICollectionView *collectionView = collectionViewBlock();
if (collectionView == nil) {
[self _cleanStateAfterUpdates];
executeCompletionBlocks(NO);
return;
}
// item updates must not send mutations to the collection view while we are reloading
self.state = IGListBatchUpdateStateExecutingBatchUpdateBlock;
@ -86,23 +96,13 @@
[collectionView layoutIfNeeded];
[delegate listAdapterUpdater:self didReloadDataWithCollectionView:collectionView];
for (IGListUpdatingCompletion block in completionBlocks) {
block(YES);
}
self.state = IGListBatchUpdateStateIdle;
executeCompletionBlocks(YES);
}
- (void)performBatchUpdatesWithCollectionViewBlock:(IGListCollectionViewBlock)collectionViewBlock {
IGAssertMainThread();
IGAssert(self.state == IGListBatchUpdateStateIdle, @"Should not call batch updates when state isn't idle");
// bail early if the collection view has been deallocated in the time since the update was queued
UICollectionView *collectionView = collectionViewBlock();
if (collectionView == nil) {
return;
}
// create local variables so we can immediately clean our state but pass these items into the batch update block
id<IGListAdapterUpdaterDelegate> delegate = self.delegate;
NSArray *fromObjects = [self.fromObjects copy];
@ -112,6 +112,26 @@
const BOOL animated = self.queuedUpdateIsAnimated;
IGListBatchUpdates *batchUpdates = self.batchUpdates;
// clean up all state so that new updates can be coalesced while the current update is in flight
[self cleanStateBeforeUpdates];
void (^executeCompletionBlocks)(BOOL) = ^(BOOL finished) {
self.applyingUpdateData = nil;
self.state = IGListBatchUpdateStateIdle;
for (IGListUpdatingCompletion block in completionBlocks) {
block(finished);
}
};
// bail early if the collection view has been deallocated in the time since the update was queued
UICollectionView *collectionView = collectionViewBlock();
if (collectionView == nil) {
[self _cleanStateAfterUpdates];
executeCompletionBlocks(NO);
return;
}
NSArray *toObjects = nil;
if (toObjectsBlock != nil) {
toObjects = objectsWithDuplicateIdentifiersRemoved(toObjectsBlock());
@ -125,9 +145,6 @@
}
#endif
// clean up all state so that new updates can be coalesced while the current update is in flight
[self cleanStateBeforeUpdates];
void (^executeUpdateBlocks)(void) = ^{
self.state = IGListBatchUpdateStateExecutingBatchUpdateBlock;
@ -152,15 +169,6 @@
self.state = IGListBatchUpdateStateExecutedBatchUpdateBlock;
};
void (^executeCompletionBlocks)(BOOL) = ^(BOOL finished) {
self.applyingUpdateData = nil;
self.state = IGListBatchUpdateStateIdle;
for (IGListUpdatingCompletion block in completionBlocks) {
block(finished);
}
};
void (^reloadDataFallback)(void) = ^{
executeUpdateBlocks();
[self _cleanStateAfterUpdates];

View file

@ -1881,4 +1881,96 @@
[self waitForExpectationsWithTimeout:30 handler:nil];
}
- (void)test_whenCollectionViewBecomesNilDuringPerformUpdates_thatStateCleanedCorrectly {
[self setupWithObjects:@[
genTestObject(@1, @1)
]];
self.adapter.experiments |= IGListExperimentGetCollectionViewAtUpdate;
// perform update on listAdapter
XCTestExpectation *expectation1 = genExpectation;
[self.adapter performUpdatesAnimated:NO completion:^(BOOL finished) {
[expectation1 fulfill];
}];
[self waitForExpectationsWithTimeout:30 handler:nil];
// update the underlying contents before performing another update
self.dataSource.objects = @[
genTestObject(@1, @1),
genTestObject(@2, @1)
];
// perform update, but set the listAdapter's collectionView to nil during the update
XCTestExpectation *expectation2 = genExpectation;
[self.adapter performUpdatesAnimated:NO completion:^(BOOL finished) {
[expectation2 fulfill];
}];
self.adapter.collectionView = nil;
[self waitForExpectationsWithTimeout:30 handler:nil];
// add a new collectionView to the listAdapter
UICollectionView *collectionView2 = [[UICollectionView alloc] initWithFrame:self.window.frame collectionViewLayout:[UICollectionViewFlowLayout new]];
[self.window addSubview:collectionView2];
self.adapter.collectionView = collectionView2;
// update the underlying contents before performing update
self.dataSource.objects = @[
genTestObject(@1, @1),
genTestObject(@2, @1),
genTestObject(@3, @1)
];
// perform update on listAdapter (now with a non-nil collectionView)
XCTestExpectation *expectation3 = genExpectation;
[self.adapter performUpdatesAnimated:NO completion:^(BOOL finished) {
[expectation3 fulfill];
}];
[self waitForExpectationsWithTimeout:30 handler:nil];
}
- (void)test_whenCollectionViewBecomesNilDuringReloadData_thatStateCleanedCorrectly {
[self setupWithObjects:@[
genTestObject(@1, @1)
]];
self.adapter.experiments |= IGListExperimentGetCollectionViewAtUpdate;
// reload data on listAdapter
XCTestExpectation *expectation1 = genExpectation;
[self.adapter reloadDataWithCompletion:^(BOOL finished) {
[expectation1 fulfill];
}];
[self waitForExpectationsWithTimeout:30 handler:nil];
// update the underlying contents before reloading again
self.dataSource.objects = @[
genTestObject(@1, @1),
genTestObject(@2, @1)
];
// reload data, but set the listAdapter's collectionView to nil during the update
XCTestExpectation *expectation2 = genExpectation;
[self.adapter reloadDataWithCompletion:^(BOOL finished) {
[expectation2 fulfill];
}];
self.adapter.collectionView = nil;
[self waitForExpectationsWithTimeout:30 handler:nil];
// add a new collectionView to the listAdapter
UICollectionView *collectionView2 = [[UICollectionView alloc] initWithFrame:self.window.frame collectionViewLayout:[UICollectionViewFlowLayout new]];
[self.window addSubview:collectionView2];
self.adapter.collectionView = collectionView2;
self.dataSource.objects = @[
genTestObject(@1, @1),
genTestObject(@2, @1),
genTestObject(@3, @1)
];
// reload data on listAdapter (now with a non-nil collectionView)
XCTestExpectation *expectation3 = genExpectation;
[self.adapter reloadDataWithCompletion:^(BOOL finished) {
[expectation3 fulfill];
}];
[self waitForExpectationsWithTimeout:30 handler:nil];
}
@end