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

Fixes crash when accessing a cell within working range updates #216

Closed
wants to merge 1 commit into from
Closed
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ This release closes the [2.0.0 milestone](https://github.com/Instagram/IGListKit
- Added new `-[IGListAdapter visibleObjects]` API. [Ryan Nystrom](https://github.com/rnystrom) [(386ae07)](https://github.com/Instagram/IGListKit/commit/386ae0786445c06e1eabf074a4181614332f155f)
- Added new `-[IGListAdapter objectForSectionController:]` API. [Ayush Saraswat](https://github.com/saraswatayu) [(#204)](https://github.com/Instagram/IGListKit/pull/204)

### Fixes

- Prevent `UICollectionView` bug when accessing a cell during working range updates. [Ryan Nystrom](https://github.com/rnystrom) [(#216)](https://github.com/Instagram/IGListKit/pull/216)

1.0.0
-----

Expand Down
8 changes: 6 additions & 2 deletions Source/IGListAdapter.m
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
@implementation IGListAdapter {
NSMapTable<UICollectionViewCell *, IGListSectionController<IGListSectionType> *> *_cellSectionControllerMap;
BOOL _isDequeuingCell;
BOOL _isSendingWorkingRangeDisplayUpdates;
}

- (void)dealloc {
Expand Down Expand Up @@ -613,7 +614,10 @@ - (void)collectionView:(UICollectionView *)collectionView willDisplayCell:(UICol

id object = [self.sectionMap objectForSection:indexPath.section];
[self.displayHandler willDisplayCell:cell forListAdapter:self sectionController:sectionController object:object indexPath:indexPath];

_isSendingWorkingRangeDisplayUpdates = YES;
[self.workingRangeHandler willDisplayItemAtIndexPath:indexPath forListAdapter:self];
_isSendingWorkingRangeDisplayUpdates = NO;
}

- (void)collectionView:(UICollectionView *)collectionView didEndDisplayingCell:(UICollectionViewCell *)cell forItemAtIndexPath:(NSIndexPath *)indexPath {
Expand Down Expand Up @@ -691,8 +695,8 @@ - (__kindof UICollectionViewCell *)cellForItemAtIndex:(NSInteger)index
IGAssertMainThread();
IGParameterAssert(sectionController != nil);

// if this is accessed while a cell is being dequeued, just return nil
if (_isDequeuingCell) {
// if this is accessed while a cell is being dequeued or displaying working range elements, just return nil
if (_isDequeuingCell || _isSendingWorkingRangeDisplayUpdates) {
return nil;
}

Expand Down
33 changes: 33 additions & 0 deletions Tests/IGListAdapterE2ETests.m
Original file line number Diff line number Diff line change
Expand Up @@ -1022,4 +1022,37 @@ - (void)test_whenBatchUpdating_withDuplicateIdentifiers_thatHaveDifferentValues_
[self waitForExpectationsWithTimeout:15 handler:nil];
}

- (void)test_whenPerformingUpdates_withWorkingRange_thatAccessingCellDoesntCrash {
[self setupWithObjects:@[
genTestObject(@1, @1),
genTestObject(@2, @1),
genTestObject(@3, @1),
]];

// section controller try to access a cell in -listAdapter:sectionControllerWillEnterWorkingRange:
// add items beyond the 100x100 frame so they access unavailable cells
self.dataSource.objects = @[
genTestObject(@1, @1),
genTestObject(@2, @1),
genTestObject(@3, @1),
genTestObject(@4, @1),
genTestObject(@5, @1),
genTestObject(@6, @1),
genTestObject(@7, @1),
genTestObject(@8, @1),
genTestObject(@9, @1),
genTestObject(@10, @1),
genTestObject(@11, @1),
];
XCTestExpectation *expectation = genExpectation;

// this will call -collectionView:performBatchUpdates:, trigger collectionView:willDisplayCell:forItemAtIndexPath:,
// which kicks off the working range logic
[self.adapter performUpdatesAnimated:YES completion:^(BOOL finished) {
[expectation fulfill];
}];

[self waitForExpectationsWithTimeout:15 handler:nil];
}

@end
6 changes: 3 additions & 3 deletions Tests/IGListWorkingRangeHandlerTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*/

Expand Down Expand Up @@ -48,7 +48,7 @@ - (UIView *)emptyViewForListAdapter:(IGListAdapter *)listAdapter {
}

- (IGListSectionController<IGListSectionType> *)listAdapter:(IGListAdapter *)listAdapter
sectionControllerForObject:(id)object {
sectionControllerForObject:(id)object {
return [_map objectForKey:object];
}

Expand Down Expand Up @@ -211,7 +211,7 @@ - (void)test_whenDisplayingItemAtPath_withWorkingRangeSizeOne_thenEndDisplayingT
// Act: Hide the first item, and watch for the second item to leave the working range.
[[mockWorkingRangeDelegate expect] listAdapter:adapter sectionControllerDidExitWorkingRange:controller2];
[adapter.workingRangeHandler didEndDisplayingItemAtIndexPath:[NSIndexPath indexPathForRow:0 inSection:0] forListAdapter:adapter];

[mockWorkingRangeDelegate verifyWithDelay:5];
}

Expand Down
2 changes: 1 addition & 1 deletion Tests/Objects/IGTestDelegateController.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

@class IGTestObject;

@interface IGTestDelegateController : IGListSectionController <IGListSectionType, IGListDisplayDelegate>
@interface IGTestDelegateController : IGListSectionController <IGListSectionType, IGListDisplayDelegate, IGListWorkingRangeDelegate>

@property (nonatomic, strong, readonly) IGTestObject *item;

Expand Down
9 changes: 9 additions & 0 deletions Tests/Objects/IGTestDelegateController.m
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ - (instancetype)init {
if (self = [super init]) {
_willDisplayCellIndexes = [NSCountedSet new];
_didEndDisplayCellIndexes = [NSCountedSet new];
self.workingRangeDelegate = self;
}
return self;
}
Expand Down Expand Up @@ -81,4 +82,12 @@ - (void)listAdapter:(IGListAdapter *)listAdapter didEndDisplayingSectionControll

- (void)listAdapter:(IGListAdapter *)listAdapter didScrollSectionController:(IGListSectionController <IGListSectionType> *)sectionController {}

#pragma mark - IGListWorkingRangeDelegate

- (void)listAdapter:(IGListAdapter *)listAdapter sectionControllerWillEnterWorkingRange:(IGListSectionController<IGListSectionType> *)sectionController {
__unused UICollectionViewCell *cell = [self.collectionContext cellForItemAtIndex:0 sectionController:self];
}

- (void)listAdapter:(IGListAdapter *)listAdapter sectionControllerDidExitWorkingRange:(IGListSectionController<IGListSectionType> *)sectionController {}

@end