Skip to content

Commit 4f2496b

Browse files
rqueuerqueue
committed
Expand ASExperimentalRangeUpdateOnChangesetUpdate to ASTableView (TextureGroup#1979)
A previous commit (TextureGroup@8f7444e) aimed to fix a preloading bug for ASCollectionView. This commit expands this fix to ASTableView as the bug occurs there too. Previous commit message for context: 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. This is because the preloading mechanism relies on a `setNeedsUpdate` call on the range controller as part of the `-collectionView:willDisplayCell:forItemAtIndexPath:` delegate method when the batch update is submitted. However, in the example outlined above, this sometimes doesn't happen automtically, causing the range update to be delayed until the next the view scrolls.
1 parent 4438535 commit 4f2496b

File tree

2 files changed

+129
-1
lines changed

2 files changed

+129
-1
lines changed

‎Source/ASTableView.mm‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1679,6 +1679,9 @@ - (void)rangeController:(ASRangeController *)rangeController updateWithChangeSet
16791679
LOG(@"--- UITableView endUpdates");
16801680
ASPerformBlockWithoutAnimation(!changeSet.animated, ^{
16811681
[super endUpdates];
1682+
if (numberOfUpdates > 0 && ASActivateExperimentalFeature(ASExperimentalRangeUpdateOnChangesetUpdate)) {
1683+
[self->_rangeController setNeedsUpdate];
1684+
}
16821685
[self->_rangeController updateIfNeeded];
16831686
[self _scheduleCheckForBatchFetchingForNumberOfChanges:numberOfUpdates];
16841687
});

‎Tests/ASTableViewTests.mm‎

Lines changed: 126 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ - (void)dealloc
118118
@interface ASTestTextCellNode : ASTextCellNode
119119
/** Calculated by counting how many times -layoutSpecThatFits: is called on the main thread. */
120120
@property (nonatomic) int numberOfLayoutsOnMainThread;
121+
@property (nonatomic) NSUInteger didEnterPreloadStateCount;
121122
@end
122123

123124
@implementation ASTestTextCellNode
@@ -130,6 +131,12 @@ - (ASLayoutSpec *)layoutSpecThatFits:(ASSizeRange)constrainedSize
130131
return [super layoutSpecThatFits:constrainedSize];
131132
}
132133

134+
- (void)didEnterPreloadState
135+
{
136+
[super didEnterPreloadState];
137+
_didEnterPreloadStateCount++;
138+
}
139+
133140
@end
134141

135142
@interface ASTableViewFilledDataSource : NSObject <ASTableDataSource, ASTableDelegate>
@@ -232,6 +239,36 @@ - (ASSizeRange)tableView:(ASTableView *)tableView constrainedSizeForRowAtIndexPa
232239

233240
@end
234241

242+
@interface ATableViewTestController: UIViewController
243+
244+
@property (nonatomic) ASTableNode *tableNode;
245+
@property (nonatomic) ASTableViewFilledDataSource *dataSource;
246+
247+
@end
248+
249+
@implementation ATableViewTestController
250+
251+
- (instancetype)initWithNibName:(NSString *)nibNameOrNil bundle:(NSBundle *)nibBundleOrNil {
252+
self = [super initWithNibName:nibNameOrNil bundle:nibBundleOrNil];
253+
if (self) {
254+
ASTableNode *tableNode = [[ASTableNode alloc] initWithStyle:UITableViewStylePlain];
255+
tableNode.frame = CGRectMake(0, 0, 100, 500);
256+
257+
ASTableViewFilledDataSource *dataSource = [ASTableViewFilledDataSource new];
258+
259+
tableNode.delegate = dataSource;
260+
tableNode.dataSource = dataSource;
261+
262+
self.tableNode = tableNode;
263+
self.dataSource = dataSource;
264+
265+
[self.view addSubview:self.tableNode.view];
266+
}
267+
return self;
268+
}
269+
270+
@end
271+
235272
@interface ASTableViewTests : ASTestCase
236273
@property (nonatomic, retain) ASTableView *testTableView;
237274
@end
@@ -242,7 +279,8 @@ - (void)setUp
242279
{
243280
[super setUp];
244281
ASConfiguration *config = [ASConfiguration new];
245-
config.experimentalFeatures = ASExperimentalOptimizeDataControllerPipeline;
282+
config.experimentalFeatures = ASExperimentalOptimizeDataControllerPipeline
283+
| ASExperimentalRangeUpdateOnChangesetUpdate;
246284
[ASConfigurationManager test_resetWithConfiguration:config];
247285
}
248286

@@ -761,6 +799,93 @@ - (void)testThatNilBatchUpdatesCanBeSubmitted
761799
[node performBatchAnimated:NO updates:nil completion:nil];
762800
}
763801

802+
- (void)testItemsInsertedIntoThePreloadRangeGetPreloaded
803+
{
804+
// Start table node setup
805+
ATableViewTestController *testController = [[ATableViewTestController alloc] initWithNibName:nil bundle:nil];
806+
ASTableNode *tableNode = testController.tableNode;
807+
ASTableViewFilledDataSource *dataSource = testController.dataSource;
808+
809+
UIWindow *window = [[UIWindow alloc] initWithFrame:[[UIScreen mainScreen] bounds]];
810+
window.rootViewController = testController;
811+
[window makeKeyAndVisible];
812+
813+
ASRangeTuningParameters minimumPreloadParams = { .leadingBufferScreenfuls = 1, .trailingBufferScreenfuls = 1 };
814+
[tableNode setTuningParameters:minimumPreloadParams forRangeMode:ASLayoutRangeModeMinimum rangeType:ASLayoutRangeTypePreload];
815+
[tableNode updateCurrentRangeWithMode:ASLayoutRangeModeMinimum];
816+
817+
[tableNode reloadData];
818+
[tableNode waitUntilAllUpdatesAreProcessed];
819+
[testController.tableNode.view layoutIfNeeded];
820+
// End table node setup
821+
822+
823+
NSIndexPath *lastVisibleIndex = [[tableNode indexPathsForVisibleRows] sortedArrayUsingSelector:@selector(compare:)].lastObject;
824+
825+
NSInteger itemCount = dataSource.rowsPerSection;
826+
BOOL isLastItemInSection = lastVisibleIndex.row == itemCount - 1;
827+
NSInteger nextItemSection = isLastItemInSection ? lastVisibleIndex.section + 1 : lastVisibleIndex.section;
828+
NSInteger nextItemRow = isLastItemInSection ? 0 : lastVisibleIndex.row + 1;
829+
830+
XCTAssertTrue(dataSource.numberOfSections > 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.");
831+
XCTAssertTrue(dataSource.rowsPerSection > 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.");
832+
833+
NSIndexPath *nextItemIndexPath = [NSIndexPath indexPathForRow:nextItemRow inSection:nextItemSection];
834+
ASTestTextCellNode *nodeBeforeUpdate = (ASTestTextCellNode *)[tableNode nodeForRowAtIndexPath:nextItemIndexPath];
835+
836+
XCTestExpectation *noChangeDone = [self expectationWithDescription:@"Batch update with no changes done and completion block has been called. Tuning params set to 1 screenful."];
837+
838+
__block ASTestTextCellNode *nodeAfterUpdate;
839+
[tableNode performBatchUpdates:^{
840+
} completion:^(BOOL finished) {
841+
nodeAfterUpdate = (ASTestTextCellNode *)[tableNode nodeForRowAtIndexPath:nextItemIndexPath];
842+
[noChangeDone fulfill];
843+
}];
844+
845+
[self waitForExpectations:@[ noChangeDone ] timeout:1];
846+
847+
XCTAssertTrue(nodeBeforeUpdate == nodeAfterUpdate, @"Node should not have changed since no updates were made.");
848+
XCTAssertTrue(nodeAfterUpdate.didEnterPreloadStateCount == 1, @"Node should have been preloaded.");
849+
850+
XCTestExpectation *changeDone = [self expectationWithDescription:@"Batch update with changes done and completion block has been called. Tuning params set to 1 screenful."];
851+
852+
[tableNode performBatchUpdates:^{
853+
NSArray *indexPaths = @[ nextItemIndexPath ];
854+
[tableNode deleteRowsAtIndexPaths:indexPaths withRowAnimation:UITableViewRowAnimationNone];
855+
[tableNode insertRowsAtIndexPaths:indexPaths withRowAnimation:UITableViewRowAnimationNone];
856+
} completion:^(BOOL finished) {
857+
nodeAfterUpdate = (ASTestTextCellNode *)[tableNode nodeForRowAtIndexPath:nextItemIndexPath];
858+
[changeDone fulfill];
859+
}];
860+
861+
[self waitForExpectations:@[ changeDone ] timeout:1];
862+
863+
XCTAssertTrue(nodeBeforeUpdate != nodeAfterUpdate, @"Node should have changed after updating.");
864+
XCTAssertTrue(nodeAfterUpdate.didEnterPreloadStateCount == 1, @"New node should have been preloaded.");
865+
866+
minimumPreloadParams = { .leadingBufferScreenfuls = 0, .trailingBufferScreenfuls = 0 };
867+
[tableNode setTuningParameters:minimumPreloadParams forRangeMode:ASLayoutRangeModeMinimum rangeType:ASLayoutRangeTypePreload];
868+
[tableNode updateCurrentRangeWithMode:ASLayoutRangeModeMinimum];
869+
870+
XCTestExpectation *changeDoneZeroSreenfuls = [self expectationWithDescription:@"Batch update with changes done and completion block has been called. Tuning params set to 0 screenful."];
871+
872+
nodeBeforeUpdate = nodeAfterUpdate;
873+
__block ASTestTextCellNode *nodeAfterUpdateZeroSreenfuls;
874+
[tableNode performBatchUpdates:^{
875+
NSArray *indexPaths = @[ nextItemIndexPath ];
876+
[tableNode deleteRowsAtIndexPaths:indexPaths withRowAnimation:UITableViewRowAnimationNone];
877+
[tableNode insertRowsAtIndexPaths:indexPaths withRowAnimation:UITableViewRowAnimationNone];
878+
} completion:^(BOOL finished) {
879+
nodeAfterUpdateZeroSreenfuls = (ASTestTextCellNode *)[tableNode nodeForRowAtIndexPath:nextItemIndexPath];
880+
[changeDoneZeroSreenfuls fulfill];
881+
}];
882+
883+
[self waitForExpectations:@[ changeDoneZeroSreenfuls ] timeout:1];
884+
885+
XCTAssertTrue(nodeBeforeUpdate != nodeAfterUpdateZeroSreenfuls, @"Node should have changed after updating.");
886+
XCTAssertTrue(nodeAfterUpdateZeroSreenfuls.didEnterPreloadStateCount == 0, @"New node should NOT have been preloaded.");
887+
}
888+
764889
// https://github.com/facebook/AsyncDisplayKit/issues/2252#issuecomment-263689979
765890
- (void)testIssue2252
766891
{

0 commit comments

Comments
 (0)