From c1bf5f126197403f4a59a1540a1f2e673555514e Mon Sep 17 00:00:00 2001 From: Zhisheng Huang Date: Mon, 29 Oct 2018 16:17:26 -0700 Subject: [PATCH] Gate the preferItemReloadsForSectionReloads only to use when there is no change to the number of sections Summary: There are some crashes related to this path when the number of sections are changed when using the `preferItemReloadsForSectionReloads` optimization for item reloads. For now, this change would just limit the flag only useable when the section count is unchanged. The proper fix would be finding a repro-case for the unsafe trace. The follow-up here is adding more logging to capture the crash reason and investigate any variable that leads to this crash. Reviewed By: calimarkus Differential Revision: D12828061 fbshipit-source-id: 040e66c051c54f889ee9fbc3e4a48ab6bf93e162 --- Source/IGListAdapterUpdater.m | 5 +++-- Tests/IGListAdapterUpdaterTests.m | 8 ++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Source/IGListAdapterUpdater.m b/Source/IGListAdapterUpdater.m index 2b65bf1e..31a20706 100644 --- a/Source/IGListAdapterUpdater.m +++ b/Source/IGListAdapterUpdater.m @@ -333,8 +333,9 @@ static NSArray *convertSectionReloadToItemUpdates(NSIndexSet *sec moves = [NSSet new]; } - // Item reloads are not safe, if any section moves happened - if (moves.count == 0 && self.preferItemReloadsForSectionReloads) { + // Item reloads are not safe, if any section moves happened or there are inserts/deletes. + if (self.preferItemReloadsForSectionReloads + && moves.count == 0 && inserts.count == 0 && deletes.count == 0 && reloads.count > 0) { [reloads enumerateIndexesUsingBlock:^(NSUInteger sectionIndex, BOOL * _Nonnull stop) { NSMutableIndexSet *localIndexSet = [NSMutableIndexSet indexSetWithIndex:sectionIndex]; if (sectionIndex < [collectionView numberOfSections] diff --git a/Tests/IGListAdapterUpdaterTests.m b/Tests/IGListAdapterUpdaterTests.m index 0d2c26f7..9ee48dc7 100644 --- a/Tests/IGListAdapterUpdaterTests.m +++ b/Tests/IGListAdapterUpdaterTests.m @@ -869,15 +869,15 @@ [mockDelegate verify]; } -- (void)test_whenReloadIsCalledWithSectionInsertAndUpdate_andPreferItemReload_orderHappened { +- (void)test_whenReloadIsCalledWithSectionInsertAndUpdate_andPreferItemReload_noItemReloads { self.updater.preferItemReloadsForSectionReloads = YES; - IGListBatchUpdateData *expectedBatchUpdateData = [[IGListBatchUpdateData alloc] initWithInsertSections:[NSIndexSet indexSetWithIndex:1] - deleteSections:[NSIndexSet new] + IGListBatchUpdateData *expectedBatchUpdateData = [[IGListBatchUpdateData alloc] initWithInsertSections:[NSIndexSet indexSetWithIndexesInRange:NSMakeRange(0, 2)] + deleteSections:[NSIndexSet indexSetWithIndex:0] moveSections:[NSSet new] insertIndexPaths:@[] deleteIndexPaths:@[] - updateIndexPaths:@[[NSIndexPath indexPathForItem:0 inSection:0]] + updateIndexPaths:@[] moveIndexPaths:@[]]; NSArray *from = @[[IGSectionObject sectionWithObjects:@[@1] identifier:@"id1"]]; NSArray *to = @[[IGSectionObject sectionWithObjects:@[@2] identifier:@"id1"],