From ab94446c630477f40a53c42f9fec64e2b6bfcdf8 Mon Sep 17 00:00:00 2001 From: Maxime Ollivier Date: Tue, 17 Sep 2024 07:11:55 -0700 Subject: [PATCH] add assert for iOS 18 cell dequeue exceptions Summary: iOS 18 has new exceptions, which can be a bit tough to debug. The common ones we see: 1. Cell dequeue outside of -cellForIndex 2. Dequeue being called more than once 3. Returning a cell that was not dequeued Lets add an assert for each. #1 and #2 can actually assert on the problematic call, which will give a nice stack strace. #3 will still have a generic stack trace, but at least we'll see the section-controller. Differential Revision: D62809660 fbshipit-source-id: 454c44598d4b21e09e6f66eb63c502b1b966a993 --- Source/IGListDiffKit/IGListAssert.h | 8 ++++++++ Source/IGListKit/IGListAdapter.m | 18 +++++++++++++++--- .../Internal/IGListAdapter+UICollectionView.m | 13 +++++++++++++ .../IGListKit/Internal/IGListAdapterInternal.h | 3 +++ 4 files changed, 39 insertions(+), 3 deletions(-) diff --git a/Source/IGListDiffKit/IGListAssert.h b/Source/IGListDiffKit/IGListAssert.h index bba78d2b3..c566e9dec 100644 --- a/Source/IGListDiffKit/IGListAssert.h +++ b/Source/IGListDiffKit/IGListAssert.h @@ -33,3 +33,11 @@ #ifndef IGAssertMainThread #define IGAssertMainThread() IGAssert( ([NSThread isMainThread] == YES), @"Must be on the main thread") #endif // IGAssertMainThread + +#ifndef IG_ASSERTIONS_ENABLED + #if !defined(NS_BLOCK_ASSERTIONS) + #define IG_ASSERTIONS_ENABLED 1 + #else + #define IG_ASSERTIONS_ENABLED 0 + #endif +#endif diff --git a/Source/IGListKit/IGListAdapter.m b/Source/IGListKit/IGListAdapter.m index 54ce8011e..518392bc7 100644 --- a/Source/IGListKit/IGListAdapter.m +++ b/Source/IGListKit/IGListAdapter.m @@ -1154,7 +1154,7 @@ - (__kindof UICollectionViewCell *)dequeueReusableCellOfClass:(Class)cellClass [self.registeredCellIdentifiers addObject:identifier]; [collectionView registerClass:cellClass forCellWithReuseIdentifier:identifier]; } - return [collectionView dequeueReusableCellWithReuseIdentifier:identifier forIndexPath:indexPath]; + return [self _dequeueReusableCellWithReuseIdentifier:identifier forIndexPath:indexPath forSectionController:sectionController]; } - (__kindof UICollectionViewCell *)dequeueReusableCellOfClass:(Class)cellClass @@ -1172,7 +1172,7 @@ - (__kindof UICollectionViewCell *)dequeueReusableCellFromStoryboardWithIdentifi UICollectionView *collectionView = self.collectionView; IGAssert(collectionView != nil, @"Reloading adapter without a collection view."); NSIndexPath *indexPath = [self indexPathForSectionController:sectionController index:index usePreviousIfInUpdateBlock:NO]; - return [collectionView dequeueReusableCellWithReuseIdentifier:identifier forIndexPath:indexPath]; + return [self _dequeueReusableCellWithReuseIdentifier:identifier forIndexPath:indexPath forSectionController:sectionController]; } - (UICollectionViewCell *)dequeueReusableCellWithNibName:(NSString *)nibName @@ -1191,7 +1191,19 @@ - (UICollectionViewCell *)dequeueReusableCellWithNibName:(NSString *)nibName UINib *nib = [UINib nibWithNibName:nibName bundle:bundle]; [collectionView registerNib:nib forCellWithReuseIdentifier:nibName]; } - return [collectionView dequeueReusableCellWithReuseIdentifier:nibName forIndexPath:indexPath]; + return [self _dequeueReusableCellWithReuseIdentifier:nibName forIndexPath:indexPath forSectionController:sectionController]; +} + +- (UICollectionViewCell *)_dequeueReusableCellWithReuseIdentifier:(NSString *)identifier forIndexPath:(NSIndexPath *)indexPath forSectionController:(IGListSectionController *)sectionController { + // These will cause a crash in iOS 18 + IGAssert(_dequeuedCells.count == 0, @"Dequeueing more than one cell (%@) for indexPath %@, section controller %@,", identifier, indexPath, sectionController); + IGAssert(_isDequeuingCell, @"Dequeueing a cell (%@) without a request from the UICollectionView for indexPath %@, section controller %@", identifier, indexPath, sectionController); + + UICollectionViewCell *const cell = [self.collectionView dequeueReusableCellWithReuseIdentifier:identifier forIndexPath:indexPath]; + if (_isDequeuingCell && cell) { + [_dequeuedCells addObject:cell]; + } + return cell; } - (__kindof UICollectionReusableView *)dequeueReusableSupplementaryViewOfKind:(NSString *)elementKind diff --git a/Source/IGListKit/Internal/IGListAdapter+UICollectionView.m b/Source/IGListKit/Internal/IGListAdapter+UICollectionView.m index 3b90857ec..e350c135b 100644 --- a/Source/IGListKit/Internal/IGListAdapter+UICollectionView.m +++ b/Source/IGListKit/Internal/IGListAdapter+UICollectionView.m @@ -44,6 +44,12 @@ - (UICollectionViewCell *)collectionView:(UICollectionView *)collectionView cell IGListSectionController *sectionController = [self sectionControllerForSection:indexPath.section]; +#if IG_ASSERTIONS_ENABLED + if (!_dequeuedCells) { + _dequeuedCells = [NSMutableSet new]; + } +#endif + // flag that a cell is being dequeued in case it tries to access a cell in the process _isDequeuingCell = YES; UICollectionViewCell *cell = [sectionController cellForItemAtIndex:indexPath.item]; @@ -52,8 +58,15 @@ - (UICollectionViewCell *)collectionView:(UICollectionView *)collectionView cell IGAssert(cell != nil, @"Returned a nil cell at indexPath <%@> from section controller: <%@>", indexPath, sectionController); if (cell) { IGAssert(cell.reuseIdentifier != nil, @"Returned a cell without a reuseIdentifier at indexPath <%@> from section controller: <%@>", indexPath, sectionController); + + if (_dequeuedCells) { + // This will cause a crash in iOS 18 + IGAssert([_dequeuedCells containsObject:cell], @"Returned a cell (%@) that was not dequeued at indexPath %@ from section controller %@", NSStringFromClass([cell class]), indexPath, sectionController); + } } + [_dequeuedCells removeAllObjects]; + // associate the section controller with the cell so that we know which section controller is using it [self mapView:cell toSectionController:sectionController]; diff --git a/Source/IGListKit/Internal/IGListAdapterInternal.h b/Source/IGListKit/Internal/IGListAdapterInternal.h index b20f3ff4a..b813d313c 100644 --- a/Source/IGListKit/Internal/IGListAdapterInternal.h +++ b/Source/IGListKit/Internal/IGListAdapterInternal.h @@ -29,7 +29,10 @@ IGListBatchContext > { __weak UICollectionView *_collectionView; + BOOL _isDequeuingCell; + NSMutableSet *_dequeuedCells; + BOOL _isDequeuingSupplementaryView; BOOL _isSendingWorkingRangeDisplayUpdates;