Skip to content

Commit 0242821

Browse files
committed
Add experiment to ensure ASCollectionView's range controller updates on changeset updates
This experiment makes sure a ASCollectionView's `rangeController` updates when a changeset WITH updates is applied. Currently it is possible for nodes inserted into the preload range to not get preloaded when performing a batch update. For example, suppose a collection node has: - Tuning parameters with a preload range of 1 screenful for the given range mode. - Nodes A and B where A is visible and B is off screen. Currently if node B is deleted and a new node C is inserted in its place, node C will not get preloaded until the collection node is scrolled.
1 parent 17d4d13 commit 0242821

5 files changed

Lines changed: 95 additions & 4 deletions

File tree

Source/ASCollectionView.mm

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2311,7 +2311,11 @@ - (void)rangeController:(ASRangeController *)rangeController updateWithChangeSet
23112311

23122312
// Flush any range changes that happened as part of submitting the update.
23132313
as_activity_scope(changeSet.rootActivity);
2314-
[self->_rangeController updateIfNeeded];
2314+
if (numberOfUpdates > 0 && ASActivateExperimentalFeature(ASExperimentalRangeUpdateOnChangesetUpdate)) {
2315+
[self->_rangeController setNeedsUpdate];
2316+
} else {
2317+
[self->_rangeController updateIfNeeded];
2318+
}
23152319
}
23162320
});
23172321
}

Source/ASExperimentalFeatures.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ typedef NS_OPTIONS(NSUInteger, ASExperimentalFeatures) {
3030
ASExperimentalOptimizeDataControllerPipeline = 1 << 9, // exp_optimize_data_controller_pipeline
3131
ASExperimentalDisableGlobalTextkitLock = 1 << 10, // exp_disable_global_textkit_lock
3232
ASExperimentalMainThreadOnlyDataController = 1 << 11, // exp_main_thread_only_data_controller
33+
ASExperimentalRangeUpdateOnChangesetUpdate = 1 << 12, // exp_range_update_on_changeset_update
3334
ASExperimentalFeatureAll = 0xFFFFFFFF
3435
};
3536

Source/ASExperimentalFeatures.mm

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@
2323
@"exp_drawing_global",
2424
@"exp_optimize_data_controller_pipeline",
2525
@"exp_disable_global_textkit_lock",
26-
@"exp_main_thread_only_data_controller"]));
26+
@"exp_main_thread_only_data_controller",
27+
@"exp_range_update_on_changeset_update"]));
2728
if (flags == ASExperimentalFeatureAll) {
2829
return allNames;
2930
}

Tests/ASCollectionViewTests.mm

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ @interface ASTextCellNodeWithSetSelectedCounter : ASTextCellNode
2424

2525
@property (nonatomic) NSUInteger setSelectedCounter;
2626
@property (nonatomic) NSUInteger applyLayoutAttributesCount;
27+
@property (nonatomic) NSUInteger didEnterPreloadStateCount;
2728

2829
@end
2930

@@ -40,6 +41,12 @@ - (void)applyLayoutAttributes:(UICollectionViewLayoutAttributes *)layoutAttribut
4041
_applyLayoutAttributesCount++;
4142
}
4243

44+
- (void)didEnterPreloadState
45+
{
46+
[super didEnterPreloadState];
47+
_didEnterPreloadStateCount++;
48+
}
49+
4350
@end
4451

4552
@interface ASTestSectionContext : NSObject <ASSectionContext>
@@ -177,7 +184,8 @@ - (void)setUp
177184
{
178185
[super setUp];
179186
ASConfiguration *config = [ASConfiguration new];
180-
config.experimentalFeatures = ASExperimentalOptimizeDataControllerPipeline;
187+
config.experimentalFeatures = ASExperimentalOptimizeDataControllerPipeline
188+
| ASExperimentalRangeUpdateOnChangesetUpdate;
181189
[ASConfigurationManager test_resetWithConfiguration:config];
182190
}
183191

@@ -518,6 +526,81 @@ - (void)testThatDeletingAndReloadingASectionThrowsAnException
518526
} completion:nil]);
519527
}
520528

529+
- (void)testItemsInsertedIntoThePreloadRangeGetPreloaded
530+
{
531+
updateValidationTestPrologue
532+
533+
ASRangeTuningParameters minimumPreloadParams = { .leadingBufferScreenfuls = 1, .trailingBufferScreenfuls = 1 };
534+
[cn setTuningParameters:minimumPreloadParams forRangeMode:ASLayoutRangeModeMinimum rangeType:ASLayoutRangeTypePreload];
535+
[cn updateCurrentRangeWithMode:ASLayoutRangeModeMinimum];
536+
537+
__weak ASCollectionViewTestController *weakController = testController;
538+
NSIndexPath *lastVisibleIndex = [cv indexPathsForVisibleItems].lastObject;
539+
540+
NSInteger itemCount = weakController.asyncDelegate->_itemCounts[lastVisibleIndex.section];
541+
BOOL isLastItemInSection = lastVisibleIndex.row == itemCount - 1;
542+
NSInteger nextItemSection = isLastItemInSection ? lastVisibleIndex.section + 1 : lastVisibleIndex.section;
543+
NSInteger nextItemRow = isLastItemInSection ? 0 : lastVisibleIndex.row + 1;
544+
545+
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.");
546+
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.");
547+
548+
NSIndexPath *nextItemIndexPath = [NSIndexPath indexPathForRow:nextItemRow inSection:nextItemSection];
549+
ASTextCellNodeWithSetSelectedCounter *nodeBeforeUpdate = (ASTextCellNodeWithSetSelectedCounter *)[cv nodeForItemAtIndexPath:nextItemIndexPath];
550+
551+
XCTestExpectation *noChangeDone = [self expectationWithDescription:@"Batch update with no changes done and completion block has been called. Tuning params set to 1 screenful."];
552+
553+
__block ASTextCellNodeWithSetSelectedCounter *nodeAfterUpdate;
554+
[cv performBatchUpdates:^{
555+
} completion:^(BOOL finished) {
556+
nodeAfterUpdate = (ASTextCellNodeWithSetSelectedCounter *)[cv nodeForItemAtIndexPath:nextItemIndexPath];
557+
[noChangeDone fulfill];
558+
}];
559+
560+
[self waitForExpectations:@[ noChangeDone ] timeout:1];
561+
562+
XCTAssertTrue(nodeBeforeUpdate == nodeAfterUpdate, @"Node should not have changed since no updates were made.");
563+
XCTAssertTrue(nodeAfterUpdate.didEnterPreloadStateCount == 1, @"Node should have been preloaded.");
564+
565+
XCTestExpectation *changeDone = [self expectationWithDescription:@"Batch update with changes done and completion block has been called. Tuning params set to 1 screenful."];
566+
567+
[cv performBatchUpdates:^{
568+
NSArray *indexPaths = @[ nextItemIndexPath ];
569+
[cv deleteItemsAtIndexPaths:indexPaths];
570+
[cv insertItemsAtIndexPaths:indexPaths];
571+
} completion:^(BOOL finished) {
572+
nodeAfterUpdate = (ASTextCellNodeWithSetSelectedCounter *)[cv nodeForItemAtIndexPath:nextItemIndexPath];
573+
[changeDone fulfill];
574+
}];
575+
576+
[self waitForExpectations:@[ changeDone ] timeout:1];
577+
578+
XCTAssertTrue(nodeBeforeUpdate != nodeAfterUpdate, @"Node should have changed after updating.");
579+
XCTAssertTrue(nodeAfterUpdate.didEnterPreloadStateCount == 1, @"New node should have been preloaded.");
580+
581+
minimumPreloadParams = { .leadingBufferScreenfuls = 0, .trailingBufferScreenfuls = 0 };
582+
[cn setTuningParameters:minimumPreloadParams forRangeMode:ASLayoutRangeModeMinimum rangeType:ASLayoutRangeTypePreload];
583+
[cn updateCurrentRangeWithMode:ASLayoutRangeModeMinimum];
584+
585+
XCTestExpectation *changeDoneZeroSreenfuls = [self expectationWithDescription:@"Batch update with changes done and completion block has been called. Tuning params set to 0 screenful."];
586+
587+
nodeBeforeUpdate = nodeAfterUpdate;
588+
__block ASTextCellNodeWithSetSelectedCounter *nodeAfterUpdateZeroSreenfuls;
589+
[cv performBatchUpdates:^{
590+
NSArray *indexPaths = @[ nextItemIndexPath ];
591+
[cv deleteItemsAtIndexPaths:indexPaths];
592+
[cv insertItemsAtIndexPaths:indexPaths];
593+
} completion:^(BOOL finished) {
594+
nodeAfterUpdateZeroSreenfuls = (ASTextCellNodeWithSetSelectedCounter *)[cv nodeForItemAtIndexPath:nextItemIndexPath];
595+
[changeDoneZeroSreenfuls fulfill];
596+
}];
597+
598+
[self waitForExpectations:@[ changeDoneZeroSreenfuls ] timeout:1];
599+
600+
XCTAssertTrue(nodeBeforeUpdate != nodeAfterUpdateZeroSreenfuls, @"Node should have changed after updating.");
601+
XCTAssertTrue(nodeAfterUpdateZeroSreenfuls.didEnterPreloadStateCount == 0, @"New node should NOT have been preloaded.");
602+
}
603+
521604
- (void)testCellNodeLayoutAttributes
522605
{
523606
updateValidationTestPrologue

Tests/ASConfigurationTests.mm

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
ASExperimentalOptimizeDataControllerPipeline,
3131
ASExperimentalDisableGlobalTextkitLock,
3232
ASExperimentalMainThreadOnlyDataController,
33+
ASExperimentalRangeUpdateOnChangesetUpdate,
3334
};
3435

3536
@interface ASConfigurationTests : ASTestCase <ASConfigurationDelegate>
@@ -53,7 +54,8 @@ + (NSArray *)names {
5354
@"exp_drawing_global",
5455
@"exp_optimize_data_controller_pipeline",
5556
@"exp_disable_global_textkit_lock",
56-
@"exp_main_thread_only_data_controller"
57+
@"exp_main_thread_only_data_controller",
58+
@"exp_range_update_on_changeset_update"
5759
];
5860
}
5961

0 commit comments

Comments
 (0)