From 8ab83504e365d4a8fda2732738357002934b8ed6 Mon Sep 17 00:00:00 2001 From: Andrew Yates Date: Fri, 2 Dec 2016 13:14:53 -0800 Subject: [PATCH 01/13] Update Readme logo (#2691) --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 6631041e76..8b7868fbb5 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -![AsyncDisplayKit](https://github.com/facebook/AsyncDisplayKit/blob/gh-pages/static/images/logo.png) +![AsyncDisplayKit](https://github.com/AsyncDisplayKit/Documentation/raw/master/docs/static/images/logo.png) [![Apps Using](https://img.shields.io/badge/Apps%20Using%20ASDK-%3E4,974-28B9FE.svg)](http://cocoapods.org/pods/AsyncDisplayKit) [![Downloads](https://img.shields.io/badge/Total%20Downloads-%3E512,000-28B9FE.svg)](http://cocoapods.org/pods/AsyncDisplayKit) From 9706b851450602fdb0c00cc2d09ba2e4acef2a01 Mon Sep 17 00:00:00 2001 From: Garrett Moon Date: Fri, 2 Dec 2016 13:29:16 -0800 Subject: [PATCH 02/13] Remove files that no longer exist from podfile exclusion (#2697) * Remove files that no longer exist from podfile exclusion * Missed some references to dealloc2main --- AsyncDisplayKit.podspec | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/AsyncDisplayKit.podspec b/AsyncDisplayKit.podspec index a3b062d43d..889cdb7e1b 100644 --- a/AsyncDisplayKit.podspec +++ b/AsyncDisplayKit.podspec @@ -30,12 +30,6 @@ Pod::Spec.new do |spec| 'AsyncDisplayKit/TextKit/ASTextKitComponents.h' ] - # ASDealloc2MainObject must be compiled with MRR - core.exclude_files = [ - 'AsyncDisplayKit/Private/_AS-objc-internal.h', - 'AsyncDisplayKit/Details/ASDealloc2MainObject.h', - 'AsyncDisplayKit/Details/ASDealloc2MainObject.m', - ] core.source_files = [ 'AsyncDisplayKit/**/*.{h,m,mm}', 'Base/*.{h,m}', @@ -46,16 +40,6 @@ Pod::Spec.new do |spec| # See https://github.com/facebook/AsyncDisplayKit/issues/1153 'AsyncDisplayKit/TextKit/*.h', ] - core.dependency 'AsyncDisplayKit/ASDealloc2MainObject' - end - - spec.subspec 'ASDealloc2MainObject' do |mrr| - mrr.requires_arc = false - mrr.source_files = [ - 'AsyncDisplayKit/Private/_AS-objc-internal.h', - 'AsyncDisplayKit/Details/ASDealloc2MainObject.h', - 'AsyncDisplayKit/Details/ASDealloc2MainObject.m', - ] end spec.subspec 'PINRemoteImage' do |pin| From 19970738025c2294e33806357e78b59e8bcd8afd Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Fri, 2 Dec 2016 14:04:05 -0800 Subject: [PATCH 03/13] Address Misc Warnings (#2698) * Address compiler warnings * Typo --- AsyncDisplayKit/ASCollectionNode.mm | 3 ++- AsyncDisplayKit/ASTableNode.mm | 3 ++- AsyncDisplayKit/Details/ASCollectionInternal.h | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/AsyncDisplayKit/ASCollectionNode.mm b/AsyncDisplayKit/ASCollectionNode.mm index faf8314caa..368ed789a7 100644 --- a/AsyncDisplayKit/ASCollectionNode.mm +++ b/AsyncDisplayKit/ASCollectionNode.mm @@ -123,7 +123,8 @@ { __weak __typeof__(self) weakSelf = self; ASDisplayNodeViewBlock collectionViewBlock = ^UIView *{ - __typeof__(self) strongSelf = weakSelf; + // Variable will be unused if event logging is off. + __unused __typeof__(self) strongSelf = weakSelf; return [[ASCollectionView alloc] _initWithFrame:frame collectionViewLayout:layout layoutFacilitator:layoutFacilitator eventLog:ASDisplayNodeGetEventLog(strongSelf)]; }; diff --git a/AsyncDisplayKit/ASTableNode.mm b/AsyncDisplayKit/ASTableNode.mm index 7ea4cc2cf5..3b9f1487f1 100644 --- a/AsyncDisplayKit/ASTableNode.mm +++ b/AsyncDisplayKit/ASTableNode.mm @@ -81,7 +81,8 @@ { __weak __typeof__(self) weakSelf = self; ASDisplayNodeViewBlock tableViewBlock = ^UIView *{ - __typeof__(self) strongSelf = weakSelf; + // Variable will be unused if event logging is off. + __unused __typeof__(self) strongSelf = weakSelf; return [[ASTableView alloc] _initWithFrame:frame style:style dataControllerClass:dataControllerClass eventLog:ASDisplayNodeGetEventLog(strongSelf)]; }; diff --git a/AsyncDisplayKit/Details/ASCollectionInternal.h b/AsyncDisplayKit/Details/ASCollectionInternal.h index 814f51d3b9..a75c8fc2a7 100644 --- a/AsyncDisplayKit/Details/ASCollectionInternal.h +++ b/AsyncDisplayKit/Details/ASCollectionInternal.h @@ -20,7 +20,7 @@ NS_ASSUME_NONNULL_BEGIN @class ASRangeController; @interface ASCollectionView () -- (instancetype)_initWithFrame:(CGRect)frame collectionViewLayout:(UICollectionViewLayout *)layout layoutFacilitator:(nullable id)layoutFacilitator eventLog:(ASEventLog*)eventLog; +- (instancetype)_initWithFrame:(CGRect)frame collectionViewLayout:(UICollectionViewLayout *)layout layoutFacilitator:(nullable id)layoutFacilitator eventLog:(nullable ASEventLog *)eventLog; @property (nonatomic, weak, readwrite) ASCollectionNode *collectionNode; @property (nonatomic, strong, readonly) ASDataController *dataController; From e361d00a732663a19afa34155f63493f9ac1ccc2 Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Fri, 2 Dec 2016 16:17:14 -0800 Subject: [PATCH 04/13] Our calculated layout is suitable for this constrainedSize, so keep using it and invalidate any pending layout that has been generated in the past. (#2706) --- AsyncDisplayKit/ASDisplayNode.mm | 3 +++ 1 file changed, 3 insertions(+) diff --git a/AsyncDisplayKit/ASDisplayNode.mm b/AsyncDisplayKit/ASDisplayNode.mm index 01bd984324..800a56c5b0 100644 --- a/AsyncDisplayKit/ASDisplayNode.mm +++ b/AsyncDisplayKit/ASDisplayNode.mm @@ -893,6 +893,9 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c) if (_calculatedDisplayNodeLayout->isValidForConstrainedSizeParentSize(constrainedSize, parentSize)) { ASDisplayNodeAssertNotNil(_calculatedDisplayNodeLayout->layout, @"-[ASDisplayNode layoutThatFits:parentSize:] _calculatedDisplayNodeLayout->layout should not be nil! %@", self); + // Our calculated layout is suitable for this constrainedSize, so keep using it and + // invalidate any pending layout that has been generated in the past. + _pendingDisplayNodeLayout = nullptr; return _calculatedDisplayNodeLayout->layout ?: [ASLayout layoutWithLayoutElement:self size:{0, 0}]; } From a2e75152f87687c89dd5566ecd86fac2ff5f09d1 Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Mon, 5 Dec 2016 10:34:03 -0800 Subject: [PATCH 05/13] [ASDisplayNode] Fix flickering for nodes that support range managed interface state (#2710) * Fix flickering for range managed nodes * Go back to old behavior to check for range managed before calling didExitPreloadState in interface state change --- AsyncDisplayKit/ASDisplayNode.mm | 8 +++++--- AsyncDisplayKitTests/ASDisplayNodeTests.m | 1 + 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/AsyncDisplayKit/ASDisplayNode.mm b/AsyncDisplayKit/ASDisplayNode.mm index 800a56c5b0..6effc4a1af 100644 --- a/AsyncDisplayKit/ASDisplayNode.mm +++ b/AsyncDisplayKit/ASDisplayNode.mm @@ -2993,12 +2993,10 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock) - (void)didExitPreloadState { if (_methodOverrides & ASDisplayNodeMethodOverrideClearFetchedData) { - if ([self supportsRangeManagedInterfaceState]) { #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wdeprecated-declarations" [self clearFetchedData]; #pragma clang diagnostic pop - } } } @@ -3068,7 +3066,11 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock) if (nowPreload) { [self didEnterPreloadState]; } else { - [self didExitPreloadState]; + // We don't want to call -didExitPreloadState on nodes that aren't being managed by a range controller. + // Otherwise we get flashing behavior from normal UIKit manipulations like navigation controller push / pop. + if ([self supportsRangeManagedInterfaceState]) { + [self didExitPreloadState]; + } } } diff --git a/AsyncDisplayKitTests/ASDisplayNodeTests.m b/AsyncDisplayKitTests/ASDisplayNodeTests.m index 6194ca57e3..23775e647f 100644 --- a/AsyncDisplayKitTests/ASDisplayNodeTests.m +++ b/AsyncDisplayKitTests/ASDisplayNodeTests.m @@ -1936,6 +1936,7 @@ static bool stringContainsPointer(NSString *description, id p) { - (void)testDidExitPreloadIsCalledWhenNodesExitPreloadRange { ASTestDisplayNode *node = [[ASTestDisplayNode alloc] init]; + [node setHierarchyState:ASHierarchyStateRangeManaged]; [node recursivelySetInterfaceState:ASInterfaceStatePreload]; [node recursivelySetInterfaceState:ASInterfaceStateDisplay]; From 19b97a68af38755750a2d693d44ee83c5433ccda Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Mon, 5 Dec 2016 11:42:26 -0800 Subject: [PATCH 06/13] Fix our assertion macros --- Base/ASAssert.h | 58 +++++++++++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/Base/ASAssert.h b/Base/ASAssert.h index 2df66b37a7..a4079c4ffc 100644 --- a/Base/ASAssert.h +++ b/Base/ASAssert.h @@ -13,46 +13,48 @@ #import #import -#define ASDisplayNodeAssertWithSignalAndLogFunction(condition, description, logFunction, ...) NSAssert(condition, description, ##__VA_ARGS__); -#define ASDisplayNodeCAssertWithSignalAndLogFunction(condition, description, logFunction, ...) NSCAssert(condition, description, ##__VA_ARGS__); -#define ASDisplayNodeAssertWithSignal(condition, description, ...) NSAssert(condition, description, ##__VA_ARGS__) -#define ASDisplayNodeCAssertWithSignal(condition, description, ...) NSCAssert(condition, description, ##__VA_ARGS__) - #define ASDISPLAYNODE_ASSERTIONS_ENABLED (!defined(NS_BLOCK_ASSERTIONS)) -#define ASDisplayNodeAssert(...) NSAssert(__VA_ARGS__) -#define ASDisplayNodeCAssert(...) NSCAssert(__VA_ARGS__) +/** + * Note: In some cases it would be sufficient to do e.g.: + * ASDisplayNodeAssert(...) NSAssert(__VA_ARGS__) + * but we prefer not to, because we want to match the autocomplete behavior of NSAssert. + * The construction listed above does not show the user what arguments are required and what are optional. + */ -#define ASDisplayNodeAssertNil(condition, description, ...) ASDisplayNodeAssertWithSignal(!(condition), nil, (description), ##__VA_ARGS__) -#define ASDisplayNodeCAssertNil(condition, description, ...) ASDisplayNodeCAssertWithSignal(!(condition), nil, (description), ##__VA_ARGS__) +#define ASDisplayNodeAssert(condition, desc, ...) NSAssert(condition, desc, ##__VA_ARGS__) +#define ASDisplayNodeCAssert(condition, desc, ...) NSCAssert(condition, desc, ##__VA_ARGS__) -#define ASDisplayNodeAssertNotNil(condition, description, ...) ASDisplayNodeAssertWithSignal((condition), nil, (description), ##__VA_ARGS__) -#define ASDisplayNodeCAssertNotNil(condition, description, ...) ASDisplayNodeCAssertWithSignal((condition), nil, (description), ##__VA_ARGS__) +#define ASDisplayNodeAssertNil(condition, desc, ...) ASDisplayNodeAssert((condition) == nil, desc, ##__VA_ARGS__) +#define ASDisplayNodeCAssertNil(condition, desc, ...) ASDisplayNodeCAssert((condition) == nil, desc, ##__VA_ARGS__) -#define ASDisplayNodeAssertImplementedBySubclass() ASDisplayNodeAssertWithSignal(NO, nil, @"This method must be implemented by subclass %@", [self class]); -#define ASDisplayNodeAssertNotInstantiable() ASDisplayNodeAssertWithSignal(NO, nil, @"This class is not instantiable."); -#define ASDisplayNodeAssertNotSupported() ASDisplayNodeAssertWithSignal(NO, nil, @"This method is not supported by class %@", [self class]); +#define ASDisplayNodeAssertNotNil(condition, desc, ...) ASDisplayNodeAssert((condition) != nil, desc, ##__VA_ARGS__) +#define ASDisplayNodeCAssertNotNil(condition, desc, ...) ASDisplayNodeCAssert((condition) != nil, desc, ##__VA_ARGS__) -#define ASDisplayNodeAssertMainThread() ASDisplayNodeAssertWithSignal(0 != pthread_main_np(), nil, @"This method must be called on the main thread") -#define ASDisplayNodeCAssertMainThread() ASDisplayNodeCAssertWithSignal(0 != pthread_main_np(), nil, @"This function must be called on the main thread") +#define ASDisplayNodeAssertImplementedBySubclass() ASDisplayNodeAssert(NO, @"This method must be implemented by subclass %@", [self class]); +#define ASDisplayNodeAssertNotInstantiable() ASDisplayNodeAssert(NO, nil, @"This class is not instantiable."); +#define ASDisplayNodeAssertNotSupported() ASDisplayNodeAssert(NO, nil, @"This method is not supported by class %@", [self class]); -#define ASDisplayNodeAssertNotMainThread() ASDisplayNodeAssertWithSignal(0 == pthread_main_np(), nil, @"This method must be called off the main thread") -#define ASDisplayNodeCAssertNotMainThread() ASDisplayNodeCAssertWithSignal(0 == pthread_main_np(), nil, @"This function must be called off the main thread") +#define ASDisplayNodeAssertMainThread() ASDisplayNodeAssert(0 != pthread_main_np(), @"This method must be called on the main thread") +#define ASDisplayNodeCAssertMainThread() ASDisplayNodeCAssert(0 != pthread_main_np(), @"This function must be called on the main thread") -#define ASDisplayNodeAssertFlag(X) ASDisplayNodeAssertWithSignal((1 == __builtin_popcount(X)), nil, nil) -#define ASDisplayNodeCAssertFlag(X) ASDisplayNodeCAssertWithSignal((1 == __builtin_popcount(X)), nil, nil) +#define ASDisplayNodeAssertNotMainThread() ASDisplayNodeAssert(0 == pthread_main_np(), @"This method must be called off the main thread") +#define ASDisplayNodeCAssertNotMainThread() ASDisplayNodeCAssert(0 == pthread_main_np(), @"This function must be called off the main thread") -#define ASDisplayNodeAssertTrue(condition) ASDisplayNodeAssertWithSignal((condition), nil, nil) -#define ASDisplayNodeCAssertTrue(condition) ASDisplayNodeCAssertWithSignal((condition), nil, nil) +#define ASDisplayNodeAssertFlag(X, desc, ...) ASDisplayNodeAssert((1 == __builtin_popcount(X)), desc, ##__VA_ARGS__) +#define ASDisplayNodeCAssertFlag(X, desc, ...) ASDisplayNodeCAssert((1 == __builtin_popcount(X)), desc, ##__VA_ARGS__) -#define ASDisplayNodeAssertFalse(condition) ASDisplayNodeAssertWithSignal(!(condition), nil, nil) -#define ASDisplayNodeCAssertFalse(condition) ASDisplayNodeCAssertWithSignal(!(condition), nil, nil) +#define ASDisplayNodeAssertTrue(condition) ASDisplayNodeAssert((condition), @"Expected %s to be true.", #condition) +#define ASDisplayNodeCAssertTrue(condition) ASDisplayNodeCAssert((condition), @"Expected %s to be true.", #condition) -#define ASDisplayNodeFailAssert(description, ...) ASDisplayNodeAssertWithSignal(NO, (description), ##__VA_ARGS__) -#define ASDisplayNodeCFailAssert(description, ...) ASDisplayNodeCAssertWithSignal(NO, (description), ##__VA_ARGS__) +#define ASDisplayNodeAssertFalse(condition) ASDisplayNodeAssert(!(condition), @"Expected %s to be false.", #condition) +#define ASDisplayNodeCAssertFalse(condition) ASDisplayNodeCAssert(!(condition), @"Expected %s to be false.", #condition) -#define ASDisplayNodeConditionalAssert(shouldTestCondition, condition, description, ...) ASDisplayNodeAssert((!(shouldTestCondition) || (condition)), nil, (description), ##__VA_ARGS__) -#define ASDisplayNodeConditionalCAssert(shouldTestCondition, condition, description, ...) ASDisplayNodeCAssert((!(shouldTestCondition) || (condition)), nil, (description), ##__VA_ARGS__) +#define ASDisplayNodeFailAssert(desc, ...) ASDisplayNodeAssert(NO, desc, ##__VA_ARGS__) +#define ASDisplayNodeCFailAssert(desc, ...) ASDisplayNodeCAssert(NO, desc, ##__VA_ARGS__) + +#define ASDisplayNodeConditionalAssert(shouldTestCondition, condition, desc, ...) ASDisplayNodeAssert((!(shouldTestCondition) || (condition)), desc, ##__VA_ARGS__) +#define ASDisplayNodeConditionalCAssert(shouldTestCondition, condition, desc, ...) ASDisplayNodeCAssert((!(shouldTestCondition) || (condition)), desc, ##__VA_ARGS__) #define ASDisplayNodeCAssertPositiveReal(description, num) ASDisplayNodeCAssert(num >= 0 && num <= CGFLOAT_MAX, @"%@ must be a real positive integer.", description) #define ASDisplayNodeCAssertInfOrPositiveReal(description, num) ASDisplayNodeCAssert(isinf(num) || (num >= 0 && num <= CGFLOAT_MAX), @"%@ must be infinite or a real positive integer.", description) From 21cad903550101f9c8f928d4d34c7784dc47c259 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Mon, 5 Dec 2016 14:51:07 -0800 Subject: [PATCH 07/13] Ensure we always fill leading screens for batching --- AsyncDisplayKit/Private/ASBatchFetching.m | 8 ++- AsyncDisplayKitTests/ASCollectionViewTests.mm | 60 +++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/AsyncDisplayKit/Private/ASBatchFetching.m b/AsyncDisplayKit/Private/ASBatchFetching.m index 22883b9d4a..539a8a1f90 100644 --- a/AsyncDisplayKit/Private/ASBatchFetching.m +++ b/AsyncDisplayKit/Private/ASBatchFetching.m @@ -61,10 +61,14 @@ BOOL ASDisplayShouldFetchBatchForContext(ASBatchContext *context, return YES; } - BOOL isScrollingTowardEnd = (ASScrollDirectionContainsDown(scrollDirection) || ASScrollDirectionContainsRight(scrollDirection)); + // If they are scrolling toward the head of content, don't batch fetch. + BOOL isScrollingTowardHead = (ASScrollDirectionContainsUp(scrollDirection) || ASScrollDirectionContainsLeft(scrollDirection)); + if (isScrollingTowardHead) { + return NO; + } CGFloat triggerDistance = viewLength * leadingScreens; CGFloat remainingDistance = contentLength - viewLength - offset; - return isScrollingTowardEnd && remainingDistance <= triggerDistance; + return remainingDistance <= triggerDistance; } diff --git a/AsyncDisplayKitTests/ASCollectionViewTests.mm b/AsyncDisplayKitTests/ASCollectionViewTests.mm index 27ad94ec7d..646dbddd46 100644 --- a/AsyncDisplayKitTests/ASCollectionViewTests.mm +++ b/AsyncDisplayKitTests/ASCollectionViewTests.mm @@ -891,4 +891,64 @@ [self waitForExpectationsWithTimeout:3 handler:nil]; } +- (void)testThatWeBatchFetchUntilLeadingScreensRequirementIsMet_Animated +{ + [self _primitiveBatchFetchingFillTestAnimated:YES]; +} + +- (void)testThatWeBatchFetchUntilLeadingScreensRequirementIsMet_Nonanimated +{ + [self _primitiveBatchFetchingFillTestAnimated:NO]; +} + +- (void)_primitiveBatchFetchingFillTestAnimated:(BOOL)animated +{ + UIWindow *window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds]; + ASCollectionViewTestController *testController = [[ASCollectionViewTestController alloc] initWithNibName:nil bundle:nil]; + window.rootViewController = testController; + + // Start with 1 empty section + __block NSInteger itemCount = 0; + testController.asyncDelegate->_itemCounts = {itemCount}; + [window makeKeyAndVisible]; + [window layoutIfNeeded]; + + ASCollectionNode *cn = testController.collectionNode; + [cn waitUntilAllUpdatesAreCommitted]; + XCTAssertGreaterThan(cn.bounds.size.height, cn.view.contentSize.height, @"Expected initial data not to fill collection view area."); + + XCTestExpectation *expectation = [self expectationWithDescription:@"Completed all batch fetches"]; + __weak ASCollectionViewTestController *weakController = testController; + __block NSInteger batchFetchCount = 0; + testController.asyncDelegate.willBeginBatchFetch = ^(ASBatchContext *context) { + dispatch_async(dispatch_get_main_queue(), ^{ + NSInteger fetchIndex = batchFetchCount++; + + NSInteger itemCount = weakController.asyncDelegate->_itemCounts[0]; + weakController.asyncDelegate->_itemCounts[0] = (itemCount + 1); + if (animated) { + [cn insertItemsAtIndexPaths:@[ [NSIndexPath indexPathForItem:itemCount inSection:0] ]]; + } else { + [cn performBatchAnimated:NO updates:^{ + [cn insertItemsAtIndexPaths:@[ [NSIndexPath indexPathForItem:itemCount inSection:0] ]]; + } completion:nil]; + } + + [context completeBatchFetching:YES]; + + // If no more batch fetches have happened in 1 second, assume we're done. + dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(1 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{ + if (fetchIndex == batchFetchCount - 1) { + [expectation fulfill]; + } + }); + }); + }; + [self waitForExpectationsWithTimeout:60 handler:nil]; + CGFloat contentHeight = cn.view.contentSize.height; + CGFloat requiredContentHeight = CGRectGetMaxY(cn.bounds) + CGRectGetHeight(cn.bounds) * cn.view.leadingScreensForBatching; + XCTAssertGreaterThan(batchFetchCount, 2); + XCTAssertGreaterThanOrEqual(contentHeight, requiredContentHeight); +} + @end From 9bc58dc024e20a6d82916c6d0007e08790dca020 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Mon, 5 Dec 2016 15:00:39 -0800 Subject: [PATCH 08/13] When not visible, only batch fetch to fill visible area --- AsyncDisplayKit/ASCollectionView.mm | 6 +++++ AsyncDisplayKit/ASTableView.mm | 6 +++++ AsyncDisplayKit/Private/ASBatchFetching.h | 4 ++- AsyncDisplayKit/Private/ASBatchFetching.m | 15 ++++++++--- AsyncDisplayKitTests/ASBatchFetchingTests.m | 28 ++++++++++----------- 5 files changed, 40 insertions(+), 19 deletions(-) diff --git a/AsyncDisplayKit/ASCollectionView.mm b/AsyncDisplayKit/ASCollectionView.mm index 5d31f49a4c..642a9fb825 100644 --- a/AsyncDisplayKit/ASCollectionView.mm +++ b/AsyncDisplayKit/ASCollectionView.mm @@ -1708,6 +1708,12 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell"; [_rangeController setNeedsUpdate]; [_rangeController updateIfNeeded]; } + + // When we aren't visible, we will only fetch up to the visible area. Now that we are visible, + // we will fetch visible area + leading screens, so we need to check. + if (visible) { + [self _checkForBatchFetching]; + } } #pragma mark ASCALayerExtendedDelegate diff --git a/AsyncDisplayKit/ASTableView.mm b/AsyncDisplayKit/ASTableView.mm index 1f0ec1d5c2..44da18d057 100644 --- a/AsyncDisplayKit/ASTableView.mm +++ b/AsyncDisplayKit/ASTableView.mm @@ -1725,6 +1725,12 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell"; [_rangeController setNeedsUpdate]; [_rangeController updateIfNeeded]; } + + // When we aren't visible, we will only fetch up to the visible area. Now that we are visible, + // we will fetch visible area + leading screens, so we need to check. + if (visible) { + [self _checkForBatchFetching]; + } } @end diff --git a/AsyncDisplayKit/Private/ASBatchFetching.h b/AsyncDisplayKit/Private/ASBatchFetching.h index a09fb898d3..7effc6bcb7 100644 --- a/AsyncDisplayKit/Private/ASBatchFetching.h +++ b/AsyncDisplayKit/Private/ASBatchFetching.h @@ -45,6 +45,7 @@ BOOL ASDisplayShouldFetchBatchForScrollView(UIScrollView Date: Mon, 5 Dec 2016 15:37:49 -0800 Subject: [PATCH 09/13] Beef up the tests --- AsyncDisplayKitTests/ASCollectionViewTests.mm | 64 ++++++++++++++----- 1 file changed, 47 insertions(+), 17 deletions(-) diff --git a/AsyncDisplayKitTests/ASCollectionViewTests.mm b/AsyncDisplayKitTests/ASCollectionViewTests.mm index 646dbddd46..76b896b774 100644 --- a/AsyncDisplayKitTests/ASCollectionViewTests.mm +++ b/AsyncDisplayKitTests/ASCollectionViewTests.mm @@ -891,31 +891,49 @@ [self waitForExpectationsWithTimeout:3 handler:nil]; } -- (void)testThatWeBatchFetchUntilLeadingScreensRequirementIsMet_Animated +- (void)testThatWeBatchFetchUntilContentRequirementIsMet_Animated { - [self _primitiveBatchFetchingFillTestAnimated:YES]; + [self _primitiveBatchFetchingFillTestAnimated:YES visible:YES controller:nil]; } -- (void)testThatWeBatchFetchUntilLeadingScreensRequirementIsMet_Nonanimated +- (void)testThatWeBatchFetchUntilContentRequirementIsMet_Nonanimated { - [self _primitiveBatchFetchingFillTestAnimated:NO]; + [self _primitiveBatchFetchingFillTestAnimated:NO visible:YES controller:nil]; } -- (void)_primitiveBatchFetchingFillTestAnimated:(BOOL)animated +- (void)testThatWeBatchFetchUntilContentRequirementIsMet_Invisible { - UIWindow *window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds]; - ASCollectionViewTestController *testController = [[ASCollectionViewTestController alloc] initWithNibName:nil bundle:nil]; - window.rootViewController = testController; + [self _primitiveBatchFetchingFillTestAnimated:NO visible:NO controller:nil]; +} +- (void)testThatWhenWeBecomeVisibleWeWillFetchAdditionalContent +{ + ASCollectionViewTestController *ctrl = [[ASCollectionViewTestController alloc] initWithNibName:nil bundle:nil]; // Start with 1 empty section - __block NSInteger itemCount = 0; - testController.asyncDelegate->_itemCounts = {itemCount}; - [window makeKeyAndVisible]; - [window layoutIfNeeded]; + ctrl.asyncDelegate->_itemCounts = {0}; + [self _primitiveBatchFetchingFillTestAnimated:NO visible:NO controller:ctrl]; + XCTAssertGreaterThan([ctrl.collectionNode numberOfItemsInSection:0], 0); + [self _primitiveBatchFetchingFillTestAnimated:NO visible:YES controller:ctrl]; +} +- (void)_primitiveBatchFetchingFillTestAnimated:(BOOL)animated visible:(BOOL)visible controller:(nullable ASCollectionViewTestController *)testController +{ + if (testController == nil) { + testController = [[ASCollectionViewTestController alloc] initWithNibName:nil bundle:nil]; + // Start with 1 empty section + testController.asyncDelegate->_itemCounts = {0}; + } ASCollectionNode *cn = testController.collectionNode; - [cn waitUntilAllUpdatesAreCommitted]; - XCTAssertGreaterThan(cn.bounds.size.height, cn.view.contentSize.height, @"Expected initial data not to fill collection view area."); + + UIWindow *window = nil; + UIView *view = nil; + if (visible) { + window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds]; + view = window; + } else { + view = cn.view; + view.frame = [UIScreen mainScreen].bounds; + } XCTestExpectation *expectation = [self expectationWithDescription:@"Completed all batch fetches"]; __weak ASCollectionViewTestController *weakController = testController; @@ -923,7 +941,7 @@ testController.asyncDelegate.willBeginBatchFetch = ^(ASBatchContext *context) { dispatch_async(dispatch_get_main_queue(), ^{ NSInteger fetchIndex = batchFetchCount++; - + NSInteger itemCount = weakController.asyncDelegate->_itemCounts[0]; weakController.asyncDelegate->_itemCounts[0] = (itemCount + 1); if (animated) { @@ -944,11 +962,23 @@ }); }); }; + window.rootViewController = testController; + + [window makeKeyAndVisible]; + [view layoutIfNeeded]; + [self waitForExpectationsWithTimeout:60 handler:nil]; CGFloat contentHeight = cn.view.contentSize.height; - CGFloat requiredContentHeight = CGRectGetMaxY(cn.bounds) + CGRectGetHeight(cn.bounds) * cn.view.leadingScreensForBatching; + CGFloat requiredContentHeight; + CGFloat itemHeight = [cn.view layoutAttributesForItemAtIndexPath:[NSIndexPath indexPathForItem:0 inSection:0]].size.height; + if (visible) { + requiredContentHeight = CGRectGetMaxY(cn.bounds) + CGRectGetHeight(cn.bounds) * cn.view.leadingScreensForBatching; + } else { + requiredContentHeight = CGRectGetMaxY(cn.bounds); + } XCTAssertGreaterThan(batchFetchCount, 2); - XCTAssertGreaterThanOrEqual(contentHeight, requiredContentHeight); + XCTAssertGreaterThanOrEqual(contentHeight, requiredContentHeight, @"Loaded too little content."); + XCTAssertLessThanOrEqual(contentHeight, requiredContentHeight + 2 * itemHeight, @"Loaded too much content."); } @end From 45c69ada364d72b20c0011512c8b378e0928e6ee Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Mon, 5 Dec 2016 16:37:44 -0800 Subject: [PATCH 10/13] Add some index path validation to avoid production crashes (#2716) --- AsyncDisplayKit/ASCollectionView.mm | 36 ++++++++++++++++++++++++++++- AsyncDisplayKit/ASTableView.mm | 36 +++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/AsyncDisplayKit/ASCollectionView.mm b/AsyncDisplayKit/ASCollectionView.mm index 642a9fb825..082832d66d 100644 --- a/AsyncDisplayKit/ASCollectionView.mm +++ b/AsyncDisplayKit/ASCollectionView.mm @@ -358,6 +358,13 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell"; [self reloadDataWithCompletion:nil]; } +- (void)scrollToItemAtIndexPath:(NSIndexPath *)indexPath atScrollPosition:(UICollectionViewScrollPosition)scrollPosition animated:(BOOL)animated +{ + if ([self validateIndexPath:indexPath]) { + [super scrollToItemAtIndexPath:indexPath atScrollPosition:scrollPosition animated:animated]; + } +} + - (void)reloadDataImmediately { ASDisplayNodeAssertMainThread(); @@ -620,8 +627,35 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell"; } } +/** + * Asserts that the index path is a valid view-index-path, and returns it if so, nil otherwise. + */ +- (nullable NSIndexPath *)validateIndexPath:(nullable NSIndexPath *)indexPath +{ + if (indexPath == nil) { + return nil; + } + + NSInteger section = indexPath.section; + if (section >= self.numberOfSections) { + ASDisplayNodeFailAssert(@"Collection view index path has invalid section %lu, section count = %lu", (unsigned long)section, (unsigned long)self.numberOfSections); + return nil; + } + + if (indexPath.item >= [self numberOfItemsInSection:section]) { + ASDisplayNodeFailAssert(@"Collection view index path has invalid item %lu in section %lu, item count = %lu", (unsigned long)indexPath.item, (unsigned long)section, (unsigned long)[self numberOfItemsInSection:section]); + return nil; + } + + return indexPath; +} + - (NSIndexPath *)convertIndexPathToCollectionNode:(NSIndexPath *)indexPath { + if ([self validateIndexPath:indexPath] == nil) { + return nil; + } + // If this is a section index path, we don't currently have a method // to do a mapping. if (indexPath.item == NSNotFound) { @@ -656,7 +690,7 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell"; - (NSIndexPath *)indexPathForNode:(ASCellNode *)cellNode { - return [_dataController completedIndexPathForNode:cellNode]; + return [self validateIndexPath:[_dataController completedIndexPathForNode:cellNode]]; } - (NSArray *)visibleNodes diff --git a/AsyncDisplayKit/ASTableView.mm b/AsyncDisplayKit/ASTableView.mm index 44da18d057..cdd36fa9e8 100644 --- a/AsyncDisplayKit/ASTableView.mm +++ b/AsyncDisplayKit/ASTableView.mm @@ -467,6 +467,13 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell"; [super reloadData]; } +- (void)scrollToRowAtIndexPath:(NSIndexPath *)indexPath atScrollPosition:(UITableViewScrollPosition)scrollPosition animated:(BOOL)animated +{ + if ([self validateIndexPath:indexPath]) { + [super scrollToRowAtIndexPath:indexPath atScrollPosition:scrollPosition animated:animated]; + } +} + - (void)relayoutItems { [_dataController relayoutAllNodes]; @@ -521,6 +528,10 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell"; - (NSIndexPath *)convertIndexPathToTableNode:(NSIndexPath *)indexPath { + if ([self validateIndexPath:indexPath] == nil) { + return nil; + } + // If this is a section index path, we don't currently have a method // to do a mapping. if (indexPath.row == NSNotFound) { @@ -553,12 +564,37 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell"; return [self indexPathForNode:cellNode waitingIfNeeded:NO]; } +/** + * Asserts that the index path is a valid view-index-path, and returns it if so, nil otherwise. + */ +- (nullable NSIndexPath *)validateIndexPath:(nullable NSIndexPath *)indexPath +{ + if (indexPath == nil) { + return nil; + } + + NSInteger section = indexPath.section; + if (section >= self.numberOfSections) { + ASDisplayNodeFailAssert(@"Table view index path has invalid section %lu, section count = %lu", (unsigned long)section, (unsigned long)self.numberOfSections); + return nil; + } + + if (indexPath.item >= [self numberOfRowsInSection:section]) { + ASDisplayNodeFailAssert(@"Table view index path has invalid item %lu in section %lu, item count = %lu", (unsigned long)indexPath.item, (unsigned long)section, (unsigned long)[self numberOfRowsInSection:section]); + return nil; + } + + return indexPath; +} + - (nullable NSIndexPath *)indexPathForNode:(ASCellNode *)cellNode waitingIfNeeded:(BOOL)wait { NSIndexPath *indexPath = [_dataController completedIndexPathForNode:cellNode]; + indexPath = [self validateIndexPath:indexPath]; if (indexPath == nil && wait) { [_dataController waitUntilAllUpdatesAreCommitted]; indexPath = [_dataController completedIndexPathForNode:cellNode]; + indexPath = [self validateIndexPath:indexPath]; } return indexPath; } From 52eb9df36666fda4b46bc3d118c991346e8d9106 Mon Sep 17 00:00:00 2001 From: Andrey Konstantinov <8ofproject@gmail.com> Date: Tue, 6 Dec 2016 22:05:23 +0300 Subject: [PATCH 11/13] Fix typo in README (#2718) --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 8b7868fbb5..d1d04855b5 100644 --- a/README.md +++ b/README.md @@ -10,10 +10,10 @@ [![Carthage compatible](https://img.shields.io/badge/Carthage-compatible-59C939.svg?style=flat)](https://github.com/Carthage/Carthage) [![Build Status](https://travis-ci.org/facebook/AsyncDisplayKit.svg)](https://travis-ci.org/facebook/AsyncDisplayKit) [![License](https://img.shields.io/cocoapods/l/AsyncDisplayKit.svg)](https://github.com/facebook/AsyncDisplayKit/blob/master/LICENSE) - + ## Installation -ASDK is available via CocoaPods or Carthage. See our [Installation](http://asyncdisplaykit.org/docs/installation.html) guide for instructions. +ASDK is available via CocoaPods or Carthage. See our [Installation](http://asyncdisplaykit.org/docs/installation.html) guide for instructions. ## Performance Gains @@ -21,7 +21,7 @@ AsyncDisplayKit's basic unit is the `node`. An ASDisplayNode is an abstraction o To keep its user interface smooth and responsive, your app should render at 60 frames per second — the gold standard on iOS. This means the main thread has one-sixtieth of a second to push each frame. That's 16 milliseconds to execute all layout and drawing code! And because of system overhead, your code usually has less than ten milliseconds to run before it causes a frame drop. -AsyncDisplayKit lets you move image decoding, text sizing and rendering, layout, and other expensive UI operations off the main thread, to keep the main thread available to respond to user interaction. +AsyncDisplayKit lets you move image decoding, text sizing and rendering, layout, and other expensive UI operations off the main thread, to keep the main thread available to respond to user interaction. ## Advanced Developer Features @@ -35,7 +35,7 @@ As the framework has grown, many features have been added that can save develope ## Getting Help -We use Slack for real-time debugging, community updates, and general talk about ASDK. [Signup](http://asdk-slack-auto-invite.herokuapp.com) youself or email AsyncDisplayKit(at)gmail.com to get an invite. +We use Slack for real-time debugging, community updates, and general talk about ASDK. [Signup](http://asdk-slack-auto-invite.herokuapp.com) yourself or email AsyncDisplayKit(at)gmail.com to get an invite. ## Contributing From d57760797aa51b93a8fbf472ce461e7c05c1f0ab Mon Sep 17 00:00:00 2001 From: Garrett Moon Date: Tue, 6 Dec 2016 11:11:00 -0800 Subject: [PATCH 12/13] Update to beta 7 of PINRemoteImage - fixes image corruption issues. (#2720) --- AsyncDisplayKit.podspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AsyncDisplayKit.podspec b/AsyncDisplayKit.podspec index 889cdb7e1b..f23bdaa276 100644 --- a/AsyncDisplayKit.podspec +++ b/AsyncDisplayKit.podspec @@ -44,7 +44,7 @@ Pod::Spec.new do |spec| spec.subspec 'PINRemoteImage' do |pin| pin.xcconfig = { 'GCC_PREPROCESSOR_DEFINITIONS' => '$(inherited) PIN_REMOTE_IMAGE=1' } - pin.dependency 'PINRemoteImage/iOS', '= 3.0.0-beta.6' + pin.dependency 'PINRemoteImage/iOS', '= 3.0.0-beta.7' pin.dependency 'PINRemoteImage/PINCache' pin.dependency 'AsyncDisplayKit/Core' end From aec8dbfb33832cafdd183f5818ddb01c8b1c3997 Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Tue, 6 Dec 2016 11:52:31 -0800 Subject: [PATCH 13/13] Fix ASDKGram deprecation warning (#2719) --- examples/ASDKgram/Sample/PhotoCellNode.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/ASDKgram/Sample/PhotoCellNode.m b/examples/ASDKgram/Sample/PhotoCellNode.m index 176cf4b608..cde6508c51 100644 --- a/examples/ASDKgram/Sample/PhotoCellNode.m +++ b/examples/ASDKgram/Sample/PhotoCellNode.m @@ -200,9 +200,9 @@ #pragma mark - Instance Methods -- (void)fetchData +- (void)didEnterPreloadState { - [super fetchData]; + [super didEnterPreloadState]; [_photoModel.commentFeed refreshFeedWithCompletionBlock:^(NSArray *newComments) { [self loadCommentsForPhoto:_photoModel];