Skip to content

Commit

Permalink
clean up IGListExperimentFixCrashOnReloadObjects
Browse files Browse the repository at this point in the history
Summary: Stability metrics look neutral, so lets ship! This was expected since we had a fix on the product side already. The IGListKit fix prevents it from happening in the first place.

Reviewed By: candance

Differential Revision: D52743455

fbshipit-source-id: e92b2a578b993cb228bbe245c3a1a5ac3bdfe5cf
  • Loading branch information
Maxime Ollivier authored and facebook-github-bot committed Jan 17, 2024
1 parent 731e894 commit 99e24af
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 19 deletions.
4 changes: 1 addition & 3 deletions Source/IGListDiffKit/IGListExperiments.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,8 @@ typedef NS_OPTIONS (NSInteger, IGListExperiment) {
IGListExperimentSkipPerformUpdateIfPossible = 1 << 3,
/// Test skipping creating {view : section controller} map, which has inconsistency issue.
IGListExperimentSkipViewSectionControllerMap = 1 << 4,
/// Use the correct section index when calling -[IGListAdapter reloadObjects ...] within the update block.
IGListExperimentFixCrashOnReloadObjects = 1 << 5,
/// Throw NSInternalInconsistencyException during an update
IGListExperimentThrowOnInconsistencyException = 1 << 6
IGListExperimentThrowOnInconsistencyException = 1 << 5
};

/**
Expand Down
14 changes: 1 addition & 13 deletions Source/IGListKit/IGListAdapter.m
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,6 @@
CGFloat max;
} OffsetRange;

@interface IGListAdapter ()
// Temporary param to key the old behavior during testing
@property (nonatomic, assign) BOOL legacyIsInDataUpdateBlock;
@end

@implementation IGListAdapter {
NSMapTable<UICollectionReusableView *, IGListSectionController *> *_viewSectionControllerMap;
// An array of blocks to execute once batch updates are finished
Expand Down Expand Up @@ -915,12 +910,7 @@ - (void)_exitBatchUpdates {
}

- (BOOL)isInDataUpdateBlock {
if (IGListExperimentEnabled(_experiments, IGListExperimentFixCrashOnReloadObjects)) {
// The updater has a better picture of when we're updating the data, including both section and item level changes.
return self.updater.isInDataUpdateBlock;
} else {
return self.legacyIsInDataUpdateBlock;
}
return self.updater.isInDataUpdateBlock;
}

#pragma mark - UIScrollViewDelegate
Expand Down Expand Up @@ -1293,10 +1283,8 @@ - (void)performBatchAnimated:(BOOL)animated updates:(void (^)(id<IGListBatchCont
[self _enterBatchUpdates];
__weak __typeof__(self) weakSelf = self;
[self.updater performUpdateWithCollectionViewBlock:[self _collectionViewBlock] animated:animated itemUpdates:^{
weakSelf.legacyIsInDataUpdateBlock = YES;
// the adapter acts as the batch context with its API stripped to just the IGListBatchContext protocol
updates(weakSelf);
weakSelf.legacyIsInDataUpdateBlock = NO;
} completion: ^(BOOL finished) {
[weakSelf _updateBackgroundView];
[weakSelf _notifyDidUpdate:IGListAdapterUpdateTypeItemUpdates animated:animated];
Expand Down
3 changes: 0 additions & 3 deletions Tests/IGListAdapterE2ETests.m
Original file line number Diff line number Diff line change
Expand Up @@ -669,9 +669,6 @@ - (void)test_whenReloadingItems_withPerformUpdates_thatReloadIsApplied {
}

- (void)test_whenReloadingItems_inItemUpdateBlock_thatDoesntCrash {
self.adapter.experiments |= IGListExperimentFixCrashOnReloadObjects;
// Without this fix, this test crashes with: Terminating app due to uncaught exception 'NSRangeException', reason: '*** -[__NSArrayI objectAtIndexedSubscript:]: index 2 beyond bounds [0 .. 1]'

[self setupWithObjects:@[
genTestObject(@1, @1),
genTestObject(@2, @2),
Expand Down

0 comments on commit 99e24af

Please sign in to comment.