From 11461f415f9600efa6e2bded457b0a3aa8f354e1 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Sat, 9 Apr 2016 20:27:37 -0700 Subject: [PATCH 01/11] [ASNetworkImageNode] Improve readability, resolve self retain, reduce locking --- AsyncDisplayKit/ASNetworkImageNode.mm | 51 ++++++++++++--------------- 1 file changed, 23 insertions(+), 28 deletions(-) diff --git a/AsyncDisplayKit/ASNetworkImageNode.mm b/AsyncDisplayKit/ASNetworkImageNode.mm index f909de9f..a3375fbc 100755 --- a/AsyncDisplayKit/ASNetworkImageNode.mm +++ b/AsyncDisplayKit/ASNetworkImageNode.mm @@ -382,32 +382,27 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; return; } - { - ASDN::MutexLocker l(strongSelf->_lock); - - //Getting a result back for a different download identifier, download must not have been successfully canceled - if (ASObjectIsEqual(strongSelf->_downloadIdentifier, downloadIdentifier) == NO && downloadIdentifier != nil) { - return; - } - - if (responseImage != NULL) { - strongSelf->_imageLoaded = YES; - strongSelf.image = responseImage; - } - - strongSelf->_downloadIdentifier = nil; - - strongSelf->_cacheUUID = nil; + ASDN::MutexLocker l(strongSelf->_lock); + + //Getting a result back for a different download identifier, download must not have been successfully canceled + if (downloadIdentifier != nil && !ASObjectIsEqual(strongSelf->_downloadIdentifier, downloadIdentifier)) { + return; } - { - ASDN::MutexLocker l(strongSelf->_lock); - if (responseImage != NULL) { - [strongSelf->_delegate imageNode:strongSelf didLoadImage:strongSelf.image]; - } - else if (error && _delegateSupportsDidFailWithError) { - [strongSelf->_delegate imageNode:strongSelf didFailWithError:error]; - } + if (responseImage != nil) { + strongSelf->_imageLoaded = YES; + strongSelf.image = responseImage; + } + + strongSelf->_downloadIdentifier = nil; + + strongSelf->_cacheUUID = nil; + + if (responseImage != nil) { + [strongSelf->_delegate imageNode:strongSelf didLoadImage:strongSelf.image]; + } + else if (error && strongSelf->_delegateSupportsDidFailWithError) { + [strongSelf->_delegate imageNode:strongSelf didFailWithError:error]; } }; @@ -417,14 +412,14 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; void (^cacheCompletion)(UIImage *) = ^(UIImage *image) { // If the cache UUID changed, that means this request was cancelled. - if (![_cacheUUID isEqual:cacheUUID]) { + if (!ASObjectIsEqual(_cacheUUID, cacheUUID)) { return; } - - if (image == NULL && _downloader != nil) { + + if (image == nil && _downloader != nil) { [self _downloadImageWithCompletion:finished]; } else { - finished(image, NULL, nil); + finished(image, nil, nil); } }; From 37c7f6f8495eb49ecf8f844fcba3990c485cb8c2 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Sat, 9 Apr 2016 20:47:19 -0700 Subject: [PATCH 02/11] [ASNetworkImageNode] Remove retain --- AsyncDisplayKit/ASNetworkImageNode.mm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/AsyncDisplayKit/ASNetworkImageNode.mm b/AsyncDisplayKit/ASNetworkImageNode.mm index a3375fbc..3a0c3415 100755 --- a/AsyncDisplayKit/ASNetworkImageNode.mm +++ b/AsyncDisplayKit/ASNetworkImageNode.mm @@ -236,9 +236,9 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; return; } - ASDN::MutexLocker l(_lock); + ASDN::MutexLocker l(strongSelf->_lock); //Getting a result back for a different download identifier, download must not have been successfully canceled - if (ASObjectIsEqual(strongSelf->_downloadIdentifier, downloadIdentifier) == NO && downloadIdentifier != nil) { + if (!ASObjectIsEqual(strongSelf->_downloadIdentifier, downloadIdentifier) && downloadIdentifier != nil) { return; } From 362c41ae80bbf08df4056065b035ec6cf45b4adc Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Mon, 11 Apr 2016 12:51:16 -0700 Subject: [PATCH 03/11] [ASNetworkImageNode] Attach progress image handler even if download starts while already visible --- AsyncDisplayKit/ASNetworkImageNode.mm | 59 ++++++++++++++------------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/AsyncDisplayKit/ASNetworkImageNode.mm b/AsyncDisplayKit/ASNetworkImageNode.mm index 3a0c3415..258575f0 100755 --- a/AsyncDisplayKit/ASNetworkImageNode.mm +++ b/AsyncDisplayKit/ASNetworkImageNode.mm @@ -14,6 +14,7 @@ #import "ASEqualityHelpers.h" #import "ASThread.h" #import "ASInternalHelpers.h" +#import "ASDisplayNodeExtras.h" #if PIN_REMOTE_IMAGE #import "ASPINRemoteImageDownloader.h" @@ -211,9 +212,8 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; - (void)visibilityDidChange:(BOOL)isVisible { [super visibilityDidChange:isVisible]; - + ASDN::MutexLocker l(_lock); if (_downloaderImplementsSetPriority) { - ASDN::MutexLocker l(_lock); if (_downloadIdentifier != nil) { if (isVisible) { [_downloader setPriority:ASImageDownloaderPriorityVisible withDownloadIdentifier:_downloadIdentifier]; @@ -222,32 +222,8 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; } } } - - if (_downloaderImplementsSetProgress) { - ASDN::MutexLocker l(_lock); - - if (_downloadIdentifier != nil) { - __weak __typeof__(self) weakSelf = self; - ASImageDownloaderProgressImage progress = nil; - if (isVisible) { - progress = ^(UIImage * _Nonnull progressImage, id _Nullable downloadIdentifier) { - __typeof__(self) strongSelf = weakSelf; - if (strongSelf == nil) { - return; - } - - ASDN::MutexLocker l(strongSelf->_lock); - //Getting a result back for a different download identifier, download must not have been successfully canceled - if (!ASObjectIsEqual(strongSelf->_downloadIdentifier, downloadIdentifier) && downloadIdentifier != nil) { - return; - } - - strongSelf.image = progressImage; - }; - } - [_downloader setProgressImageBlock:progress callbackQueue:dispatch_get_main_queue() withDownloadIdentifier:_downloadIdentifier]; - } - } + + [self _updateProgressImageBlockOnDownloaderIfNeeded]; } - (void)clearFetchedData @@ -277,6 +253,32 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; #pragma mark - Private methods -- only call with lock. +- (void)_updateProgressImageBlockOnDownloaderIfNeeded +{ + if (!_downloaderImplementsSetProgress || _downloadIdentifier == nil) { + return; + } + + ASImageDownloaderProgressImage progress = nil; + if (ASInterfaceStateIncludesVisible(self.interfaceState)) { + __weak __typeof__(self) weakSelf = self; + progress = ^(UIImage * _Nonnull progressImage, id _Nullable downloadIdentifier) { + __typeof__(self) strongSelf = weakSelf; + if (strongSelf == nil) { + return; + } + + ASDN::MutexLocker l(strongSelf->_lock); + //Getting a result back for a different download identifier, download must not have been successfully canceled + if (ASObjectIsEqual(strongSelf->_downloadIdentifier, downloadIdentifier) == NO && downloadIdentifier != nil) { + return; + } + strongSelf.image = progressImage; + }; + } + [_downloader setProgressImageBlock:progress callbackQueue:dispatch_get_main_queue() withDownloadIdentifier:_downloadIdentifier]; +} + - (void)_clearImage { // Destruction of bigger images on the main thread can be expensive @@ -335,6 +337,7 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; }]; #pragma clang diagnostic pop } + [self _updateProgressImageBlockOnDownloaderIfNeeded]; }); } From 0022cad000f37a3e30ab0433c36bf3222a1f6b4b Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Tue, 12 Apr 2016 14:34:11 -0700 Subject: [PATCH 04/11] [ASMultiplexImageNode] Copy recent changes made to ASNetworkImageNode for progress image handling --- AsyncDisplayKit/ASMultiplexImageNode.mm | 59 +++++++++++++------------ AsyncDisplayKit/ASNetworkImageNode.mm | 3 +- 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/AsyncDisplayKit/ASMultiplexImageNode.mm b/AsyncDisplayKit/ASMultiplexImageNode.mm index 351ff9c7..8215ece2 100644 --- a/AsyncDisplayKit/ASMultiplexImageNode.mm +++ b/AsyncDisplayKit/ASMultiplexImageNode.mm @@ -22,6 +22,7 @@ #import "ASPhotosFrameworkImageRequest.h" #import "ASEqualityHelpers.h" #import "ASInternalHelpers.h" +#import "ASDisplayNodeExtras.h" #if !AS_IOS8_SDK_OR_LATER #error ASMultiplexImageNode can be used on iOS 7, but must be linked against the iOS 8 SDK. @@ -293,9 +294,8 @@ typedef void(^ASMultiplexImageLoadCompletionBlock)(UIImage *image, id imageIdent - (void)visibilityDidChange:(BOOL)isVisible { [super visibilityDidChange:isVisible]; - + ASDN::MutexLocker l(_downloadIdentifierLock); if (_downloaderImplementsSetPriority) { - ASDN::MutexLocker l(_downloadIdentifierLock); if (_downloadIdentifier != nil) { if (isVisible) { [_downloader setPriority:ASImageDownloaderPriorityVisible withDownloadIdentifier:_downloadIdentifier]; @@ -305,32 +305,7 @@ typedef void(^ASMultiplexImageLoadCompletionBlock)(UIImage *image, id imageIdent } } - if (_downloaderImplementsSetProgress) { - ASDN::MutexLocker l(_downloadIdentifierLock); - - if (_downloadIdentifier != nil) { - __weak __typeof__(self) weakSelf = self; - ASImageDownloaderProgressImage progress = nil; - if (isVisible) { - progress = ^(UIImage * _Nonnull progressImage, id _Nullable downloadIdentifier) { - __typeof__(self) strongSelf = weakSelf; - if (strongSelf == nil) { - return; - } - - ASDN::MutexLocker l(strongSelf->_downloadIdentifierLock); - //Getting a result back for a different download identifier, download must not have been successfully canceled - if (ASObjectIsEqual(strongSelf->_downloadIdentifier, downloadIdentifier) == NO && downloadIdentifier != nil) { - return; - } - - strongSelf.image = progressImage; - }; - } - - [_downloader setProgressImageBlock:progress callbackQueue:dispatch_get_main_queue() withDownloadIdentifier:_downloadIdentifier]; - } - } + [self _updateProgressImageBlockOnDownloaderIfNeeded]; } #pragma mark - Core @@ -478,6 +453,33 @@ typedef void(^ASMultiplexImageLoadCompletionBlock)(UIImage *image, id imageIdent } #pragma mark - + +- (void)_updateProgressImageBlockOnDownloaderIfNeeded +{ + if (!_downloaderImplementsSetProgress || _downloadIdentifier == nil) { + return; + } + + ASImageDownloaderProgressImage progress = nil; + if (ASInterfaceStateIncludesVisible(self.interfaceState)) { + __weak __typeof__(self) weakSelf = self; + progress = ^(UIImage * _Nonnull progressImage, id _Nullable downloadIdentifier) { + __typeof__(self) strongSelf = weakSelf; + if (strongSelf == nil) { + return; + } + + ASDN::MutexLocker l(strongSelf->_downloadIdentifierLock); + //Getting a result back for a different download identifier, download must not have been successfully canceled + if (ASObjectIsEqual(strongSelf->_downloadIdentifier, downloadIdentifier) == NO && downloadIdentifier != nil) { + return; + } + strongSelf.image = progressImage; + }; + } + [_downloader setProgressImageBlock:progress callbackQueue:dispatch_get_main_queue() withDownloadIdentifier:_downloadIdentifier]; +} + - (void)_clearImage { // Destruction of bigger images on the main thread can be expensive @@ -823,6 +825,7 @@ typedef void(^ASMultiplexImageLoadCompletionBlock)(UIImage *image, id imageIdent }]]; #pragma clang diagnostic pop } + [self _updateProgressImageBlockOnDownloaderIfNeeded]; }); } diff --git a/AsyncDisplayKit/ASNetworkImageNode.mm b/AsyncDisplayKit/ASNetworkImageNode.mm index 258575f0..fb3df668 100755 --- a/AsyncDisplayKit/ASNetworkImageNode.mm +++ b/AsyncDisplayKit/ASNetworkImageNode.mm @@ -212,6 +212,7 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; - (void)visibilityDidChange:(BOOL)isVisible { [super visibilityDidChange:isVisible]; + ASDN::MutexLocker l(_lock); if (_downloaderImplementsSetPriority) { if (_downloadIdentifier != nil) { @@ -388,7 +389,7 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; ASDN::MutexLocker l(strongSelf->_lock); //Getting a result back for a different download identifier, download must not have been successfully canceled - if (downloadIdentifier != nil && !ASObjectIsEqual(strongSelf->_downloadIdentifier, downloadIdentifier)) { + if (ASObjectIsEqual(strongSelf->_downloadIdentifier, downloadIdentifier) == NO && downloadIdentifier != nil) { return; } From 7128f696786b83bc319569e976c42cce7dd5eb82 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Tue, 12 Apr 2016 17:35:46 -0700 Subject: [PATCH 05/11] [ASMultiplexImageNode] Avoid deadlock by reading our interface state earlier --- AsyncDisplayKit/ASMultiplexImageNode.mm | 6 +++++- AsyncDisplayKit/Details/ASPINRemoteImageDownloader.m | 6 +++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/AsyncDisplayKit/ASMultiplexImageNode.mm b/AsyncDisplayKit/ASMultiplexImageNode.mm index 8215ece2..5c405906 100644 --- a/AsyncDisplayKit/ASMultiplexImageNode.mm +++ b/AsyncDisplayKit/ASMultiplexImageNode.mm @@ -456,12 +456,16 @@ typedef void(^ASMultiplexImageLoadCompletionBlock)(UIImage *image, id imageIdent - (void)_updateProgressImageBlockOnDownloaderIfNeeded { + // Read our interface state so that we don't lock super while holding our lock. + ASInterfaceState interfaceState = self.interfaceState; + ASDN::MutexLocker l(_downloadIdentifierLock); + if (!_downloaderImplementsSetProgress || _downloadIdentifier == nil) { return; } ASImageDownloaderProgressImage progress = nil; - if (ASInterfaceStateIncludesVisible(self.interfaceState)) { + if (ASInterfaceStateIncludesVisible(interfaceState)) { __weak __typeof__(self) weakSelf = self; progress = ^(UIImage * _Nonnull progressImage, id _Nullable downloadIdentifier) { __typeof__(self) strongSelf = weakSelf; diff --git a/AsyncDisplayKit/Details/ASPINRemoteImageDownloader.m b/AsyncDisplayKit/Details/ASPINRemoteImageDownloader.m index 13f304f7..778f7cf1 100644 --- a/AsyncDisplayKit/Details/ASPINRemoteImageDownloader.m +++ b/AsyncDisplayKit/Details/ASPINRemoteImageDownloader.m @@ -36,9 +36,9 @@ return result.image; } -- (void)fetchCachedImageWithURL:(NSURL *)URL - callbackQueue:(dispatch_queue_t)callbackQueue - completion:(void (^)(CGImageRef imageFromCache))completion +- (void)cachedImageWithURL:(NSURL *)URL + callbackQueue:(dispatch_queue_t)callbackQueue + completion:(ASImageCacherCompletion)completion { // We do not check the cache here and instead check it in downloadImageWithURL to avoid checking the cache twice. // If we're targeting the main queue and we're on the main thread, complete immediately. From faf29dd97855f01a856789251fd764b887fffe58 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Tue, 12 Apr 2016 17:45:40 -0700 Subject: [PATCH 06/11] Fix indentation --- AsyncDisplayKit/ASMultiplexImageNode.mm | 46 ++++++++++++------------- AsyncDisplayKit/ASNetworkImageNode.mm | 2 +- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/AsyncDisplayKit/ASMultiplexImageNode.mm b/AsyncDisplayKit/ASMultiplexImageNode.mm index 5c405906..d7ce8e31 100644 --- a/AsyncDisplayKit/ASMultiplexImageNode.mm +++ b/AsyncDisplayKit/ASMultiplexImageNode.mm @@ -456,32 +456,32 @@ typedef void(^ASMultiplexImageLoadCompletionBlock)(UIImage *image, id imageIdent - (void)_updateProgressImageBlockOnDownloaderIfNeeded { - // Read our interface state so that we don't lock super while holding our lock. - ASInterfaceState interfaceState = self.interfaceState; + // Read our interface state so that we don't lock super while holding our lock. + ASInterfaceState interfaceState = self.interfaceState; ASDN::MutexLocker l(_downloadIdentifierLock); - - if (!_downloaderImplementsSetProgress || _downloadIdentifier == nil) { - return; - } + + if (!_downloaderImplementsSetProgress || _downloadIdentifier == nil) { + return; + } - ASImageDownloaderProgressImage progress = nil; - if (ASInterfaceStateIncludesVisible(interfaceState)) { - __weak __typeof__(self) weakSelf = self; - progress = ^(UIImage * _Nonnull progressImage, id _Nullable downloadIdentifier) { - __typeof__(self) strongSelf = weakSelf; - if (strongSelf == nil) { - return; - } + ASImageDownloaderProgressImage progress = nil; + if (ASInterfaceStateIncludesVisible(interfaceState)) { + __weak __typeof__(self) weakSelf = self; + progress = ^(UIImage * _Nonnull progressImage, id _Nullable downloadIdentifier) { + __typeof__(self) strongSelf = weakSelf; + if (strongSelf == nil) { + return; + } - ASDN::MutexLocker l(strongSelf->_downloadIdentifierLock); - //Getting a result back for a different download identifier, download must not have been successfully canceled - if (ASObjectIsEqual(strongSelf->_downloadIdentifier, downloadIdentifier) == NO && downloadIdentifier != nil) { - return; - } - strongSelf.image = progressImage; - }; - } - [_downloader setProgressImageBlock:progress callbackQueue:dispatch_get_main_queue() withDownloadIdentifier:_downloadIdentifier]; + ASDN::MutexLocker l(strongSelf->_downloadIdentifierLock); + //Getting a result back for a different download identifier, download must not have been successfully canceled + if (ASObjectIsEqual(strongSelf->_downloadIdentifier, downloadIdentifier) == NO && downloadIdentifier != nil) { + return; + } + strongSelf.image = progressImage; + }; + } + [_downloader setProgressImageBlock:progress callbackQueue:dispatch_get_main_queue() withDownloadIdentifier:_downloadIdentifier]; } - (void)_clearImage diff --git a/AsyncDisplayKit/ASNetworkImageNode.mm b/AsyncDisplayKit/ASNetworkImageNode.mm index fb3df668..8624be9f 100755 --- a/AsyncDisplayKit/ASNetworkImageNode.mm +++ b/AsyncDisplayKit/ASNetworkImageNode.mm @@ -212,7 +212,7 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; - (void)visibilityDidChange:(BOOL)isVisible { [super visibilityDidChange:isVisible]; - + ASDN::MutexLocker l(_lock); if (_downloaderImplementsSetPriority) { if (_downloadIdentifier != nil) { From 60c3ed27ff50bb24822d1229ecff44e752e400e4 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Tue, 12 Apr 2016 17:46:44 -0700 Subject: [PATCH 07/11] Fix more indentation --- AsyncDisplayKit/Details/ASPINRemoteImageDownloader.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/AsyncDisplayKit/Details/ASPINRemoteImageDownloader.m b/AsyncDisplayKit/Details/ASPINRemoteImageDownloader.m index 778f7cf1..1a157466 100644 --- a/AsyncDisplayKit/Details/ASPINRemoteImageDownloader.m +++ b/AsyncDisplayKit/Details/ASPINRemoteImageDownloader.m @@ -37,8 +37,8 @@ } - (void)cachedImageWithURL:(NSURL *)URL - callbackQueue:(dispatch_queue_t)callbackQueue - completion:(ASImageCacherCompletion)completion + callbackQueue:(dispatch_queue_t)callbackQueue + completion:(ASImageCacherCompletion)completion { // We do not check the cache here and instead check it in downloadImageWithURL to avoid checking the cache twice. // If we're targeting the main queue and we're on the main thread, complete immediately. From 140ca717b13215188d07be8126c140aa3513f220 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Tue, 12 Apr 2016 22:14:34 -0700 Subject: [PATCH 08/11] [ASMultiplexImageNode] Do not eagerly cancel image downloads --- AsyncDisplayKit/ASMultiplexImageNode.mm | 3 --- 1 file changed, 3 deletions(-) diff --git a/AsyncDisplayKit/ASMultiplexImageNode.mm b/AsyncDisplayKit/ASMultiplexImageNode.mm index d7ce8e31..eee0f731 100644 --- a/AsyncDisplayKit/ASMultiplexImageNode.mm +++ b/AsyncDisplayKit/ASMultiplexImageNode.mm @@ -416,9 +416,6 @@ typedef void(^ASMultiplexImageLoadCompletionBlock)(UIImage *image, id imageIdent - (void)_loadImageIdentifiers { - // Kill any in-flight downloads. - [self _setDownloadIdentifier:nil]; - // Grab the best possible image we can load right now. id bestImmediatelyAvailableImageIdentifier = nil; UIImage *bestImmediatelyAvailableImage = [self _bestImmediatelyAvailableImageFromDataSource:&bestImmediatelyAvailableImageIdentifier]; From f5adc7999b09d2e2f3401b9e2ee755f8cb925b05 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Fri, 22 Apr 2016 12:23:02 -0500 Subject: [PATCH 09/11] [ASMultiplexImageNode] Reduce lockage in visibilityDidChange --- AsyncDisplayKit/ASMultiplexImageNode.mm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/AsyncDisplayKit/ASMultiplexImageNode.mm b/AsyncDisplayKit/ASMultiplexImageNode.mm index eee0f731..e521cb83 100644 --- a/AsyncDisplayKit/ASMultiplexImageNode.mm +++ b/AsyncDisplayKit/ASMultiplexImageNode.mm @@ -294,8 +294,9 @@ typedef void(^ASMultiplexImageLoadCompletionBlock)(UIImage *image, id imageIdent - (void)visibilityDidChange:(BOOL)isVisible { [super visibilityDidChange:isVisible]; - ASDN::MutexLocker l(_downloadIdentifierLock); + if (_downloaderImplementsSetPriority) { + ASDN::MutexLocker l(_downloadIdentifierLock); if (_downloadIdentifier != nil) { if (isVisible) { [_downloader setPriority:ASImageDownloaderPriorityVisible withDownloadIdentifier:_downloadIdentifier]; From bc8a2b19ef7f0d1b5d81c82e8ce84d9372b6cf7e Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Fri, 22 Apr 2016 12:34:13 -0500 Subject: [PATCH 10/11] [ASNetworkImageNode] Carry recent progress image block changes over from multiplex image node --- AsyncDisplayKit/ASMultiplexImageNode.mm | 6 +++++- AsyncDisplayKit/ASNetworkImageNode.mm | 12 ++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/AsyncDisplayKit/ASMultiplexImageNode.mm b/AsyncDisplayKit/ASMultiplexImageNode.mm index e521cb83..7fcad269 100644 --- a/AsyncDisplayKit/ASMultiplexImageNode.mm +++ b/AsyncDisplayKit/ASMultiplexImageNode.mm @@ -452,9 +452,13 @@ typedef void(^ASMultiplexImageLoadCompletionBlock)(UIImage *image, id imageIdent #pragma mark - +/** + @note: This should be called without _downloadIdentifierLock held. We will lock + super to read our interface state and it's best to avoid acquiring both locks. + */ - (void)_updateProgressImageBlockOnDownloaderIfNeeded { - // Read our interface state so that we don't lock super while holding our lock. + // Read our interface state before locking so that we don't lock super while holding our lock. ASInterfaceState interfaceState = self.interfaceState; ASDN::MutexLocker l(_downloadIdentifierLock); diff --git a/AsyncDisplayKit/ASNetworkImageNode.mm b/AsyncDisplayKit/ASNetworkImageNode.mm index 8624be9f..f75a6091 100755 --- a/AsyncDisplayKit/ASNetworkImageNode.mm +++ b/AsyncDisplayKit/ASNetworkImageNode.mm @@ -213,8 +213,8 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; { [super visibilityDidChange:isVisible]; - ASDN::MutexLocker l(_lock); if (_downloaderImplementsSetPriority) { + ASDN::MutexLocker l(_lock); if (_downloadIdentifier != nil) { if (isVisible) { [_downloader setPriority:ASImageDownloaderPriorityVisible withDownloadIdentifier:_downloadIdentifier]; @@ -254,14 +254,22 @@ static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; #pragma mark - Private methods -- only call with lock. +/** + @note: This should be called without _lock held. We will lock + super to read our interface state and it's best to avoid acquiring both locks. + */ - (void)_updateProgressImageBlockOnDownloaderIfNeeded { + // Read our interface state before locking so that we don't lock super while holding our lock. + ASInterfaceState interfaceState = self.interfaceState; + ASDN::MutexLocker l(_lock); + if (!_downloaderImplementsSetProgress || _downloadIdentifier == nil) { return; } ASImageDownloaderProgressImage progress = nil; - if (ASInterfaceStateIncludesVisible(self.interfaceState)) { + if (ASInterfaceStateIncludesVisible(interfaceState)) { __weak __typeof__(self) weakSelf = self; progress = ^(UIImage * _Nonnull progressImage, id _Nullable downloadIdentifier) { __typeof__(self) strongSelf = weakSelf; From 05b22531e6c9c6cc3074c4058f475bc698558add Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Fri, 22 Apr 2016 19:12:38 -0500 Subject: [PATCH 11/11] Include necessary header --- AsyncDisplayKit/ASNetworkImageNode.mm | 1 + 1 file changed, 1 insertion(+) diff --git a/AsyncDisplayKit/ASNetworkImageNode.mm b/AsyncDisplayKit/ASNetworkImageNode.mm index 19e9af69..0ff4d51b 100755 --- a/AsyncDisplayKit/ASNetworkImageNode.mm +++ b/AsyncDisplayKit/ASNetworkImageNode.mm @@ -15,6 +15,7 @@ #import "ASThread.h" #import "ASInternalHelpers.h" #import "ASImageContainerProtocolCategories.h" +#import "ASDisplayNodeExtras.h" #if PIN_REMOTE_IMAGE #import "ASPINRemoteImageDownloader.h"