Elgato_dark/IGListKit
1
0
Fork 0
mirror of https://github.com/Instagram/IGListKit synced 2026-05-23 09:18:29 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

Prevent stale adapter:collectionView corruptions

Browse source 
Summary:
This becomes an issue pretty easily in embedded `IGListAdapter` environments (lists in lists). IGListKit creates a single `IGListSectionController` instance per object, but cells are reused within `IGListCollectionView`. If you assign `adapter.collectionView = cell.collectionView` (like we recommend in our [example](https://github.com/Instagram/IGListKit/blob/master/Examples/Examples-iOS/IGListKitExamples/SectionControllers/HorizontalSectionController.swift#L40)), then you can have many adapters with a weak ref to the **same collection view**.

Obviously when updates happen across all embedded lists, adapters could potentially update the same collection view and apply a corrupted batch update.

I proposed using `<objc/runtime.h>` and using associated objects, but since `OBJC_ASSOCIATION_ASSIGN` is not zerod-on-release reference (its just `unsafe_unretained`) I'd have to make a weak wrapped object, which is overkill.

Instead I think a global weak:weak map is fine. We d
Closes https://github.com/Instagram/IGListKit/pull/517

Differential Revision: D4623300

Pulled By: rnystrom

fbshipit-source-id: 53d2dd158923c431e793b0c8e28997e9bbf55b8b
This commit is contained in:
Ryan Nystrom 2017-02-28 07:15:35 -08:00 committed by Facebook Github Bot
parent fb5f15ed5e
commit f639cdd287
3 changed files with 82 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
CHANGELOG.md
View file

@ -53,6 +53,8 @@ This release closes the [3.0.0 milestone](https://github.com/Instagram/IGListKit
- Fix bug where scroll position would be incorrect in call to `-[IGListAdapter scrollToObject:supplementaryKinds:scrollDirection:scrollPosition:animated:` with scrollDirection/scrollPosition of UICollectionViewScrollDirectionVertical/UICollectionViewScrollPositionCenteredVertically or UICollectionViewScrollDirectionHorizontal/UICollectionViewScrollPositionCenteredHorizontally and with a collection view with nonzero contentInset. [David Yamnitsky](https://github.com/nitsky) [(5cc0fcd)](https://github.com/Instagram/IGListKit/commit/5cc0fcd1d77d6296f57ce1c298301b9881cb4d4a)
- Fix a crash when reusing collection views between embedded `IGListAdapter`s. [Ryan Nystrom](https://github.com/rnystrom) [(#517)](https://github.com/Instagram/IGListKit/pull/517)
2.1.0
-----

10
Source/IGListAdapter.m
View file

@ -64,11 +64,19 @@
- (void)setCollectionView:(IGListCollectionView *)collectionView {
IGAssertMainThread();
IGParameterAssert([collectionView isKindOfClass:[IGListCollectionView class]]);
// if collection view has been used by a different list adapter, treat it as if we were using a new collection view
// this happens when embedding a IGListCollectionView inside a UICollectionViewCell that is reused
if (_collectionView != collectionView || _collectionView.dataSource != self) {
// if the collection view was being used with another IGListAdapter (e.g. cell reuse)
// destroy the previous association so the old adapter doesn't update the wrong collection view
static NSMapTable<IGListCollectionView *, IGListAdapter *> *globalCollectionViewAdapterMap = nil;
if (globalCollectionViewAdapterMap == nil) {
globalCollectionViewAdapterMap = [NSMapTable weakToWeakObjectsMapTable];
}
[[globalCollectionViewAdapterMap objectForKey:collectionView] setCollectionView:nil];
[globalCollectionViewAdapterMap setObject:self forKey:collectionView];
// dump old registered section controllers in the case that we are changing collection views or setting for
// the first time
_registeredCellClasses = [NSMutableSet new];

71
Tests/IGListAdapterE2ETests.m
View file

@ -1365,4 +1365,75 @@
[self waitForExpectationsWithTimeout:15 handler:nil];
}
- (void)test_whenAdaptersSwapCollectionViews_thatOldAdapterDoesntUpdateOldCollectionView {
IGListAdapter *adapter1 = [[IGListAdapter alloc] initWithUpdater:[IGListAdapterUpdater new] viewController:nil workingRangeSize:0];
IGTestDelegateDataSource *dataSource1 = [IGTestDelegateDataSource new];
dataSource1.objects = @[genTestObject(@1, @2)];
adapter1.dataSource = dataSource1;
adapter1.collectionView = self.collectionView;
[self.collectionView layoutIfNeeded];
XCTAssertEqual([self.collectionView numberOfSections], 1);
XCTAssertEqual([self.collectionView numberOfItemsInSection:0], 2);
IGListAdapter *adapter2 = [[IGListAdapter alloc] initWithUpdater:[IGListAdapterUpdater new] viewController:nil workingRangeSize:0];
IGTestDelegateDataSource *dataSource2 = [IGTestDelegateDataSource new];
dataSource2.objects = @[genTestObject(@1, @1), genTestObject(@2, @1)];
adapter2.dataSource = dataSource2;
adapter2.collectionView = self.collectionView;
XCTAssertEqual([self.collectionView numberOfSections], 2);
XCTAssertEqual([self.collectionView numberOfItemsInSection:0], 1);
XCTAssertEqual([self.collectionView numberOfItemsInSection:1], 1);
dataSource1.objects = @[genTestObject(@1, @2), genTestObject(@2, @2), genTestObject(@3, @2), genTestObject(@4, @2)];
XCTestExpectation *expectation = genExpectation;
[adapter1 performUpdatesAnimated:YES completion:^(BOOL finished) {
XCTAssertEqual([self.collectionView numberOfSections], 2);
XCTAssertEqual([self.collectionView numberOfItemsInSection:0], 1);
XCTAssertEqual([self.collectionView numberOfItemsInSection:1], 1);
[expectation fulfill];
}];
[self waitForExpectationsWithTimeout:15 handler:nil];
}
- (void)test_whenAdaptersSwapCollectionViews_ {
IGListAdapter *adapter1 = [[IGListAdapter alloc] initWithUpdater:[IGListAdapterUpdater new] viewController:nil workingRangeSize:0];
IGTestDelegateDataSource *dataSource1 = [IGTestDelegateDataSource new];
dataSource1.objects = @[genTestObject(@1, @2)];
adapter1.dataSource = dataSource1;
adapter1.collectionView = self.collectionView;
[self.collectionView layoutIfNeeded];
XCTAssertEqual([self.collectionView numberOfSections], 1);
XCTAssertEqual([self.collectionView numberOfItemsInSection:0], 2);
IGListAdapter *adapter2 = [[IGListAdapter alloc] initWithUpdater:[IGListAdapterUpdater new] viewController:nil workingRangeSize:0];
IGTestDelegateDataSource *dataSource2 = [IGTestDelegateDataSource new];
dataSource2.objects = @[genTestObject(@1, @1), genTestObject(@2, @1)];
adapter2.dataSource = dataSource2;
adapter2.collectionView = self.collectionView;
[self.collectionView layoutIfNeeded];
XCTAssertEqual([self.collectionView numberOfSections], 2);
XCTAssertEqual([self.collectionView numberOfItemsInSection:0], 1);
XCTAssertEqual([self.collectionView numberOfItemsInSection:1], 1);
dataSource2.objects = @[genTestObject(@1, @2), genTestObject(@2, @1), genTestObject(@3, @1), genTestObject(@4, @1)];
XCTestExpectation *expectation = genExpectation;
[adapter2 performUpdatesAnimated:YES completion:^(BOOL finished) {
XCTAssertEqual([self.collectionView numberOfSections], 4);
XCTAssertEqual([self.collectionView numberOfItemsInSection:0], 2);
XCTAssertEqual([self.collectionView numberOfItemsInSection:1], 1);
XCTAssertEqual([self.collectionView numberOfItemsInSection:1], 1);
XCTAssertEqual([self.collectionView numberOfItemsInSection:1], 1);
[expectation fulfill];
}];
[self waitForExpectationsWithTimeout:15 handler:nil];
}
@end