diff --git a/CHANGELOG.md b/CHANGELOG.md index 471615641..5716f6857 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## master * Add your own contributions to the next release on the line below this with your name. +- [ASRunloopQueue] Introduce new runloop queue(ASCATransactionQueue) to coalesce Interface state update calls for view controller transitions. - [ASRangeController] Fix stability of "minimum" rangeMode if the app has more than one layout before scrolling. - **Important** ASDisplayNode's cornerRadius is a new thread-safe bridged property that should be preferred over CALayer's. Use the latter at your own risk! [Huy Nguyen](https://github.com/nguyenhuy) [#749](https://github.com/TextureGroup/Texture/pull/749). - [ASCellNode] Adds mapping for UITableViewCell focusStyle [Alex Hill](https://github.com/alexhillc) [#727](https://github.com/TextureGroup/Texture/pull/727) diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm index 1e772e514..5d6f305d3 100644 --- a/Source/ASDisplayNode.mm +++ b/Source/ASDisplayNode.mm @@ -63,7 +63,7 @@ // We have to forward declare the protocol as this place otherwise it will not compile compiling with an Base SDK < iOS 10 @protocol CALayerDelegate; -@interface ASDisplayNode () +@interface ASDisplayNode () /** * See ASDisplayNodeInternal.h for ivars @@ -2739,7 +2739,7 @@ - (void)setHierarchyState:(ASHierarchyState)newState // Entered or exited range managed state. if ((newState & ASHierarchyStateRangeManaged) != (oldState & ASHierarchyStateRangeManaged)) { if (newState & ASHierarchyStateRangeManaged) { - [self enterInterfaceState:self.supernode.interfaceState]; + [self enterInterfaceState:self.supernode.pendingInterfaceState]; } else { // The case of exiting a range-managed state should be fairly rare. Adding or removing the node // to a view hierarchy will cause its interfaceState to be either fully set or unset (all fields), @@ -2782,30 +2782,34 @@ - (void)didExitHierarchy ASDisplayNodeAssert(_flags.isExitingHierarchy, @"You should never call -didExitHierarchy directly. Appearance is automatically managed by ASDisplayNode"); ASDisplayNodeAssert(!_flags.isEnteringHierarchy, @"ASDisplayNode inconsistency. __enterHierarchy and __exitHierarchy are mutually exclusive"); ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); - - if (![self supportsRangeManagedInterfaceState]) { - self.interfaceState = ASInterfaceStateNone; - } else { - // This case is important when tearing down hierarchies. We must deliver a visibileStateDidChange:NO callback, as part our API guarantee that this method can be used for - // things like data analytics about user content viewing. We cannot call the method in the dealloc as any incidental retain operations in client code would fail. - // Additionally, it may be that a Standard UIView which is containing us is moving between hierarchies, and we should not send the call if we will be re-added in the - // same runloop. Strategy: strong reference (might be the last!), wait one runloop, and confirm we are still outside the hierarchy (both layer-backed and view-backed). - // TODO: This approach could be optimized by only performing the dispatch for root elements + recursively apply the interface state change. This would require a closer - // integration with _ASDisplayLayer to ensure that the superlayer pointer has been cleared by this stage (to check if we are root or not), or a different delegate call. - - if (ASInterfaceStateIncludesVisible(self.interfaceState)) { - dispatch_async(dispatch_get_main_queue(), ^{ - // This block intentionally retains self. - __instanceLock__.lock(); - unsigned isInHierarchy = _flags.isInHierarchy; - BOOL isVisible = ASInterfaceStateIncludesVisible(_interfaceState); - ASInterfaceState newState = (_interfaceState & ~ASInterfaceStateVisible); - __instanceLock__.unlock(); - - if (!isInHierarchy && isVisible) { - self.interfaceState = newState; + + // This case is important when tearing down hierarchies. We must deliver a visibileStateDidChange:NO callback, as part our API guarantee that this method can be used for + // things like data analytics about user content viewing. We cannot call the method in the dealloc as any incidental retain operations in client code would fail. + // Additionally, it may be that a Standard UIView which is containing us is moving between hierarchies, and we should not send the call if we will be re-added in the + // same runloop. Strategy: strong reference (might be the last!), wait one runloop, and confirm we are still outside the hierarchy (both layer-backed and view-backed). + // TODO: This approach could be optimized by only performing the dispatch for root elements + recursively apply the interface state change. This would require a closer + // integration with _ASDisplayLayer to ensure that the superlayer pointer has been cleared by this stage (to check if we are root or not), or a different delegate call. + if (ASInterfaceStateIncludesVisible(_pendingInterfaceState)) { + void(^exitVisibleInterfaceState)(void) = ^{ + // This block intentionally retains self. + __instanceLock__.lock(); + unsigned isStillInHierarchy = _flags.isInHierarchy; + BOOL isVisible = ASInterfaceStateIncludesVisible(_pendingInterfaceState); + ASInterfaceState newState = (_pendingInterfaceState & ~ASInterfaceStateVisible); + __instanceLock__.unlock(); + + if (!isStillInHierarchy && isVisible) { + if (![self supportsRangeManagedInterfaceState]) { + newState = ASInterfaceStateNone; } - }); + self.interfaceState = newState; + } + }; + + if ([[ASCATransactionQueue sharedQueue] disabled]) { + dispatch_async(dispatch_get_main_queue(), exitVisibleInterfaceState); + } else { + exitVisibleInterfaceState(); } } } @@ -2866,25 +2870,53 @@ - (ASInterfaceState)interfaceState } - (void)setInterfaceState:(ASInterfaceState)newState +{ + if ([[ASCATransactionQueue sharedQueue] disabled]) { + [self applyPendingInterfaceState:newState]; + } else { + ASDN::MutexLocker l(__instanceLock__); + if (_pendingInterfaceState != newState) { + _pendingInterfaceState = newState; + [[ASCATransactionQueue sharedQueue] enqueue:self]; + } + } +} + +- (ASInterfaceState)pendingInterfaceState +{ + ASDN::MutexLocker l(__instanceLock__); + return _pendingInterfaceState; +} + +- (void)applyPendingInterfaceState:(ASInterfaceState)newPendingState { //This method is currently called on the main thread. The assert has been added here because all of the //did(Enter|Exit)(Display|Visible|Preload)State methods currently guarantee calling on main. ASDisplayNodeAssertMainThread(); - // It should never be possible for a node to be visible but not be allowed / expected to display. - ASDisplayNodeAssertFalse(ASInterfaceStateIncludesVisible(newState) && !ASInterfaceStateIncludesDisplay(newState)); + // This method manages __instanceLock__ itself, to ensure the lock is not held while didEnter/Exit(.*)State methods are called, thus avoid potential deadlocks ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); ASInterfaceState oldState = ASInterfaceStateNone; + ASInterfaceState newState = ASInterfaceStateNone; { ASDN::MutexLocker l(__instanceLock__); - if (_interfaceState == newState) { - return; + // newPendingState will not be used when ASCATransactionQueue is enabled + // and use _pendingInterfaceState instead for interfaceState update. + if ([[ASCATransactionQueue sharedQueue] disabled]) { + _pendingInterfaceState = newPendingState; } oldState = _interfaceState; + newState = _pendingInterfaceState; + if (newState == oldState) { + return; + } _interfaceState = newState; } + // It should never be possible for a node to be visible but not be allowed / expected to display. + ASDisplayNodeAssertFalse(ASInterfaceStateIncludesVisible(newState) && !ASInterfaceStateIncludesDisplay(newState)); + // TODO: Trigger asynchronous measurement if it is not already cached or being calculated. // if ((newState & ASInterfaceStateMeasureLayout) != (oldState & ASInterfaceStateMeasureLayout)) { // } @@ -2981,6 +3013,12 @@ - (void)setInterfaceState:(ASInterfaceState)newState [self interfaceStateDidChange:newState fromState:oldState]; } +- (void)prepareForCATransactionCommit +{ + // Apply _pendingInterfaceState actual _interfaceState, note that ASInterfaceStateNone is not used. + [self applyPendingInterfaceState:ASInterfaceStateNone]; +} + - (void)interfaceStateDidChange:(ASInterfaceState)newState fromState:(ASInterfaceState)oldState { // Subclass hook diff --git a/Source/ASRunLoopQueue.h b/Source/ASRunLoopQueue.h index 50a0eeb24..02711910b 100644 --- a/Source/ASRunLoopQueue.h +++ b/Source/ASRunLoopQueue.h @@ -20,8 +20,15 @@ NS_ASSUME_NONNULL_BEGIN +@protocol ASCATransactionQueueObserving +- (void)prepareForCATransactionCommit; +@end + +@interface ASAbstractRunLoopQueue : NSObject +@end + AS_SUBCLASSING_RESTRICTED -@interface ASRunLoopQueue : NSObject +@interface ASRunLoopQueue : ASAbstractRunLoopQueue /** * Create a new queue with the given run loop and handler. @@ -41,13 +48,37 @@ AS_SUBCLASSING_RESTRICTED - (void)enqueue:(ObjectType)object; -@property (nonatomic, readonly) BOOL isEmpty; +@property (atomic, readonly) BOOL isEmpty; @property (nonatomic, assign) NSUInteger batchSize; // Default == 1. @property (nonatomic, assign) BOOL ensureExclusiveMembership; // Default == YES. Set-like behavior. @end +AS_SUBCLASSING_RESTRICTED +@interface ASCATransactionQueue : ASAbstractRunLoopQueue + +@property (atomic, readonly) BOOL isEmpty; +@property (atomic, readonly) BOOL disabled; +/** + * The queue to run on main run loop before CATransaction commit. + * + * @discussion this queue will run after ASRunLoopQueue and before CATransaction commit + * to get last chance of updating/coalesce info like interface state. + * Each node will only be called once per transaction commit to reflect interface change. + */ +@property (class, atomic, readonly) ASCATransactionQueue *sharedQueue; + +- (void)enqueue:(id)object; + +/** + * @abstract Apply a node's interfaceState immediately rather than adding to the queue. + */ +- (void)disable; + +@end + + AS_SUBCLASSING_RESTRICTED @interface ASDeallocQueue : NSObject diff --git a/Source/ASRunLoopQueue.mm b/Source/ASRunLoopQueue.mm index 974e34814..12265d1df 100644 --- a/Source/ASRunLoopQueue.mm +++ b/Source/ASRunLoopQueue.mm @@ -181,27 +181,6 @@ - (void)dealloc @end -#pragma mark - ASRunLoopQueue - -@interface ASRunLoopQueue () { - CFRunLoopRef _runLoop; - CFRunLoopSourceRef _runLoopSource; - CFRunLoopObserverRef _runLoopObserver; - NSPointerArray *_internalQueue; // Use NSPointerArray so we can decide __strong or __weak per-instance. - ASDN::RecursiveMutex _internalQueueLock; - - // In order to not pollute the top-level activities, each queue has 1 root activity. - os_activity_t _rootActivity; - -#if ASRunLoopQueueLoggingEnabled - NSTimer *_runloopQueueLoggingTimer; -#endif -} - -@property (nonatomic, copy) void (^queueConsumer)(id dequeuedItem, BOOL isQueueDrained); - -@end - #if AS_KDEBUG_ENABLE /** * This is real, private CA API. Valid as of iOS 10. @@ -218,7 +197,23 @@ + (int)currentState; @end #endif -@implementation ASRunLoopQueue +#pragma mark - ASAbstractRunLoopQueue + +@interface ASAbstractRunLoopQueue (Private) ++ (void)load; ++ (void)registerCATransactionObservers; +@end + +@implementation ASAbstractRunLoopQueue + +- (instancetype)init +{ + if (self != [super init]) { + return nil; + } + ASDisplayNodeAssert(self.class != [ASAbstractRunLoopQueue class], @"Should never create instances of abstract class ASAbstractRunLoopQueue."); + return self; +} #if AS_KDEBUG_ENABLE + (void)load @@ -265,6 +260,31 @@ + (void)registerCATransactionObservers #endif // AS_KDEBUG_ENABLE +@end + +#pragma mark - ASRunLoopQueue + +@interface ASRunLoopQueue () { + CFRunLoopRef _runLoop; + CFRunLoopSourceRef _runLoopSource; + CFRunLoopObserverRef _runLoopObserver; + NSPointerArray *_internalQueue; // Use NSPointerArray so we can decide __strong or __weak per-instance. + ASDN::RecursiveMutex _internalQueueLock; + + // In order to not pollute the top-level activities, each queue has 1 root activity. + os_activity_t _rootActivity; + +#if ASRunLoopQueueLoggingEnabled + NSTimer *_runloopQueueLoggingTimer; +#endif +} + +@property (nonatomic, copy) void (^queueConsumer)(id dequeuedItem, BOOL isQueueDrained); + +@end + +@implementation ASRunLoopQueue + - (instancetype)initWithRunLoop:(CFRunLoopRef)runloop retainObjects:(BOOL)retainsObjects handler:(void (^)(id _Nullable, BOOL))handlerBlock { if (self = [super init]) { @@ -464,3 +484,232 @@ - (void)unlock } @end + +#pragma mark - ASCATransactionQueue + +@interface ASCATransactionQueue () { + CFRunLoopRef _runLoop; + CFRunLoopSourceRef _runLoopSource; + CFRunLoopObserverRef _preTransactionObserver; + CFRunLoopObserverRef _postTransactionObserver; + NSPointerArray *_internalQueue; + ASDN::RecursiveMutex _internalQueueLock; + BOOL _disableInterfaceStateCoalesce; + BOOL _CATransactionCommitInProgress; + + // In order to not pollute the top-level activities, each queue has 1 root activity. + os_activity_t _rootActivity; + +#if ASRunLoopQueueLoggingEnabled + NSTimer *_runloopQueueLoggingTimer; +#endif +} + +@end + +@implementation ASCATransactionQueue + +// CoreAnimation commit order is 2000000, the goal of this is to process shortly beforehand +// but after most other scheduled work on the runloop has processed. +static int const kASASCATransactionQueueOrder = 1000000; +// This will mark the end of current loop and any node enqueued between kASASCATransactionQueueOrder +// and kASASCATransactionQueuePostOrder will apply interface change immediately. +static int const kASASCATransactionQueuePostOrder = 3000000; + ++ (ASCATransactionQueue *)sharedQueue +{ + static dispatch_once_t onceToken; + static ASCATransactionQueue *sharedQueue; + dispatch_once(&onceToken, ^{ + sharedQueue = [[ASCATransactionQueue alloc] init]; + }); + return sharedQueue; +} + +- (instancetype)init +{ + if (self = [super init]) { + _runLoop = CFRunLoopGetMain(); + NSPointerFunctionsOptions options = NSPointerFunctionsStrongMemory; + _internalQueue = [[NSPointerArray alloc] initWithOptions:options]; + + // We don't want to pollute the top-level app activities with run loop batches, so we create one top-level + // activity per queue, and each batch activity joins that one instead. + _rootActivity = as_activity_create("Process run loop queue items", OS_ACTIVITY_NONE, OS_ACTIVITY_FLAG_DEFAULT); + { + // Log a message identifying this queue into the queue's root activity. + as_activity_scope_verbose(_rootActivity); + as_log_verbose(ASDisplayLog(), "Created run loop queue: %@", self); + } + + // Self is guaranteed to outlive the observer. Without the high cost of a weak pointer, + // __unsafe_unretained allows us to avoid flagging the memory cycle detector. + __unsafe_unretained __typeof__(self) weakSelf = self; + void (^handlerBlock) (CFRunLoopObserverRef observer, CFRunLoopActivity activity) = ^(CFRunLoopObserverRef observer, CFRunLoopActivity activity) { + [weakSelf processQueue]; + }; + void (^postHandlerBlock) (CFRunLoopObserverRef observer, CFRunLoopActivity activity) = ^(CFRunLoopObserverRef observer, CFRunLoopActivity activity) { + ASDN::MutexLocker l(_internalQueueLock); + _CATransactionCommitInProgress = NO; + }; + _preTransactionObserver = CFRunLoopObserverCreateWithHandler(NULL, kCFRunLoopBeforeWaiting, true, kASASCATransactionQueueOrder, handlerBlock); + _postTransactionObserver = CFRunLoopObserverCreateWithHandler(NULL, kCFRunLoopBeforeWaiting, true, kASASCATransactionQueuePostOrder, postHandlerBlock); + + CFRunLoopAddObserver(_runLoop, _preTransactionObserver, kCFRunLoopCommonModes); + CFRunLoopAddObserver(_runLoop, _postTransactionObserver, kCFRunLoopCommonModes); + + // It is not guaranteed that the runloop will turn if it has no scheduled work, and this causes processing of + // the queue to stop. Attaching a custom loop source to the run loop and signal it if new work needs to be done + CFRunLoopSourceContext sourceContext = {}; + sourceContext.perform = runLoopSourceCallback; +#if ASRunLoopQueueLoggingEnabled + sourceContext.info = (__bridge void *)self; +#endif + _runLoopSource = CFRunLoopSourceCreate(NULL, 0, &sourceContext); + CFRunLoopAddSource(_runLoop, _runLoopSource, kCFRunLoopCommonModes); + +#if ASRunLoopQueueLoggingEnabled + _runloopQueueLoggingTimer = [NSTimer timerWithTimeInterval:1.0 target:self selector:@selector(checkRunLoop) userInfo:nil repeats:YES]; + [[NSRunLoop mainRunLoop] addTimer:_runloopQueueLoggingTimer forMode:NSRunLoopCommonModes]; +#endif + } + return self; +} + +- (void)dealloc +{ + CFRunLoopRemoveSource(_runLoop, _runLoopSource, kCFRunLoopCommonModes); + CFRelease(_runLoopSource); + _runLoopSource = nil; + + if (CFRunLoopObserverIsValid(_preTransactionObserver)) { + CFRunLoopObserverInvalidate(_preTransactionObserver); + } + if (CFRunLoopObserverIsValid(_postTransactionObserver)) { + CFRunLoopObserverInvalidate(_postTransactionObserver); + } + CFRelease(_preTransactionObserver); + CFRelease(_postTransactionObserver); + _preTransactionObserver = nil; + _postTransactionObserver = nil; +} + +#if ASRunLoopQueueLoggingEnabled +- (void)checkRunLoop +{ + NSLog(@"<%@> - Jobs: %ld", self, _internalQueue.size()); +} +#endif + +- (void)processQueue +{ + // If we have an execution block, this vector will be populated, otherwise remains empty. + // This is to avoid needlessly retaining/releasing the objects if we don't have a block. + std::vector itemsToProcess; + + { + ASDN::MutexLocker l(_internalQueueLock); + + // Mark the queue will end coalescing shortly until after CATransactionCommit. + // This will give the queue a chance to apply any further interfaceState changes/enqueue + // immediately within current runloop instead of pushing the work to next runloop cycle. + _CATransactionCommitInProgress = YES; + + NSInteger internalQueueCount = _internalQueue.count; + // Early-exit if the queue is empty. + if (internalQueueCount == 0) { + return; + } + + ASSignpostStart(ASSignpostRunLoopQueueBatch); + + /** + * For each item in the next batch, if it's non-nil then NULL it out + * and if we have an execution block then add it in. + * This could be written a bunch of different ways but + * this particular one nicely balances readability, safety, and efficiency. + */ + NSInteger foundItemCount = 0; + for (NSInteger i = 0; i < internalQueueCount && foundItemCount < internalQueueCount; i++) { + /** + * It is safe to use unsafe_unretained here. If the queue is weak, the + * object will be added to the autorelease pool. If the queue is strong, + * it will retain the object until we transfer it (retain it) in itemsToProcess. + */ + __unsafe_unretained id ptr = (__bridge id)[_internalQueue pointerAtIndex:i]; + if (ptr != nil) { + foundItemCount++; + itemsToProcess.push_back(ptr); + [_internalQueue replacePointerAtIndex:i withPointer:NULL]; + } + } + + [_internalQueue compact]; + } + + // itemsToProcess will be empty if _queueConsumer == nil so no need to check again. + auto count = itemsToProcess.size(); + if (count > 0) { + as_activity_scope_verbose(as_activity_create("Process run loop queue batch", _rootActivity, OS_ACTIVITY_FLAG_DEFAULT)); + auto itemsEnd = itemsToProcess.cend(); + for (auto iterator = itemsToProcess.begin(); iterator < itemsEnd; iterator++) { + __unsafe_unretained id value = *iterator; + [value prepareForCATransactionCommit]; + as_log_verbose(ASDisplayLog(), "processed %@", value); + } + if (count > 1) { + as_log_verbose(ASDisplayLog(), "processed %lu items", (unsigned long)count); + } + } + + ASSignpostEnd(ASSignpostRunLoopQueueBatch); +} + +- (void)enqueue:(id)object +{ + if (!object) { + return; + } + + if (_disableInterfaceStateCoalesce || _CATransactionCommitInProgress) { + [object prepareForCATransactionCommit]; + return; + } + + ASDN::MutexLocker l(_internalQueueLock); + + // Check if the object exists. + BOOL foundObject = NO; + + for (id currentObject in _internalQueue) { + if (currentObject == object) { + foundObject = YES; + break; + } + } + + if (!foundObject) { + [_internalQueue addPointer:(__bridge void *)object]; + + CFRunLoopSourceSignal(_runLoopSource); + CFRunLoopWakeUp(_runLoop); + } +} + +- (BOOL)isEmpty +{ + ASDN::MutexLocker l(_internalQueueLock); + return _internalQueue.count == 0; +} + +- (void)disable +{ + _disableInterfaceStateCoalesce = YES; +} + +- (BOOL)disabled +{ + return _disableInterfaceStateCoalesce; +} + +@end diff --git a/Source/Private/ASDisplayNode+FrameworkPrivate.h b/Source/Private/ASDisplayNode+FrameworkPrivate.h index dbb520787..b309bdcc2 100644 --- a/Source/Private/ASDisplayNode+FrameworkPrivate.h +++ b/Source/Private/ASDisplayNode+FrameworkPrivate.h @@ -141,6 +141,10 @@ __unused static NSString * _Nonnull NSStringFromASHierarchyStateChange(ASHierarc // delegate to inform of ASInterfaceState changes (used by ASNodeController) @property (nonatomic, weak) id interfaceStateDelegate; +// The -pendingInterfaceState holds the value that will be applied to -interfaceState by the +// ASCATransactionQueue. If already applied, it matches -interfaceState. Thread-safe access. +@property (nonatomic, readonly) ASInterfaceState pendingInterfaceState; + // These methods are recursive, and either union or remove the provided interfaceState to all sub-elements. - (void)enterInterfaceState:(ASInterfaceState)interfaceState; - (void)exitInterfaceState:(ASInterfaceState)interfaceState; diff --git a/Source/Private/ASDisplayNodeInternal.h b/Source/Private/ASDisplayNodeInternal.h index 1be9e8c94..545f1c5a2 100644 --- a/Source/Private/ASDisplayNodeInternal.h +++ b/Source/Private/ASDisplayNodeInternal.h @@ -77,7 +77,7 @@ FOUNDATION_EXPORT NSString * const ASRenderingEngineDidDisplayNodesScheduledBefo { @package _ASPendingState *_pendingViewState; - + ASInterfaceState _pendingInterfaceState; UIView *_view; CALayer *_layer; diff --git a/Tests/ASCollectionViewTests.mm b/Tests/ASCollectionViewTests.mm index 311d7943f..c3dc8a2c4 100644 --- a/Tests/ASCollectionViewTests.mm +++ b/Tests/ASCollectionViewTests.mm @@ -23,6 +23,8 @@ #import #import #import +#import +#import "ASDisplayNodeTestsHelper.h" @interface ASTextCellNodeWithSetSelectedCounter : ASTextCellNode @@ -860,6 +862,7 @@ - (void)testThatDeletedItemsAreMarkedInvisible [cn waitUntilAllUpdatesAreProcessed]; [cn.view layoutIfNeeded]; ASCellNode *node = [cn nodeForItemAtIndexPath:[NSIndexPath indexPathForItem:0 inSection:0]]; + ASCATransactionQueueWait(); XCTAssertTrue(node.visible); testController.asyncDelegate->_itemCounts = {0}; [cn deleteItemsAtIndexPaths: @[[NSIndexPath indexPathForItem:0 inSection:0]]]; @@ -1047,7 +1050,7 @@ - (void)testInitialRangeBounds window.rootViewController = testController; [window makeKeyAndVisible]; - // Trigger the initial reload to start + // Trigger the initial reload to start [window layoutIfNeeded]; // Test the APIs that monitor ASCollectionNode update handling @@ -1071,7 +1074,7 @@ - (void)testInitialRangeBounds for (NSInteger i = 0; i < c; i++) { NSIndexPath *ip = [NSIndexPath indexPathForItem:i inSection:s]; ASCellNode *node = [cn nodeForItemAtIndexPath:ip]; - [[NSRunLoop mainRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:0.001]]; + ASCATransactionQueueWait(); if (node.inPreloadState) { CGRect frame = [cn.view layoutAttributesForItemAtIndexPath:ip].frame; r = CGRectUnion(r, frame); diff --git a/Tests/ASDisplayNodeImplicitHierarchyTests.m b/Tests/ASDisplayNodeImplicitHierarchyTests.m index 11e2ce4af..d033ac4f6 100644 --- a/Tests/ASDisplayNodeImplicitHierarchyTests.m +++ b/Tests/ASDisplayNodeImplicitHierarchyTests.m @@ -114,6 +114,7 @@ - (void)testInitialNodeInsertionWhenEnterPreloadState } ASSpecTestDisplayNode *node = [[ASSpecTestDisplayNode alloc] init]; + [node setHierarchyState:ASHierarchyStateRangeManaged]; node.automaticallyManagesSubnodes = YES; node.layoutSpecBlock = ^(ASDisplayNode *weakNode, ASSizeRange constrainedSize) { ASAbsoluteLayoutSpec *absoluteLayout = [ASAbsoluteLayoutSpec absoluteLayoutSpecWithChildren:@[subnodes[3]]]; @@ -130,6 +131,7 @@ - (void)testInitialNodeInsertionWhenEnterPreloadState ASDisplayNodeSizeToFitSizeRange(node, ASSizeRangeMake(CGSizeZero, CGSizeMake(CGFLOAT_MAX, CGFLOAT_MAX))); [node recursivelySetInterfaceState:ASInterfaceStatePreload]; + ASCATransactionQueueWait(); // No premature view allocation XCTAssertFalse(node.isNodeLoaded); // Subnodes should be inserted, laid out and entered preload state diff --git a/Tests/ASDisplayNodeTests.mm b/Tests/ASDisplayNodeTests.mm index ef5baee05..17bf960e0 100644 --- a/Tests/ASDisplayNodeTests.mm +++ b/Tests/ASDisplayNodeTests.mm @@ -91,7 +91,7 @@ @interface ASDisplayNode (HackForTests) - (id)initWithViewClass:(Class)viewClass; - (id)initWithLayerClass:(Class)layerClass; - +- (void)setInterfaceState:(ASInterfaceState)state; // FIXME: Importing ASDisplayNodeInternal.h causes a heap of problems. - (void)enterInterfaceState:(ASInterfaceState)interfaceState; @end @@ -122,6 +122,12 @@ @interface ASTestResponderNode : ASTestDisplayNode @implementation ASTestDisplayNode +- (void)setInterfaceState:(ASInterfaceState)state +{ + [super setInterfaceState:state]; + ASCATransactionQueueWait(); +} + - (CGSize)calculateSizeThatFits:(CGSize)constrainedSize { return _calculateSizeBlock ? _calculateSizeBlock(self, constrainedSize) : CGSizeZero; @@ -2058,9 +2064,9 @@ - (void)testThatNodeGetsRenderedIfItGoesFromZeroSizeToRealSizeButOnlyOnce // Underlying issue for: https://github.com/facebook/AsyncDisplayKit/issues/2205 - (void)testThatRasterizedNodesGetInterfaceStateUpdatesWhenContainerEntersHierarchy { - ASDisplayNode *supernode = [[ASDisplayNode alloc] init]; + ASDisplayNode *supernode = [[ASTestDisplayNode alloc] init]; [supernode enableSubtreeRasterization]; - ASDisplayNode *subnode = [[ASDisplayNode alloc] init]; + ASDisplayNode *subnode = [[ASTestDisplayNode alloc] init]; ASSetDebugNames(supernode, subnode); UIWindow *window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds]; [supernode addSubnode:subnode]; @@ -2076,9 +2082,9 @@ - (void)testThatRasterizedNodesGetInterfaceStateUpdatesWhenContainerEntersHierar // Underlying issue for: https://github.com/facebook/AsyncDisplayKit/issues/2205 - (void)testThatRasterizedNodesGetInterfaceStateUpdatesWhenAddedToContainerThatIsInHierarchy { - ASDisplayNode *supernode = [[ASDisplayNode alloc] init]; + ASDisplayNode *supernode = [[ASTestDisplayNode alloc] init]; [supernode enableSubtreeRasterization]; - ASDisplayNode *subnode = [[ASDisplayNode alloc] init]; + ASDisplayNode *subnode = [[ASTestDisplayNode alloc] init]; ASSetDebugNames(supernode, subnode); UIWindow *window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds]; @@ -2192,8 +2198,7 @@ - (void)testThatSubnodeGetsInterfaceStateSetIfRasterized [node view]; // Node needs to be loaded [node enterInterfaceState:ASInterfaceStatePreload]; - - + XCTAssertTrue((node.interfaceState & ASInterfaceStatePreload) == ASInterfaceStatePreload); XCTAssertTrue((subnode.interfaceState & ASInterfaceStatePreload) == ASInterfaceStatePreload); XCTAssertTrue(node.hasPreloaded); diff --git a/Tests/ASDisplayNodeTestsHelper.h b/Tests/ASDisplayNodeTestsHelper.h index 447350004..98b4719c6 100644 --- a/Tests/ASDisplayNodeTestsHelper.h +++ b/Tests/ASDisplayNodeTestsHelper.h @@ -28,5 +28,6 @@ BOOL ASDisplayNodeRunRunLoopUntilBlockIsTrue(as_condition_block_t block); void ASDisplayNodeSizeToFitSize(ASDisplayNode *node, CGSize size); void ASDisplayNodeSizeToFitSizeRange(ASDisplayNode *node, ASSizeRange sizeRange); +void ASCATransactionQueueWait(void); ASDISPLAYNODE_EXTERN_C_END diff --git a/Tests/ASDisplayNodeTestsHelper.m b/Tests/ASDisplayNodeTestsHelper.m index 05397b83b..0a37da1e4 100644 --- a/Tests/ASDisplayNodeTestsHelper.m +++ b/Tests/ASDisplayNodeTestsHelper.m @@ -18,6 +18,7 @@ #import "ASDisplayNodeTestsHelper.h" #import #import +#import #import @@ -62,3 +63,14 @@ void ASDisplayNodeSizeToFitSizeRange(ASDisplayNode *node, ASSizeRange sizeRange) CGSize sizeThatFits = [node layoutThatFits:sizeRange].size; node.bounds = (CGRect){.origin = CGPointZero, .size = sizeThatFits}; } + +void ASCATransactionQueueWait(void) +{ + NSDate *date = [NSDate dateWithTimeIntervalSinceNow:1]; + BOOL whileResult = YES; + while ([date timeIntervalSinceNow] > 0 && + (whileResult = ![[ASCATransactionQueue sharedQueue] isEmpty])) { + [[NSRunLoop currentRunLoop] runUntilDate: + [NSDate dateWithTimeIntervalSinceNow:0.01]]; + } +} diff --git a/Tests/ASRunLoopQueueTests.m b/Tests/ASRunLoopQueueTests.m index ccf15ae7b..2fc9c04b5 100644 --- a/Tests/ASRunLoopQueueTests.m +++ b/Tests/ASRunLoopQueueTests.m @@ -2,8 +2,8 @@ // ASRunLoopQueueTests.m // Texture // -// Copyright (c) 2017-present, Pinterest, Inc. All rights reserved. -// Licensed under the Apache License, Version 2.0 (the "License"); +// Copyright (c) 2017-present, +// Pinterest, Inc. Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at // @@ -12,9 +12,21 @@ #import #import +#import "ASDisplayNodeTestsHelper.h" static NSTimeInterval const kRunLoopRunTime = 0.001; // Allow the RunLoop to run for one millisecond each time. +@interface QueueObject : NSObject +@property (nonatomic, assign) BOOL queueObjectProcessed; +@end + +@implementation QueueObject +- (void)prepareForCATransactionCommit +{ + self.queueObjectProcessed = YES; +} +@end + @interface ASRunLoopQueueTests : XCTestCase @end @@ -157,4 +169,24 @@ - (void)testWeakQueueWithAllDeallocatedObjectsIsDrained XCTAssertTrue(queue.isEmpty); } +- (void)testASCATransactionQueueDisable +{ + ASCATransactionQueue *queue = [[ASCATransactionQueue alloc] init]; + [queue disable]; + QueueObject *object = [[QueueObject alloc] init]; + [[ASCATransactionQueue sharedQueue] enqueue:object]; + XCTAssertTrue([queue isEmpty]); + XCTAssertTrue([queue disabled]); +} + +- (void)testASCATransactionQueueProcess +{ + ASCATransactionQueue *queue = [[ASCATransactionQueue alloc] init]; + QueueObject *object = [[QueueObject alloc] init]; + [queue enqueue:object]; + XCTAssertFalse(object.queueObjectProcessed); + ASCATransactionQueueWait(); + XCTAssertTrue(object.queueObjectProcessed); +} + @end diff --git a/Tests/ASVideoNodeTests.m b/Tests/ASVideoNodeTests.m index 278509efe..96892b176 100644 --- a/Tests/ASVideoNodeTests.m +++ b/Tests/ASVideoNodeTests.m @@ -21,6 +21,7 @@ #import #import #import +#import "ASDisplayNodeTestsHelper.h" @interface ASVideoNodeTests : XCTestCase { @@ -351,9 +352,9 @@ - (void)testVideoResumedWhenBufferIsLikelyToKeepUp [_videoNode setInterfaceState:ASInterfaceStateVisible | ASInterfaceStateDisplay | ASInterfaceStatePreload]; [_videoNode prepareToPlayAsset:assetMock withKeys:_requestedKeys]; + ASCATransactionQueueWait(); [_videoNode pause]; _videoNode.shouldBePlaying = YES; - XCTAssertFalse(_videoNode.isPlaying); [_videoNode observeValueForKeyPath:@"playbackLikelyToKeepUp" ofObject:[_videoNode currentItem] change:@{NSKeyValueChangeNewKey : @YES} context:NULL];