From 29076d1a8311c63aec4e98bbd8d4e885f5142d92 Mon Sep 17 00:00:00 2001 From: Garrett Moon Date: Wed, 14 Mar 2018 12:20:49 -0700 Subject: [PATCH] Revert "New runloop queue to coalesce Interface state update calls. (#788)" This reverts commit 2618c50073092e99fe1df5213ba6a0619e908988. --- Source/ASDisplayNode.mm | 96 ++---- Source/ASRunLoopQueue.h | 35 +-- Source/ASRunLoopQueue.mm | 293 ++---------------- .../Private/ASDisplayNode+FrameworkPrivate.h | 4 - Source/Private/ASDisplayNodeInternal.h | 2 +- Tests/ASCollectionViewTests.mm | 7 +- Tests/ASDisplayNodeImplicitHierarchyTests.m | 2 - Tests/ASDisplayNodeTests.mm | 19 +- Tests/ASDisplayNodeTestsHelper.h | 1 - Tests/ASDisplayNodeTestsHelper.m | 12 - Tests/ASRunLoopQueueTests.m | 36 +-- Tests/ASVideoNodeTests.m | 3 +- 12 files changed, 66 insertions(+), 444 deletions(-) diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm index 1128383f..6bec3066 100644 --- a/Source/ASDisplayNode.mm +++ b/Source/ASDisplayNode.mm @@ -63,7 +63,7 @@ NSInteger const ASDefaultDrawingPriority = ASDefaultTransactionPriority; // 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 @@ -2836,7 +2836,7 @@ ASDISPLAYNODE_INLINE BOOL subtreeIsRasterized(ASDisplayNode *node) { // Entered or exited range managed state. if ((newState & ASHierarchyStateRangeManaged) != (oldState & ASHierarchyStateRangeManaged)) { if (newState & ASHierarchyStateRangeManaged) { - [self enterInterfaceState:self.supernode.pendingInterfaceState]; + [self enterInterfaceState:self.supernode.interfaceState]; } 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), @@ -2879,34 +2879,30 @@ ASDISPLAYNODE_INLINE BOOL subtreeIsRasterized(ASDisplayNode *node) { 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__); - - // 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; + + 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; } - self.interfaceState = newState; - } - }; - - if ([[ASCATransactionQueue sharedQueue] disabled]) { - dispatch_async(dispatch_get_main_queue(), exitVisibleInterfaceState); - } else { - exitVisibleInterfaceState(); + }); } } } @@ -2967,53 +2963,25 @@ ASDISPLAYNODE_INLINE BOOL subtreeIsRasterized(ASDisplayNode *node) { } - (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__); - // 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) { + if (_interfaceState == newState) { return; } + oldState = _interfaceState; _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)) { // } @@ -3110,12 +3078,6 @@ ASDISPLAYNODE_INLINE BOOL subtreeIsRasterized(ASDisplayNode *node) { [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 02711910..50a0eeb2 100644 --- a/Source/ASRunLoopQueue.h +++ b/Source/ASRunLoopQueue.h @@ -20,15 +20,8 @@ NS_ASSUME_NONNULL_BEGIN -@protocol ASCATransactionQueueObserving -- (void)prepareForCATransactionCommit; -@end - -@interface ASAbstractRunLoopQueue : NSObject -@end - AS_SUBCLASSING_RESTRICTED -@interface ASRunLoopQueue : ASAbstractRunLoopQueue +@interface ASRunLoopQueue : NSObject /** * Create a new queue with the given run loop and handler. @@ -48,37 +41,13 @@ AS_SUBCLASSING_RESTRICTED - (void)enqueue:(ObjectType)object; -@property (atomic, readonly) BOOL isEmpty; +@property (nonatomic, 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 12265d1d..974e3481 100644 --- a/Source/ASRunLoopQueue.mm +++ b/Source/ASRunLoopQueue.mm @@ -181,6 +181,27 @@ static void runLoopSourceCallback(void *info) { @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. @@ -197,23 +218,7 @@ typedef enum { @end #endif -#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; -} +@implementation ASRunLoopQueue #if AS_KDEBUG_ENABLE + (void)load @@ -260,31 +265,6 @@ typedef enum { #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]) { @@ -484,232 +464,3 @@ typedef enum { } @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 b309bdcc..dbb52078 100644 --- a/Source/Private/ASDisplayNode+FrameworkPrivate.h +++ b/Source/Private/ASDisplayNode+FrameworkPrivate.h @@ -141,10 +141,6 @@ __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 ef2d460a..4c6cd91d 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 c3dc8a2c..311d7943 100644 --- a/Tests/ASCollectionViewTests.mm +++ b/Tests/ASCollectionViewTests.mm @@ -23,8 +23,6 @@ #import #import #import -#import -#import "ASDisplayNodeTestsHelper.h" @interface ASTextCellNodeWithSetSelectedCounter : ASTextCellNode @@ -862,7 +860,6 @@ [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]]]; @@ -1050,7 +1047,7 @@ 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 @@ -1074,7 +1071,7 @@ for (NSInteger i = 0; i < c; i++) { NSIndexPath *ip = [NSIndexPath indexPathForItem:i inSection:s]; ASCellNode *node = [cn nodeForItemAtIndexPath:ip]; - ASCATransactionQueueWait(); + [[NSRunLoop mainRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:0.001]]; 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 d033ac4f..11e2ce4a 100644 --- a/Tests/ASDisplayNodeImplicitHierarchyTests.m +++ b/Tests/ASDisplayNodeImplicitHierarchyTests.m @@ -114,7 +114,6 @@ } ASSpecTestDisplayNode *node = [[ASSpecTestDisplayNode alloc] init]; - [node setHierarchyState:ASHierarchyStateRangeManaged]; node.automaticallyManagesSubnodes = YES; node.layoutSpecBlock = ^(ASDisplayNode *weakNode, ASSizeRange constrainedSize) { ASAbsoluteLayoutSpec *absoluteLayout = [ASAbsoluteLayoutSpec absoluteLayoutSpecWithChildren:@[subnodes[3]]]; @@ -131,7 +130,6 @@ 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 1176a709..fba3e4bb 100644 --- a/Tests/ASDisplayNodeTests.mm +++ b/Tests/ASDisplayNodeTests.mm @@ -97,7 +97,7 @@ for (ASDisplayNode *n in @[ nodes ]) {\ @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 @@ -128,12 +128,6 @@ for (ASDisplayNode *n in @[ nodes ]) {\ @implementation ASTestDisplayNode -- (void)setInterfaceState:(ASInterfaceState)state -{ - [super setInterfaceState:state]; - ASCATransactionQueueWait(); -} - - (CGSize)calculateSizeThatFits:(CGSize)constrainedSize { return _calculateSizeBlock ? _calculateSizeBlock(self, constrainedSize) : CGSizeZero; @@ -2143,9 +2137,9 @@ static bool stringContainsPointer(NSString *description, id p) { // Underlying issue for: https://github.com/facebook/AsyncDisplayKit/issues/2205 - (void)testThatRasterizedNodesGetInterfaceStateUpdatesWhenContainerEntersHierarchy { - ASDisplayNode *supernode = [[ASTestDisplayNode alloc] init]; + ASDisplayNode *supernode = [[ASDisplayNode alloc] init]; [supernode enableSubtreeRasterization]; - ASDisplayNode *subnode = [[ASTestDisplayNode alloc] init]; + ASDisplayNode *subnode = [[ASDisplayNode alloc] init]; ASSetDebugNames(supernode, subnode); UIWindow *window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds]; [supernode addSubnode:subnode]; @@ -2161,9 +2155,9 @@ static bool stringContainsPointer(NSString *description, id p) { // Underlying issue for: https://github.com/facebook/AsyncDisplayKit/issues/2205 - (void)testThatRasterizedNodesGetInterfaceStateUpdatesWhenAddedToContainerThatIsInHierarchy { - ASDisplayNode *supernode = [[ASTestDisplayNode alloc] init]; + ASDisplayNode *supernode = [[ASDisplayNode alloc] init]; [supernode enableSubtreeRasterization]; - ASDisplayNode *subnode = [[ASTestDisplayNode alloc] init]; + ASDisplayNode *subnode = [[ASDisplayNode alloc] init]; ASSetDebugNames(supernode, subnode); UIWindow *window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds]; @@ -2277,7 +2271,8 @@ static bool stringContainsPointer(NSString *description, id p) { [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 98b4719c..44735000 100644 --- a/Tests/ASDisplayNodeTestsHelper.h +++ b/Tests/ASDisplayNodeTestsHelper.h @@ -28,6 +28,5 @@ 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 0a37da1e..05397b83 100644 --- a/Tests/ASDisplayNodeTestsHelper.m +++ b/Tests/ASDisplayNodeTestsHelper.m @@ -18,7 +18,6 @@ #import "ASDisplayNodeTestsHelper.h" #import #import -#import #import @@ -63,14 +62,3 @@ 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 2fc9c04b..ccf15ae7 100644 --- a/Tests/ASRunLoopQueueTests.m +++ b/Tests/ASRunLoopQueueTests.m @@ -2,8 +2,8 @@ // ASRunLoopQueueTests.m // Texture // -// Copyright (c) 2017-present, -// Pinterest, Inc. Licensed under the Apache License, Version 2.0 (the "License"); +// Copyright (c) 2017-present, Pinterest, Inc. All rights reserved. +// 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,21 +12,9 @@ #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 @@ -169,24 +157,4 @@ static NSTimeInterval const kRunLoopRunTime = 0.001; // Allow the RunLoop to run 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 96892b17..278509ef 100644 --- a/Tests/ASVideoNodeTests.m +++ b/Tests/ASVideoNodeTests.m @@ -21,7 +21,6 @@ #import #import #import -#import "ASDisplayNodeTestsHelper.h" @interface ASVideoNodeTests : XCTestCase { @@ -352,9 +351,9 @@ [_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];