From 929d118c962dcf595386daf5c91ce30e1f8e577d Mon Sep 17 00:00:00 2001 From: Max Wang Date: Sun, 16 Jul 2017 16:59:28 -0700 Subject: [PATCH 1/9] fix SIMULATE_WEB_RESPONSE not imported #449 --- examples/ASCollectionView/Sample/ViewController.m | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/examples/ASCollectionView/Sample/ViewController.m b/examples/ASCollectionView/Sample/ViewController.m index d00ea60df..4e6a04e97 100644 --- a/examples/ASCollectionView/Sample/ViewController.m +++ b/examples/ASCollectionView/Sample/ViewController.m @@ -16,7 +16,7 @@ // #import "ViewController.h" - +#import "AppDelegate.h" #import #import "SupplementaryNode.h" #import "ItemNode.h" @@ -70,8 +70,8 @@ - (void)viewDidLoad { NSLog(@"ViewController is not nil"); strongSelf->_data = [[NSArray alloc] init]; - [strongSelf->_collectionView performBatchUpdates:^{ - [strongSelf->_collectionView insertSections:[[NSIndexSet alloc] initWithIndexesInRange:NSMakeRange(0, 100)]]; + [strongSelf->_collectionNode performBatchUpdates:^{ + [strongSelf->_collectionNode insertSections:[[NSIndexSet alloc] initWithIndexesInRange:NSMakeRange(0, 100)]]; } completion:nil]; NSLog(@"ViewController finished updating collectionView"); } @@ -81,7 +81,7 @@ - (void)viewDidLoad }; dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(2 * NSEC_PER_SEC)), dispatch_get_main_queue(), mockWebService); - dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(2 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{ + dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(4 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{ [self.navigationController popViewControllerAnimated:YES]; }); #endif From 329f35ff56aa6605ba9157174568cfd18a7a5373 Mon Sep 17 00:00:00 2001 From: Max Wang Date: Mon, 5 Feb 2018 14:10:21 -0800 Subject: [PATCH 2/9] Fix to make rangeMode update in right time --- Source/ASCollectionView.mm | 1 + Source/ASTableView.mm | 1 + Source/Details/ASRangeController.h | 5 +++++ Source/Details/ASRangeController.mm | 14 ++++++++++---- Tests/ASCollectionViewTests.mm | 1 + 5 files changed, 18 insertions(+), 4 deletions(-) diff --git a/Source/ASCollectionView.mm b/Source/ASCollectionView.mm index 0c706faa4..7c20dbddf 100644 --- a/Source/ASCollectionView.mm +++ b/Source/ASCollectionView.mm @@ -1511,6 +1511,7 @@ - (void)scrollViewDidScroll:(UIScrollView *)scrollView // If a scroll happenes the current range mode needs to go to full ASInterfaceState interfaceState = [self interfaceStateForRangeController:_rangeController]; if (ASInterfaceStateIncludesVisible(interfaceState)) { + _rangeController.contentOffsetHasChanged = YES; [_rangeController updateCurrentRangeWithMode:ASLayoutRangeModeFull]; [self _checkForBatchFetching]; } diff --git a/Source/ASTableView.mm b/Source/ASTableView.mm index 4606726f2..7bcd0fd5c 100644 --- a/Source/ASTableView.mm +++ b/Source/ASTableView.mm @@ -1218,6 +1218,7 @@ - (void)scrollViewDidScroll:(UIScrollView *)scrollView // If a scroll happenes the current range mode needs to go to full ASInterfaceState interfaceState = [self interfaceStateForRangeController:_rangeController]; if (ASInterfaceStateIncludesVisible(interfaceState)) { + _rangeController.contentOffsetHasChanged = YES; [_rangeController updateCurrentRangeWithMode:ASLayoutRangeModeFull]; [self _checkForBatchFetching]; } diff --git a/Source/Details/ASRangeController.h b/Source/Details/ASRangeController.h index 46a5cbf69..152c4e8b5 100644 --- a/Source/Details/ASRangeController.h +++ b/Source/Details/ASRangeController.h @@ -100,6 +100,11 @@ AS_SUBCLASSING_RESTRICTED */ @property (nonatomic, weak) id delegate; +/** + * Property that indicates whether the scroll view for this range controller has ever changed its contentOffset. + */ +@property (nonatomic, assign) BOOL contentOffsetHasChanged; + @end diff --git a/Source/Details/ASRangeController.mm b/Source/Details/ASRangeController.mm index 818fb3a1d..4d10838b7 100644 --- a/Source/Details/ASRangeController.mm +++ b/Source/Details/ASRangeController.mm @@ -45,6 +45,7 @@ @interface ASRangeController () NSSet *_allPreviousIndexPaths; NSHashTable *_visibleNodes; ASLayoutRangeMode _currentRangeMode; + BOOL _contentOffsetHasChanged; BOOL _preserveCurrentRangeMode; BOOL _didRegisterForNodeDisplayNotifications; CFTimeInterval _pendingDisplayNodesTimestamp; @@ -77,6 +78,7 @@ - (instancetype)init _rangeIsValid = YES; _currentRangeMode = ASLayoutRangeModeUnspecified; + _contentOffsetHasChanged = NO; _preserveCurrentRangeMode = NO; _previousScrollDirection = ASScrollDirectionDown | ASScrollDirectionRight; @@ -237,9 +239,13 @@ - (void)_updateVisibleNodeIndexPaths ASInterfaceState selfInterfaceState = [self interfaceState]; ASLayoutRangeMode rangeMode = _currentRangeMode; - // If the range mode is explicitly set via updateCurrentRangeWithMode: it will last in that mode until the - // range controller becomes visible again or explicitly changes the range mode again - if ((!_preserveCurrentRangeMode && ASInterfaceStateIncludesVisible(selfInterfaceState)) || [[self class] isFirstRangeUpdateForRangeMode:rangeMode]) { + BOOL updateRangeMode = !_preserveCurrentRangeMode && _contentOffsetHasChanged; + + // If we've never scrolled before, we never update the range mode, so it doesn't jump into Full too early. + // This can happen if we have multiple, noisy updates occurring from application code before the user has engaged. + // If the range mode is explicitly set via updateCurrentRangeWithMode:, we'll preserve that for at least one update cycle. + // Once the user has scrolled and the range is visible, we'll always resume managing the range mode automatically. + if ((updateRangeMode && ASInterfaceStateIncludesVisible(selfInterfaceState)) || [[self class] isFirstRangeUpdateForRangeMode:rangeMode]) { rangeMode = [ASRangeController rangeModeForInterfaceState:selfInterfaceState currentRangeMode:_currentRangeMode]; } @@ -412,7 +418,7 @@ - (void)_updateVisibleNodeIndexPaths // NSLog(@"custom: %@", visibleNodePathsSet); // } [modifiedIndexPaths sortUsingSelector:@selector(compare:)]; - NSLog(@"Range update complete; modifiedIndexPaths: %@", [self descriptionWithIndexPaths:modifiedIndexPaths]); + NSLog(@"Range update complete; modifiedIndexPaths: %@, rangeMode: %d", [self descriptionWithIndexPaths:modifiedIndexPaths], rangeMode); #endif ASSignpostEnd(ASSignpostRangeControllerUpdate); diff --git a/Tests/ASCollectionViewTests.mm b/Tests/ASCollectionViewTests.mm index b223e246f..11ab711d6 100644 --- a/Tests/ASCollectionViewTests.mm +++ b/Tests/ASCollectionViewTests.mm @@ -1071,6 +1071,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]]; if (node.inPreloadState) { CGRect frame = [cn.view layoutAttributesForItemAtIndexPath:ip].frame; r = CGRectUnion(r, frame); From 5fabc1e0a9ff09adb272a31c364a84b4fb1113fd Mon Sep 17 00:00:00 2001 From: Max Wang Date: Wed, 16 May 2018 22:20:34 -0700 Subject: [PATCH 3/9] remove uncessary assert --- Source/Details/ASDataController.mm | 8 -------- 1 file changed, 8 deletions(-) diff --git a/Source/Details/ASDataController.mm b/Source/Details/ASDataController.mm index f48c8dfc7..71282be8c 100644 --- a/Source/Details/ASDataController.mm +++ b/Source/Details/ASDataController.mm @@ -452,14 +452,6 @@ - (void)waitUntilAllUpdatesAreProcessed - (BOOL)isProcessingUpdates { ASDisplayNodeAssertMainThread(); -#if ASDISPLAYNODE_ASSERTIONS_ENABLED - // Using dispatch_group_wait is much more expensive than our manually managed count, but it's crucial they always match. - BOOL editingTransactionQueueBusy = dispatch_group_wait(_editingTransactionGroup, DISPATCH_TIME_NOW) != 0; - ASDisplayNodeAssert(editingTransactionQueueBusy == (_editingTransactionGroupCount > 0), - @"editingTransactionQueueBusy = %@, but _editingTransactionGroupCount = %d !", - editingTransactionQueueBusy ? @"YES" : @"NO", (int)_editingTransactionGroupCount); -#endif - return _mainSerialQueue.numberOfScheduledBlocks > 0 || _editingTransactionGroupCount > 0; } From 24af857c9bed9e74215b5e04c0bddafda2de1cf1 Mon Sep 17 00:00:00 2001 From: Max Wang Date: Wed, 29 Aug 2018 16:35:07 -0700 Subject: [PATCH 4/9] Allow to add interface state delegate in background threads. --- Source/ASDisplayNode.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm index 0fc41d40a..530d3a2f9 100644 --- a/Source/ASDisplayNode.mm +++ b/Source/ASDisplayNode.mm @@ -3241,7 +3241,7 @@ - (BOOL)shouldScheduleDisplayWithNewInterfaceState:(ASInterfaceState)newInterfac - (void)addInterfaceStateDelegate:(id )interfaceStateDelegate { - ASDisplayNodeAssertMainThread(); + ASDN::MutexLocker l(__instanceLock__); // Not a fan of lazy loading, but this method won't get called very often and avoiding // the overhead of creating this is probably worth it. From 49e13370f9fc26393a65b6690fe05d0b38ee9cd2 Mon Sep 17 00:00:00 2001 From: Max Wang Date: Thu, 30 Aug 2018 13:15:38 -0700 Subject: [PATCH 5/9] Allow to add interface state delegate in background threads. --- Source/ASDisplayNode.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm index 530d3a2f9..c3f6d27f2 100644 --- a/Source/ASDisplayNode.mm +++ b/Source/ASDisplayNode.mm @@ -3253,7 +3253,7 @@ - (void)addInterfaceStateDelegate:(id )interfaceStateD - (void)removeInterfaceStateDelegate:(id )interfaceStateDelegate { - ASDisplayNodeAssertMainThread(); + ASDN::MutexLocker l(__instanceLock__); [_interfaceStateDelegates removeObject:interfaceStateDelegate]; } From ee88e85e4699d22a54a84d2887a78bf08c0fe404 Mon Sep 17 00:00:00 2001 From: Max Wang Date: Thu, 30 Aug 2018 16:49:50 -0700 Subject: [PATCH 6/9] lock around _interfaceStateDelegates --- Source/ASDisplayNode.mm | 96 +++++++++++++++++++++++++---------------- 1 file changed, 58 insertions(+), 38 deletions(-) diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm index c3f6d27f2..59eb34a04 100644 --- a/Source/ASDisplayNode.mm +++ b/Source/ASDisplayNode.mm @@ -572,10 +572,11 @@ - (void)_didLoad for (ASDisplayNodeDidLoadBlock block in onDidLoadBlocks) { block(self); } - - for (id delegate in _interfaceStateDelegates) { - if ([delegate respondsToSelector:@selector(nodeDidLoad)]) { - [delegate nodeDidLoad]; + @synchronized (_interfaceStateDelegates) { + for (id delegate in _interfaceStateDelegates) { + if ([delegate respondsToSelector:@selector(nodeDidLoad)]) { + [delegate nodeDidLoad]; + } } } } @@ -1282,9 +1283,11 @@ - (void)layout ASDisplayNodeAssertMainThread(); ASAssertUnlocked(__instanceLock__); ASDisplayNodeAssertTrue(self.isNodeLoaded); - for (id delegate in _interfaceStateDelegates) { - if ([delegate respondsToSelector:@selector(nodeDidLayout)]) { - [delegate nodeDidLayout]; + @synchronized (_interfaceStateDelegates) { + for (id delegate in _interfaceStateDelegates) { + if ([delegate respondsToSelector:@selector(nodeDidLayout)]) { + [delegate nodeDidLayout]; + } } } } @@ -1509,10 +1512,11 @@ - (void)_pendingNodeDidDisplay:(ASDisplayNode *)node if (_pendingDisplayNodes.isEmpty) { [self hierarchyDisplayDidFinish]; - - for (id delegate in _interfaceStateDelegates) { - if ([delegate respondsToSelector:@selector(hierarchyDisplayDidFinish)]) { - [delegate hierarchyDisplayDidFinish]; + @synchronized (_interfaceStateDelegates) { + for (id delegate in _interfaceStateDelegates) { + if ([delegate respondsToSelector:@selector(hierarchyDisplayDidFinish)]) { + [delegate hierarchyDisplayDidFinish]; + } } } @@ -3017,6 +3021,8 @@ - (void)didExitHierarchy #pragma mark - Interface State + + /** * We currently only set interface state on nodes in table/collection views. For other nodes, if they are * in the hierarchy we enable all ASInterfaceState types with `ASInterfaceStateInHierarchy`, otherwise `None`. @@ -3225,9 +3231,11 @@ - (void)interfaceStateDidChange:(ASInterfaceState)newState fromState:(ASInterfac // Subclass hook ASAssertUnlocked(__instanceLock__); ASDisplayNodeAssertMainThread(); - for (id delegate in _interfaceStateDelegates) { - if ([delegate respondsToSelector:@selector(interfaceStateDidChange:fromState:)]) { - [delegate interfaceStateDidChange:newState fromState:oldState]; + @synchronized (_interfaceStateDelegates) { + for (id delegate in _interfaceStateDelegates) { + if ([delegate respondsToSelector:@selector(interfaceStateDidChange:fromState:)]) { + [delegate interfaceStateDidChange:newState fromState:oldState]; + } } } } @@ -3241,20 +3249,21 @@ - (BOOL)shouldScheduleDisplayWithNewInterfaceState:(ASInterfaceState)newInterfac - (void)addInterfaceStateDelegate:(id )interfaceStateDelegate { - ASDN::MutexLocker l(__instanceLock__); - // Not a fan of lazy loading, but this method won't get called very often and avoiding // the overhead of creating this is probably worth it. if (_interfaceStateDelegates == nil) { _interfaceStateDelegates = [NSHashTable weakObjectsHashTable]; } - [_interfaceStateDelegates addObject:interfaceStateDelegate]; + @synchronized (_interfaceStateDelegates) { + [_interfaceStateDelegates addObject:interfaceStateDelegate]; + } } - (void)removeInterfaceStateDelegate:(id )interfaceStateDelegate { - ASDN::MutexLocker l(__instanceLock__); - [_interfaceStateDelegates removeObject:interfaceStateDelegate]; + @synchronized (_interfaceStateDelegates) { + [_interfaceStateDelegates removeObject:interfaceStateDelegate]; + } } - (BOOL)isVisible @@ -3268,9 +3277,11 @@ - (void)didEnterVisibleState // subclass override ASDisplayNodeAssertMainThread(); ASAssertUnlocked(__instanceLock__); - for (id delegate in _interfaceStateDelegates) { - if ([delegate respondsToSelector:@selector(didEnterVisibleState)]) { - [delegate didEnterVisibleState]; + @synchronized (_interfaceStateDelegates) { + for (id delegate in _interfaceStateDelegates) { + if ([delegate respondsToSelector:@selector(didEnterVisibleState)]) { + [delegate didEnterVisibleState]; + } } } #if AS_ENABLE_TIPS @@ -3283,9 +3294,11 @@ - (void)didExitVisibleState // subclass override ASDisplayNodeAssertMainThread(); ASAssertUnlocked(__instanceLock__); - for (id delegate in _interfaceStateDelegates) { - if ([delegate respondsToSelector:@selector(didExitVisibleState)]) { - [delegate didExitVisibleState]; + @synchronized (_interfaceStateDelegates) { + for (id delegate in _interfaceStateDelegates) { + if ([delegate respondsToSelector:@selector(didExitVisibleState)]) { + [delegate didExitVisibleState]; + } } } } @@ -3301,9 +3314,11 @@ - (void)didEnterDisplayState // subclass override ASDisplayNodeAssertMainThread(); ASAssertUnlocked(__instanceLock__); - for (id delegate in _interfaceStateDelegates) { - if ([delegate respondsToSelector:@selector(didEnterDisplayState)]) { - [delegate didEnterDisplayState]; + @synchronized (_interfaceStateDelegates) { + for (id delegate in _interfaceStateDelegates) { + if ([delegate respondsToSelector:@selector(didEnterDisplayState)]) { + [delegate didEnterDisplayState]; + } } } } @@ -3313,9 +3328,11 @@ - (void)didExitDisplayState // subclass override ASDisplayNodeAssertMainThread(); ASAssertUnlocked(__instanceLock__); - for (id delegate in _interfaceStateDelegates) { - if ([delegate respondsToSelector:@selector(didExitDisplayState)]) { - [delegate didExitDisplayState]; + @synchronized (_interfaceStateDelegates) { + for (id delegate in _interfaceStateDelegates) { + if ([delegate respondsToSelector:@selector(didExitDisplayState)]) { + [delegate didExitDisplayState]; + } } } } @@ -3367,10 +3384,11 @@ - (void)didEnterPreloadState if (self.automaticallyManagesSubnodes) { [self layoutIfNeeded]; } - - for (id delegate in _interfaceStateDelegates) { - if ([delegate respondsToSelector:@selector(didEnterPreloadState)]) { - [delegate didEnterPreloadState]; + @synchronized (_interfaceStateDelegates) { + for (id delegate in _interfaceStateDelegates) { + if ([delegate respondsToSelector:@selector(didEnterPreloadState)]) { + [delegate didEnterPreloadState]; + } } } } @@ -3379,9 +3397,11 @@ - (void)didExitPreloadState { ASDisplayNodeAssertMainThread(); ASAssertUnlocked(__instanceLock__); - for (id delegate in _interfaceStateDelegates) { - if ([delegate respondsToSelector:@selector(didExitPreloadState)]) { - [delegate didExitPreloadState]; + @synchronized (_interfaceStateDelegates) { + for (id delegate in _interfaceStateDelegates) { + if ([delegate respondsToSelector:@selector(didExitPreloadState)]) { + [delegate didExitPreloadState]; + } } } } From 1b605f4354987de5b0add9b0f33a7a14af650243 Mon Sep 17 00:00:00 2001 From: Max Wang Date: Thu, 30 Aug 2018 17:56:33 -0700 Subject: [PATCH 7/9] lock _interfaceStateDelegates to local variable --- Source/ASDisplayNode.mm | 105 ++++++++++++---------------------------- 1 file changed, 31 insertions(+), 74 deletions(-) diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm index 59eb34a04..22de272f0 100644 --- a/Source/ASDisplayNode.mm +++ b/Source/ASDisplayNode.mm @@ -460,6 +460,16 @@ - (void)dealloc #pragma mark - Loading +#define ASDisplayNodeCallInterfaceStateDelegates(method, __lock) \ + __lock.lock(); \ + NSHashTable *delegates = _interfaceStateDelegates; \ + __lock.unlock(); \ + for (id delegate in delegates) { \ + if ([delegate respondsToSelector:@selector(method)]) { \ + [delegate method]; \ + } \ + } + - (BOOL)_locked_shouldLoadViewOrLayer { ASAssertLocked(__instanceLock__); @@ -572,13 +582,7 @@ - (void)_didLoad for (ASDisplayNodeDidLoadBlock block in onDidLoadBlocks) { block(self); } - @synchronized (_interfaceStateDelegates) { - for (id delegate in _interfaceStateDelegates) { - if ([delegate respondsToSelector:@selector(nodeDidLoad)]) { - [delegate nodeDidLoad]; - } - } - } + ASDisplayNodeCallInterfaceStateDelegates(nodeDidLoad, __instanceLock__); } - (void)didLoad @@ -1279,17 +1283,12 @@ - (ASLayoutSpec *)layoutSpecThatFits:(ASSizeRange)constrainedSize - (void)layout { + // Hook for subclasses ASDisplayNodeAssertMainThread(); ASAssertUnlocked(__instanceLock__); ASDisplayNodeAssertTrue(self.isNodeLoaded); - @synchronized (_interfaceStateDelegates) { - for (id delegate in _interfaceStateDelegates) { - if ([delegate respondsToSelector:@selector(nodeDidLayout)]) { - [delegate nodeDidLayout]; - } - } - } + ASDisplayNodeCallInterfaceStateDelegates(nodeDidLayout, __instanceLock__); } #pragma mark Layout Transition @@ -1512,13 +1511,7 @@ - (void)_pendingNodeDidDisplay:(ASDisplayNode *)node if (_pendingDisplayNodes.isEmpty) { [self hierarchyDisplayDidFinish]; - @synchronized (_interfaceStateDelegates) { - for (id delegate in _interfaceStateDelegates) { - if ([delegate respondsToSelector:@selector(hierarchyDisplayDidFinish)]) { - [delegate hierarchyDisplayDidFinish]; - } - } - } + ASDisplayNodeCallInterfaceStateDelegates(hierarchyDisplayDidFinish, __instanceLock__); BOOL placeholderShouldPersist = [self placeholderShouldPersist]; @@ -3231,11 +3224,12 @@ - (void)interfaceStateDidChange:(ASInterfaceState)newState fromState:(ASInterfac // Subclass hook ASAssertUnlocked(__instanceLock__); ASDisplayNodeAssertMainThread(); - @synchronized (_interfaceStateDelegates) { - for (id delegate in _interfaceStateDelegates) { - if ([delegate respondsToSelector:@selector(interfaceStateDidChange:fromState:)]) { - [delegate interfaceStateDidChange:newState fromState:oldState]; - } + __instanceLock__.lock(); + NSHashTable *delegates = _interfaceStateDelegates; + __instanceLock__.unlock(); + for (id delegate in delegates) { + if ([delegate respondsToSelector:@selector(interfaceStateDidChange:fromState:)]) { + [delegate interfaceStateDidChange:newState fromState:oldState]; } } } @@ -3249,21 +3243,19 @@ - (BOOL)shouldScheduleDisplayWithNewInterfaceState:(ASInterfaceState)newInterfac - (void)addInterfaceStateDelegate:(id )interfaceStateDelegate { + ASDN::MutexLocker l(__instanceLock__); // Not a fan of lazy loading, but this method won't get called very often and avoiding // the overhead of creating this is probably worth it. if (_interfaceStateDelegates == nil) { _interfaceStateDelegates = [NSHashTable weakObjectsHashTable]; } - @synchronized (_interfaceStateDelegates) { - [_interfaceStateDelegates addObject:interfaceStateDelegate]; - } + [_interfaceStateDelegates addObject:interfaceStateDelegate]; } - (void)removeInterfaceStateDelegate:(id )interfaceStateDelegate { - @synchronized (_interfaceStateDelegates) { - [_interfaceStateDelegates removeObject:interfaceStateDelegate]; - } + ASDN::MutexLocker l(__instanceLock__); + [_interfaceStateDelegates removeObject:interfaceStateDelegate]; } - (BOOL)isVisible @@ -3277,13 +3269,7 @@ - (void)didEnterVisibleState // subclass override ASDisplayNodeAssertMainThread(); ASAssertUnlocked(__instanceLock__); - @synchronized (_interfaceStateDelegates) { - for (id delegate in _interfaceStateDelegates) { - if ([delegate respondsToSelector:@selector(didEnterVisibleState)]) { - [delegate didEnterVisibleState]; - } - } - } + ASDisplayNodeCallInterfaceStateDelegates(didEnterVisibleState, __instanceLock__); #if AS_ENABLE_TIPS [ASTipsController.shared nodeDidAppear:self]; #endif @@ -3294,13 +3280,7 @@ - (void)didExitVisibleState // subclass override ASDisplayNodeAssertMainThread(); ASAssertUnlocked(__instanceLock__); - @synchronized (_interfaceStateDelegates) { - for (id delegate in _interfaceStateDelegates) { - if ([delegate respondsToSelector:@selector(didExitVisibleState)]) { - [delegate didExitVisibleState]; - } - } - } + ASDisplayNodeCallInterfaceStateDelegates(didExitVisibleState, __instanceLock__); } - (BOOL)isInDisplayState @@ -3314,13 +3294,7 @@ - (void)didEnterDisplayState // subclass override ASDisplayNodeAssertMainThread(); ASAssertUnlocked(__instanceLock__); - @synchronized (_interfaceStateDelegates) { - for (id delegate in _interfaceStateDelegates) { - if ([delegate respondsToSelector:@selector(didEnterDisplayState)]) { - [delegate didEnterDisplayState]; - } - } - } + ASDisplayNodeCallInterfaceStateDelegates(didEnterDisplayState, __instanceLock__); } - (void)didExitDisplayState @@ -3328,13 +3302,7 @@ - (void)didExitDisplayState // subclass override ASDisplayNodeAssertMainThread(); ASAssertUnlocked(__instanceLock__); - @synchronized (_interfaceStateDelegates) { - for (id delegate in _interfaceStateDelegates) { - if ([delegate respondsToSelector:@selector(didExitDisplayState)]) { - [delegate didExitDisplayState]; - } - } - } + ASDisplayNodeCallInterfaceStateDelegates(didExitDisplayState, __instanceLock__); } - (BOOL)isInPreloadState @@ -3384,26 +3352,15 @@ - (void)didEnterPreloadState if (self.automaticallyManagesSubnodes) { [self layoutIfNeeded]; } - @synchronized (_interfaceStateDelegates) { - for (id delegate in _interfaceStateDelegates) { - if ([delegate respondsToSelector:@selector(didEnterPreloadState)]) { - [delegate didEnterPreloadState]; - } - } - } + ASDisplayNodeCallInterfaceStateDelegates(didEnterPreloadState, __instanceLock__); } - (void)didExitPreloadState { ASDisplayNodeAssertMainThread(); ASAssertUnlocked(__instanceLock__); - @synchronized (_interfaceStateDelegates) { - for (id delegate in _interfaceStateDelegates) { - if ([delegate respondsToSelector:@selector(didExitPreloadState)]) { - [delegate didExitPreloadState]; - } - } - } + ASDisplayNodeCallInterfaceStateDelegates(didExitPreloadState, __instanceLock__); + } - (void)clearContents From dfbd03b090e4cff8842ae778600150e6c81708a2 Mon Sep 17 00:00:00 2001 From: Max Wang Date: Fri, 31 Aug 2018 10:12:37 -0700 Subject: [PATCH 8/9] Fix comments --- CHANGELOG.md | 1 + Source/ASDisplayNode.mm | 28 ++++++++++++++-------------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01ecb97ca..b1ef98aa2 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. +- [ASDisplayNode] Allow add/remove interface state delegates on background thread. [Max Wang](https://github.com/wsdwsd0829). [#1090](https://github.com/TextureGroup/Texture/pull/1090) - [ASNetworkImageNode] Allow delegate methods to be called on background thread. [Max Wang](https://github.com/wsdwsd0829). [#1007](https://github.com/TextureGroup/Texture/pull/1007) - [ASLayoutTransition] Add support for preserving order after node moves during transitions. (This order defines the z-order as well.) [Kevin Smith](https://github.com/wiseoldduck) [#1006] - [ASDisplayNode] Adds support for multiple interface state delegates. [Garrett Moon](https://github.com/garrettmoon) [#979](https://github.com/TextureGroup/Texture/pull/979) diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm index 22de272f0..b434f444f 100644 --- a/Source/ASDisplayNode.mm +++ b/Source/ASDisplayNode.mm @@ -460,10 +460,10 @@ - (void)dealloc #pragma mark - Loading -#define ASDisplayNodeCallInterfaceStateDelegates(method, __lock) \ - __lock.lock(); \ - NSHashTable *delegates = _interfaceStateDelegates; \ - __lock.unlock(); \ +#define ASDisplayNodeCallInterfaceStateDelegates(method) \ + __instanceLock__.lock(); \ + NSHashTable *delegates = [_interfaceStateDelegates copy]; \ + __instanceLock__.unlock(); \ for (id delegate in delegates) { \ if ([delegate respondsToSelector:@selector(method)]) { \ [delegate method]; \ @@ -582,7 +582,7 @@ - (void)_didLoad for (ASDisplayNodeDidLoadBlock block in onDidLoadBlocks) { block(self); } - ASDisplayNodeCallInterfaceStateDelegates(nodeDidLoad, __instanceLock__); + ASDisplayNodeCallInterfaceStateDelegates(nodeDidLoad); } - (void)didLoad @@ -1288,7 +1288,7 @@ - (void)layout ASDisplayNodeAssertMainThread(); ASAssertUnlocked(__instanceLock__); ASDisplayNodeAssertTrue(self.isNodeLoaded); - ASDisplayNodeCallInterfaceStateDelegates(nodeDidLayout, __instanceLock__); + ASDisplayNodeCallInterfaceStateDelegates(nodeDidLayout); } #pragma mark Layout Transition @@ -1511,7 +1511,7 @@ - (void)_pendingNodeDidDisplay:(ASDisplayNode *)node if (_pendingDisplayNodes.isEmpty) { [self hierarchyDisplayDidFinish]; - ASDisplayNodeCallInterfaceStateDelegates(hierarchyDisplayDidFinish, __instanceLock__); + ASDisplayNodeCallInterfaceStateDelegates(hierarchyDisplayDidFinish); BOOL placeholderShouldPersist = [self placeholderShouldPersist]; @@ -3225,7 +3225,7 @@ - (void)interfaceStateDidChange:(ASInterfaceState)newState fromState:(ASInterfac ASAssertUnlocked(__instanceLock__); ASDisplayNodeAssertMainThread(); __instanceLock__.lock(); - NSHashTable *delegates = _interfaceStateDelegates; + NSHashTable *delegates = [_interfaceStateDelegates copy]; __instanceLock__.unlock(); for (id delegate in delegates) { if ([delegate respondsToSelector:@selector(interfaceStateDidChange:fromState:)]) { @@ -3269,7 +3269,7 @@ - (void)didEnterVisibleState // subclass override ASDisplayNodeAssertMainThread(); ASAssertUnlocked(__instanceLock__); - ASDisplayNodeCallInterfaceStateDelegates(didEnterVisibleState, __instanceLock__); + ASDisplayNodeCallInterfaceStateDelegates(didEnterVisibleState); #if AS_ENABLE_TIPS [ASTipsController.shared nodeDidAppear:self]; #endif @@ -3280,7 +3280,7 @@ - (void)didExitVisibleState // subclass override ASDisplayNodeAssertMainThread(); ASAssertUnlocked(__instanceLock__); - ASDisplayNodeCallInterfaceStateDelegates(didExitVisibleState, __instanceLock__); + ASDisplayNodeCallInterfaceStateDelegates(didExitVisibleState); } - (BOOL)isInDisplayState @@ -3294,7 +3294,7 @@ - (void)didEnterDisplayState // subclass override ASDisplayNodeAssertMainThread(); ASAssertUnlocked(__instanceLock__); - ASDisplayNodeCallInterfaceStateDelegates(didEnterDisplayState, __instanceLock__); + ASDisplayNodeCallInterfaceStateDelegates(didEnterDisplayState); } - (void)didExitDisplayState @@ -3302,7 +3302,7 @@ - (void)didExitDisplayState // subclass override ASDisplayNodeAssertMainThread(); ASAssertUnlocked(__instanceLock__); - ASDisplayNodeCallInterfaceStateDelegates(didExitDisplayState, __instanceLock__); + ASDisplayNodeCallInterfaceStateDelegates(didExitDisplayState); } - (BOOL)isInPreloadState @@ -3352,14 +3352,14 @@ - (void)didEnterPreloadState if (self.automaticallyManagesSubnodes) { [self layoutIfNeeded]; } - ASDisplayNodeCallInterfaceStateDelegates(didEnterPreloadState, __instanceLock__); + ASDisplayNodeCallInterfaceStateDelegates(didEnterPreloadState); } - (void)didExitPreloadState { ASDisplayNodeAssertMainThread(); ASAssertUnlocked(__instanceLock__); - ASDisplayNodeCallInterfaceStateDelegates(didExitPreloadState, __instanceLock__); + ASDisplayNodeCallInterfaceStateDelegates(didExitPreloadState); } From ec3542a65421db1263e728de887a1e021834bc1c Mon Sep 17 00:00:00 2001 From: Max Wang Date: Fri, 31 Aug 2018 10:15:17 -0700 Subject: [PATCH 9/9] remove extra spaces --- Source/ASDisplayNode.mm | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm index b434f444f..2f60a4a7d 100644 --- a/Source/ASDisplayNode.mm +++ b/Source/ASDisplayNode.mm @@ -1283,7 +1283,6 @@ - (ASLayoutSpec *)layoutSpecThatFits:(ASSizeRange)constrainedSize - (void)layout { - // Hook for subclasses ASDisplayNodeAssertMainThread(); ASAssertUnlocked(__instanceLock__); @@ -3014,8 +3013,6 @@ - (void)didExitHierarchy #pragma mark - Interface State - - /** * We currently only set interface state on nodes in table/collection views. For other nodes, if they are * in the hierarchy we enable all ASInterfaceState types with `ASInterfaceStateInHierarchy`, otherwise `None`. @@ -3360,7 +3357,6 @@ - (void)didExitPreloadState ASDisplayNodeAssertMainThread(); ASAssertUnlocked(__instanceLock__); ASDisplayNodeCallInterfaceStateDelegates(didExitPreloadState); - } - (void)clearContents