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

Add experiment to ensure ASCollectionView's range controller updates … #1976

Merged
merged 1 commit into from
Mar 25, 2021
Merged
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
3 changes: 3 additions & 0 deletions Source/ASCollectionView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -2311,6 +2311,9 @@ - (void)rangeController:(ASRangeController *)rangeController updateWithChangeSet

// Flush any range changes that happened as part of submitting the update.
as_activity_scope(changeSet.rootActivity);
if (numberOfUpdates > 0 && ASActivateExperimentalFeature(ASExperimentalRangeUpdateOnChangesetUpdate)) {
[self->_rangeController setNeedsUpdate];
nguyenhuy marked this conversation as resolved.
Show resolved Hide resolved
}
[self->_rangeController updateIfNeeded];
}
});
Expand Down
1 change: 1 addition & 0 deletions Source/ASExperimentalFeatures.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ typedef NS_OPTIONS(NSUInteger, ASExperimentalFeatures) {
ASExperimentalOptimizeDataControllerPipeline = 1 << 9, // exp_optimize_data_controller_pipeline
ASExperimentalDisableGlobalTextkitLock = 1 << 10, // exp_disable_global_textkit_lock
ASExperimentalMainThreadOnlyDataController = 1 << 11, // exp_main_thread_only_data_controller
ASExperimentalRangeUpdateOnChangesetUpdate = 1 << 12, // exp_range_update_on_changeset_update
ASExperimentalFeatureAll = 0xFFFFFFFF
};

Expand Down
3 changes: 2 additions & 1 deletion Source/ASExperimentalFeatures.mm
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
@"exp_drawing_global",
@"exp_optimize_data_controller_pipeline",
@"exp_disable_global_textkit_lock",
@"exp_main_thread_only_data_controller"]));
@"exp_main_thread_only_data_controller",
@"exp_range_update_on_changeset_update"]));
if (flags == ASExperimentalFeatureAll) {
return allNames;
}
Expand Down
85 changes: 84 additions & 1 deletion Tests/ASCollectionViewTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ @interface ASTextCellNodeWithSetSelectedCounter : ASTextCellNode

@property (nonatomic) NSUInteger setSelectedCounter;
@property (nonatomic) NSUInteger applyLayoutAttributesCount;
@property (nonatomic) NSUInteger didEnterPreloadStateCount;

@end

Expand All @@ -40,6 +41,12 @@ - (void)applyLayoutAttributes:(UICollectionViewLayoutAttributes *)layoutAttribut
_applyLayoutAttributesCount++;
}

- (void)didEnterPreloadState
{
[super didEnterPreloadState];
_didEnterPreloadStateCount++;
}

@end

@interface ASTestSectionContext : NSObject <ASSectionContext>
Expand Down Expand Up @@ -177,7 +184,8 @@ - (void)setUp
{
[super setUp];
ASConfiguration *config = [ASConfiguration new];
config.experimentalFeatures = ASExperimentalOptimizeDataControllerPipeline;
config.experimentalFeatures = ASExperimentalOptimizeDataControllerPipeline
| ASExperimentalRangeUpdateOnChangesetUpdate;
[ASConfigurationManager test_resetWithConfiguration:config];
}

Expand Down Expand Up @@ -518,6 +526,81 @@ - (void)testThatDeletingAndReloadingASectionThrowsAnException
} completion:nil]);
}

- (void)testItemsInsertedIntoThePreloadRangeGetPreloaded
{
updateValidationTestPrologue

ASRangeTuningParameters minimumPreloadParams = { .leadingBufferScreenfuls = 1, .trailingBufferScreenfuls = 1 };
[cn setTuningParameters:minimumPreloadParams forRangeMode:ASLayoutRangeModeMinimum rangeType:ASLayoutRangeTypePreload];
[cn updateCurrentRangeWithMode:ASLayoutRangeModeMinimum];

__weak ASCollectionViewTestController *weakController = testController;
NSIndexPath *lastVisibleIndex = [cv indexPathsForVisibleItems].lastObject;

NSInteger itemCount = weakController.asyncDelegate->_itemCounts[lastVisibleIndex.section];
BOOL isLastItemInSection = lastVisibleIndex.row == itemCount - 1;
NSInteger nextItemSection = isLastItemInSection ? lastVisibleIndex.section + 1 : lastVisibleIndex.section;
NSInteger nextItemRow = isLastItemInSection ? 0 : lastVisibleIndex.row + 1;

XCTAssertTrue(weakController.asyncDelegate->_itemCounts.size() > nextItemSection, @"There is no items after the last visible item. Update the section/row counts so that there is one for this test to work properly.");
XCTAssertTrue(weakController.asyncDelegate->_itemCounts[nextItemSection] > nextItemRow, @"There is no items after the last visible item. Update the section/row counts so that there is one for this test to work properly.");

NSIndexPath *nextItemIndexPath = [NSIndexPath indexPathForRow:nextItemRow inSection:nextItemSection];
ASTextCellNodeWithSetSelectedCounter *nodeBeforeUpdate = (ASTextCellNodeWithSetSelectedCounter *)[cv nodeForItemAtIndexPath:nextItemIndexPath];

XCTestExpectation *noChangeDone = [self expectationWithDescription:@"Batch update with no changes done and completion block has been called. Tuning params set to 1 screenful."];

__block ASTextCellNodeWithSetSelectedCounter *nodeAfterUpdate;
[cv performBatchUpdates:^{
} completion:^(BOOL finished) {
nodeAfterUpdate = (ASTextCellNodeWithSetSelectedCounter *)[cv nodeForItemAtIndexPath:nextItemIndexPath];
[noChangeDone fulfill];
}];

[self waitForExpectations:@[ noChangeDone ] timeout:1];

XCTAssertTrue(nodeBeforeUpdate == nodeAfterUpdate, @"Node should not have changed since no updates were made.");
XCTAssertTrue(nodeAfterUpdate.didEnterPreloadStateCount == 1, @"Node should have been preloaded.");

XCTestExpectation *changeDone = [self expectationWithDescription:@"Batch update with changes done and completion block has been called. Tuning params set to 1 screenful."];

[cv performBatchUpdates:^{
NSArray *indexPaths = @[ nextItemIndexPath ];
[cv deleteItemsAtIndexPaths:indexPaths];
[cv insertItemsAtIndexPaths:indexPaths];
} completion:^(BOOL finished) {
nodeAfterUpdate = (ASTextCellNodeWithSetSelectedCounter *)[cv nodeForItemAtIndexPath:nextItemIndexPath];
[changeDone fulfill];
}];

[self waitForExpectations:@[ changeDone ] timeout:1];

XCTAssertTrue(nodeBeforeUpdate != nodeAfterUpdate, @"Node should have changed after updating.");
XCTAssertTrue(nodeAfterUpdate.didEnterPreloadStateCount == 1, @"New node should have been preloaded.");

minimumPreloadParams = { .leadingBufferScreenfuls = 0, .trailingBufferScreenfuls = 0 };
[cn setTuningParameters:minimumPreloadParams forRangeMode:ASLayoutRangeModeMinimum rangeType:ASLayoutRangeTypePreload];
[cn updateCurrentRangeWithMode:ASLayoutRangeModeMinimum];

XCTestExpectation *changeDoneZeroSreenfuls = [self expectationWithDescription:@"Batch update with changes done and completion block has been called. Tuning params set to 0 screenful."];

nodeBeforeUpdate = nodeAfterUpdate;
__block ASTextCellNodeWithSetSelectedCounter *nodeAfterUpdateZeroSreenfuls;
[cv performBatchUpdates:^{
NSArray *indexPaths = @[ nextItemIndexPath ];
[cv deleteItemsAtIndexPaths:indexPaths];
[cv insertItemsAtIndexPaths:indexPaths];
} completion:^(BOOL finished) {
nodeAfterUpdateZeroSreenfuls = (ASTextCellNodeWithSetSelectedCounter *)[cv nodeForItemAtIndexPath:nextItemIndexPath];
[changeDoneZeroSreenfuls fulfill];
}];

[self waitForExpectations:@[ changeDoneZeroSreenfuls ] timeout:1];

XCTAssertTrue(nodeBeforeUpdate != nodeAfterUpdateZeroSreenfuls, @"Node should have changed after updating.");
XCTAssertTrue(nodeAfterUpdateZeroSreenfuls.didEnterPreloadStateCount == 0, @"New node should NOT have been preloaded.");
}

- (void)testCellNodeLayoutAttributes
{
updateValidationTestPrologue
Expand Down
4 changes: 3 additions & 1 deletion Tests/ASConfigurationTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
ASExperimentalOptimizeDataControllerPipeline,
ASExperimentalDisableGlobalTextkitLock,
ASExperimentalMainThreadOnlyDataController,
ASExperimentalRangeUpdateOnChangesetUpdate,
};

@interface ASConfigurationTests : ASTestCase <ASConfigurationDelegate>
Expand All @@ -53,7 +54,8 @@ + (NSArray *)names {
@"exp_drawing_global",
@"exp_optimize_data_controller_pipeline",
@"exp_disable_global_textkit_lock",
@"exp_main_thread_only_data_controller"
@"exp_main_thread_only_data_controller",
@"exp_range_update_on_changeset_update"
];
}

Expand Down