From eab427914046f56775bcab9e731d647536611fb8 Mon Sep 17 00:00:00 2001 From: Oana Popescu Date: Mon, 6 Jun 2016 19:12:56 +0300 Subject: [PATCH 1/2] Updated image downloader with a single session that manages all tasks - the URL session is created and maintained y the image downloader - the session will be injected to each operation and it will use it to create the data task - when delegate callbacks are called, the downloader will call the same delegate methods on the appropriate operation - if no session is injected upon creating a operation, it will create its own session for backwards compatibility. --- SDWebImage/SDWebImageDownloader.m | 83 ++++++++++- SDWebImage/SDWebImageDownloaderOperation.h | 33 ++++- SDWebImage/SDWebImageDownloaderOperation.m | 159 ++++++++++++--------- 3 files changed, 206 insertions(+), 69 deletions(-) diff --git a/SDWebImage/SDWebImageDownloader.m b/SDWebImage/SDWebImageDownloader.m index 96aefa7..ca70303 100644 --- a/SDWebImage/SDWebImageDownloader.m +++ b/SDWebImage/SDWebImageDownloader.m @@ -13,7 +13,7 @@ static NSString *const kProgressCallbackKey = @"progress"; static NSString *const kCompletedCallbackKey = @"completed"; -@interface SDWebImageDownloader () +@interface SDWebImageDownloader () @property (strong, nonatomic) NSOperationQueue *downloadQueue; @property (weak, nonatomic) NSOperation *lastAddedOperation; @@ -23,6 +23,9 @@ static NSString *const kCompletedCallbackKey = @"completed"; // This queue is used to serialize the handling of the network responses of all the download operation in a single queue @property (SDDispatchQueueSetterSementics, nonatomic) dispatch_queue_t barrierQueue; +// The session in which data tasks will run +@property (strong, nonatomic) NSURLSession *session; + @end @implementation SDWebImageDownloader @@ -74,11 +77,26 @@ static NSString *const kCompletedCallbackKey = @"completed"; #endif _barrierQueue = dispatch_queue_create("com.hackemist.SDWebImageDownloaderBarrierQueue", DISPATCH_QUEUE_CONCURRENT); _downloadTimeout = 15.0; + + NSURLSessionConfiguration *sessionConfig = [NSURLSessionConfiguration defaultSessionConfiguration]; + sessionConfig.timeoutIntervalForRequest = _downloadTimeout; + + /** + * Create the session for this task + * We send nil as delegate queue so that the session creates a serial operation queue for performing all delegate + * method calls and completion handler calls. + */ + self.session = [NSURLSession sessionWithConfiguration:sessionConfig + delegate:self + delegateQueue:nil]; } return self; } - (void)dealloc { + [self.session invalidateAndCancel]; + self.session = nil; + [self.downloadQueue cancelAllOperations]; SDDispatchQueueRelease(_barrierQueue); } @@ -133,6 +151,7 @@ static NSString *const kCompletedCallbackKey = @"completed"; request.allHTTPHeaderFields = wself.HTTPHeaders; } operation = [[wself.operationClass alloc] initWithRequest:request + inSession:self.session options:options progress:^(NSInteger receivedSize, NSInteger expectedSize) { SDWebImageDownloader *sself = wself; @@ -233,4 +252,66 @@ static NSString *const kCompletedCallbackKey = @"completed"; [self.downloadQueue cancelAllOperations]; } +#pragma mark Helper methods + +- (SDWebImageDownloaderOperation *)operationWithTask:(NSURLSessionTask *)task { + SDWebImageDownloaderOperation *returnOperation = nil; + for (SDWebImageDownloaderOperation *operation in self.downloadQueue.operations) { + if (operation.dataTask.taskIdentifier == task.taskIdentifier) { + returnOperation = operation; + break; + } + } + return returnOperation; +} + +#pragma mark NSURLSessionDataDelegate + +- (void)URLSession:(NSURLSession *)session + dataTask:(NSURLSessionDataTask *)dataTask +didReceiveResponse:(NSURLResponse *)response + completionHandler:(void (^)(NSURLSessionResponseDisposition disposition))completionHandler { + + // Identify the operation that runs this task and pass it the delegate method + SDWebImageDownloaderOperation *dataOperation = [self operationWithTask:dataTask]; + + [dataOperation URLSession:session dataTask:dataTask didReceiveResponse:response completionHandler:completionHandler]; +} + +- (void)URLSession:(NSURLSession *)session dataTask:(NSURLSessionDataTask *)dataTask didReceiveData:(NSData *)data { + + // Identify the operation that runs this task and pass it the delegate method + SDWebImageDownloaderOperation *dataOperation = [self operationWithTask:dataTask]; + + [dataOperation URLSession:session dataTask:dataTask didReceiveData:data]; +} + +- (void)URLSession:(NSURLSession *)session + dataTask:(NSURLSessionDataTask *)dataTask + willCacheResponse:(NSCachedURLResponse *)proposedResponse + completionHandler:(void (^)(NSCachedURLResponse *cachedResponse))completionHandler { + + // Identify the operation that runs this task and pass it the delegate method + SDWebImageDownloaderOperation *dataOperation = [self operationWithTask:dataTask]; + + [dataOperation URLSession:session dataTask:dataTask willCacheResponse:proposedResponse completionHandler:completionHandler]; +} + +#pragma mark NSURLSessionTaskDelegate + +- (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task didCompleteWithError:(NSError *)error { + // Identify the operation that runs this task and pass it the delegate method + SDWebImageDownloaderOperation *dataOperation = [self operationWithTask:task]; + + [dataOperation URLSession:session task:task didCompleteWithError:error]; +} + +- (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task didReceiveChallenge:(NSURLAuthenticationChallenge *)challenge completionHandler:(void (^)(NSURLSessionAuthChallengeDisposition disposition, NSURLCredential *credential))completionHandler { + + // Identify the operation that runs this task and pass it the delegate method + SDWebImageDownloaderOperation *dataOperation = [self operationWithTask:task]; + + [dataOperation URLSession:session task:task didReceiveChallenge:challenge completionHandler:completionHandler]; +} + @end diff --git a/SDWebImage/SDWebImageDownloaderOperation.h b/SDWebImage/SDWebImageDownloaderOperation.h index 4d31130..7a995d2 100644 --- a/SDWebImage/SDWebImageDownloaderOperation.h +++ b/SDWebImage/SDWebImageDownloaderOperation.h @@ -15,13 +15,18 @@ extern NSString *const SDWebImageDownloadReceiveResponseNotification; extern NSString *const SDWebImageDownloadStopNotification; extern NSString *const SDWebImageDownloadFinishNotification; -@interface SDWebImageDownloaderOperation : NSOperation +@interface SDWebImageDownloaderOperation : NSOperation /** - * The request used by the operation's connection. + * The request used by the operation's task. */ @property (strong, nonatomic, readonly) NSURLRequest *request; +/** + * The operation's task + */ +@property (strong, nonatomic, readonly) NSURLSessionTask *dataTask; + @property (assign, nonatomic) BOOL shouldDecompressImages; @@ -53,6 +58,7 @@ extern NSString *const SDWebImageDownloadFinishNotification; * @see SDWebImageDownloaderOperation * * @param request the URL request + * @param session the URL session in which this operation will run * @param options downloader options * @param progressBlock the block executed when a new chunk of data arrives. * @note the progress block is executed on a background queue @@ -63,9 +69,32 @@ extern NSString *const SDWebImageDownloadFinishNotification; * @return the initialized instance */ - (id)initWithRequest:(NSURLRequest *)request + inSession:(NSURLSession *)session options:(SDWebImageDownloaderOptions)options progress:(SDWebImageDownloaderProgressBlock)progressBlock completed:(SDWebImageDownloaderCompletedBlock)completedBlock cancelled:(SDWebImageNoParamsBlock)cancelBlock; +/** + * Initializes a `SDWebImageDownloaderOperation` object + * + * @see SDWebImageDownloaderOperation + * + * @param request the URL request + * @param options downloader options + * @param progressBlock the block executed when a new chunk of data arrives. + * @note the progress block is executed on a background queue + * @param completedBlock the block executed when the download is done. + * @note the completed block is executed on the main queue for success. If errors are found, there is a chance the block will be executed on a background queue + * @param cancelBlock the block executed if the download (operation) is cancelled + * + * @return the initialized instance. The operation will run in a separate session created for this operation + */ +- (id)initWithRequest:(NSURLRequest *)request + options:(SDWebImageDownloaderOptions)options + progress:(SDWebImageDownloaderProgressBlock)progressBlock + completed:(SDWebImageDownloaderCompletedBlock)completedBlock + cancelled:(SDWebImageNoParamsBlock)cancelBlock +__deprecated_msg("Method deprecated. Use `initWithRequest:inSession:options:progress:completed:cancelled`"); + @end diff --git a/SDWebImage/SDWebImageDownloaderOperation.m b/SDWebImage/SDWebImageDownloaderOperation.m index 2c36f92..901e638 100644 --- a/SDWebImage/SDWebImageDownloaderOperation.m +++ b/SDWebImage/SDWebImageDownloaderOperation.m @@ -17,7 +17,7 @@ NSString *const SDWebImageDownloadReceiveResponseNotification = @"SDWebImageDown NSString *const SDWebImageDownloadStopNotification = @"SDWebImageDownloadStopNotification"; NSString *const SDWebImageDownloadFinishNotification = @"SDWebImageDownloadFinishNotification"; -@interface SDWebImageDownloaderOperation () +@interface SDWebImageDownloaderOperation () @property (copy, nonatomic) SDWebImageDownloaderProgressBlock progressBlock; @property (copy, nonatomic) SDWebImageDownloaderCompletedBlock completedBlock; @@ -27,8 +27,13 @@ NSString *const SDWebImageDownloadFinishNotification = @"SDWebImageDownloadFinis @property (assign, nonatomic, getter = isFinished) BOOL finished; @property (strong, nonatomic) NSMutableData *imageData; -@property (strong, nonatomic) NSURLSession *session; -@property (strong, nonatomic) NSURLSessionDataTask *dataTask; +// This is weak because it is injected by whoever manages this session. If this gets nil-ed out, we won't be able to run +// the task associated with this operation +@property (weak, nonatomic) NSURLSession *session; +// This is set to NO if we're using an injected NSURLSession, and to YES otherwise +@property (assign, nonatomic) BOOL ownSession; + +@property (strong, nonatomic, readwrite) NSURLSessionTask *dataTask; @property (strong, atomic) NSThread *thread; @@ -52,6 +57,21 @@ NSString *const SDWebImageDownloadFinishNotification = @"SDWebImageDownloadFinis progress:(SDWebImageDownloaderProgressBlock)progressBlock completed:(SDWebImageDownloaderCompletedBlock)completedBlock cancelled:(SDWebImageNoParamsBlock)cancelBlock { + + return [self initWithRequest:request + inSession:nil + options:options + progress:progressBlock + completed:completedBlock + cancelled:cancelBlock]; +} + +- (id)initWithRequest:(NSURLRequest *)request + inSession:(NSURLSession *)session + options:(SDWebImageDownloaderOptions)options + progress:(SDWebImageDownloaderProgressBlock)progressBlock + completed:(SDWebImageDownloaderCompletedBlock)completedBlock + cancelled:(SDWebImageNoParamsBlock)cancelBlock { if ((self = [super init])) { _request = request; _shouldDecompressImages = YES; @@ -62,6 +82,7 @@ NSString *const SDWebImageDownloadFinishNotification = @"SDWebImageDownloadFinis _executing = NO; _finished = NO; _expectedSize = 0; + _session = session; responseFromCached = YES; // Initially wrong until `- URLSession:dataTask:willCacheResponse:completionHandler: is called or not called } return self; @@ -93,17 +114,21 @@ NSString *const SDWebImageDownloadFinishNotification = @"SDWebImageDownloadFinis }]; } #endif - NSURLSessionConfiguration *sessionConfig = [NSURLSessionConfiguration defaultSessionConfiguration]; - sessionConfig.timeoutIntervalForRequest = 15; - - /** - * Create the session for this task - * We send nil as delegate queue so that the session creates a serial operation queue for performing all delegate - * method calls and completion handler calls. - */ - self.session = [NSURLSession sessionWithConfiguration:sessionConfig - delegate:self - delegateQueue:nil]; + if (!self.session) { + self.ownSession = YES; + + NSURLSessionConfiguration *sessionConfig = [NSURLSessionConfiguration defaultSessionConfiguration]; + sessionConfig.timeoutIntervalForRequest = 15; + + /** + * Create the session for this task + * We send nil as delegate queue so that the session creates a serial operation queue for performing all delegate + * method calls and completion handler calls. + */ + self.session = [NSURLSession sessionWithConfiguration:sessionConfig + delegate:self + delegateQueue:nil]; + } self.executing = YES; self.dataTask = [self.session dataTaskWithRequest:self.request]; @@ -188,8 +213,10 @@ NSString *const SDWebImageDownloadFinishNotification = @"SDWebImageDownloadFinis self.dataTask = nil; self.imageData = nil; self.thread = nil; - [self.session invalidateAndCancel]; - self.session = nil; + if (self.ownSession) { + [self.session invalidateAndCancel]; + self.session = nil; + } } - (void)setFinished:(BOOL)finished { @@ -208,7 +235,7 @@ NSString *const SDWebImageDownloadFinishNotification = @"SDWebImageDownloadFinis return YES; } -#pragma mark NSURLSessionTaskDelegate +#pragma mark NSURLSessionDataDelegate - (void)URLSession:(NSURLSession *)session dataTask:(NSURLSessionDataTask *)dataTask @@ -339,32 +366,24 @@ didReceiveResponse:(NSURLResponse *)response } } -+ (UIImageOrientation)orientationFromPropertyValue:(NSInteger)value { - switch (value) { - case 1: - return UIImageOrientationUp; - case 3: - return UIImageOrientationDown; - case 8: - return UIImageOrientationLeft; - case 6: - return UIImageOrientationRight; - case 2: - return UIImageOrientationUpMirrored; - case 4: - return UIImageOrientationDownMirrored; - case 5: - return UIImageOrientationLeftMirrored; - case 7: - return UIImageOrientationRightMirrored; - default: - return UIImageOrientationUp; +- (void)URLSession:(NSURLSession *)session + dataTask:(NSURLSessionDataTask *)dataTask + willCacheResponse:(NSCachedURLResponse *)proposedResponse + completionHandler:(void (^)(NSCachedURLResponse *cachedResponse))completionHandler { + + responseFromCached = NO; // If this method is called, it means the response wasn't read from cache + NSCachedURLResponse *cachedResponse = proposedResponse; + + if (self.request.cachePolicy == NSURLRequestReloadIgnoringLocalCacheData) { + // Prevents caching of responses + cachedResponse = nil; + } + if (completionHandler) { + completionHandler(cachedResponse); } } -- (UIImage *)scaledImageForKey:(NSString *)key image:(UIImage *)image { - return SDScaledImageForKey(key, image); -} +#pragma mark NSURLSessionTaskDelegate - (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task didCompleteWithError:(NSError *)error { @synchronized(self) { @@ -419,32 +438,7 @@ didReceiveResponse:(NSURLResponse *)response [self done]; } -- (void)URLSession:(NSURLSession *)session - dataTask:(NSURLSessionDataTask *)dataTask - willCacheResponse:(NSCachedURLResponse *)proposedResponse - completionHandler:(void (^)(NSCachedURLResponse *cachedResponse))completionHandler { - - responseFromCached = NO; // If this method is called, it means the response wasn't read from cache - NSCachedURLResponse *cachedResponse = proposedResponse; - - if (self.request.cachePolicy == NSURLRequestReloadIgnoringLocalCacheData) { - // Prevents caching of responses - cachedResponse = nil; - } - if (completionHandler) { - completionHandler(cachedResponse); - } -} - -- (BOOL)shouldContinueWhenAppEntersBackground { - return self.options & SDWebImageDownloaderContinueInBackground; -} - -- (void)URLSession:(NSURLSession *)session - task:(NSURLSessionTask *)task -didReceiveChallenge:(NSURLAuthenticationChallenge *)challenge - completionHandler:(void (^)(NSURLSessionAuthChallengeDisposition disposition, - NSURLCredential *credential))completionHandler { +- (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task didReceiveChallenge:(NSURLAuthenticationChallenge *)challenge completionHandler:(void (^)(NSURLSessionAuthChallengeDisposition disposition, NSURLCredential *credential))completionHandler { NSURLSessionAuthChallengeDisposition disposition = NSURLSessionAuthChallengePerformDefaultHandling; __block NSURLCredential *credential = nil; @@ -474,4 +468,37 @@ didReceiveChallenge:(NSURLAuthenticationChallenge *)challenge } } +#pragma mark Helper methods + ++ (UIImageOrientation)orientationFromPropertyValue:(NSInteger)value { + switch (value) { + case 1: + return UIImageOrientationUp; + case 3: + return UIImageOrientationDown; + case 8: + return UIImageOrientationLeft; + case 6: + return UIImageOrientationRight; + case 2: + return UIImageOrientationUpMirrored; + case 4: + return UIImageOrientationDownMirrored; + case 5: + return UIImageOrientationLeftMirrored; + case 7: + return UIImageOrientationRightMirrored; + default: + return UIImageOrientationUp; + } +} + +- (UIImage *)scaledImageForKey:(NSString *)key image:(UIImage *)image { + return SDScaledImageForKey(key, image); +} + +- (BOOL)shouldContinueWhenAppEntersBackground { + return self.options & SDWebImageDownloaderContinueInBackground; +} + @end From 1e412927cc94f5b63c00680a1af73efaeb09f137 Mon Sep 17 00:00:00 2001 From: Oana Popescu Date: Mon, 6 Jun 2016 20:09:43 +0300 Subject: [PATCH 2/2] Keeping both owned and unowned sessions around as part of the operation. We need different ownership for both because we need to invalidate the owned session upon completion. The unowned session is wet to avoid cyclical references. --- SDWebImage/SDWebImageDownloaderOperation.m | 30 +++++++++++----------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/SDWebImage/SDWebImageDownloaderOperation.m b/SDWebImage/SDWebImageDownloaderOperation.m index 901e638..d4e2a99 100644 --- a/SDWebImage/SDWebImageDownloaderOperation.m +++ b/SDWebImage/SDWebImageDownloaderOperation.m @@ -29,9 +29,9 @@ NSString *const SDWebImageDownloadFinishNotification = @"SDWebImageDownloadFinis // This is weak because it is injected by whoever manages this session. If this gets nil-ed out, we won't be able to run // the task associated with this operation -@property (weak, nonatomic) NSURLSession *session; -// This is set to NO if we're using an injected NSURLSession, and to YES otherwise -@property (assign, nonatomic) BOOL ownSession; +@property (weak, nonatomic) NSURLSession *unownedSession; +// This is set if we're using not using an injected NSURLSession. We're responsible of invalidating this one +@property (strong, nonatomic) NSURLSession *ownedSession; @property (strong, nonatomic, readwrite) NSURLSessionTask *dataTask; @@ -82,7 +82,7 @@ NSString *const SDWebImageDownloadFinishNotification = @"SDWebImageDownloadFinis _executing = NO; _finished = NO; _expectedSize = 0; - _session = session; + _unownedSession = session; responseFromCached = YES; // Initially wrong until `- URLSession:dataTask:willCacheResponse:completionHandler: is called or not called } return self; @@ -114,24 +114,24 @@ NSString *const SDWebImageDownloadFinishNotification = @"SDWebImageDownloadFinis }]; } #endif - if (!self.session) { - self.ownSession = YES; - + NSURLSession *session = self.unownedSession; + if (!self.unownedSession) { NSURLSessionConfiguration *sessionConfig = [NSURLSessionConfiguration defaultSessionConfiguration]; sessionConfig.timeoutIntervalForRequest = 15; - + /** * Create the session for this task * We send nil as delegate queue so that the session creates a serial operation queue for performing all delegate * method calls and completion handler calls. */ - self.session = [NSURLSession sessionWithConfiguration:sessionConfig - delegate:self - delegateQueue:nil]; + self.ownedSession = [NSURLSession sessionWithConfiguration:sessionConfig + delegate:self + delegateQueue:nil]; + session = self.ownedSession; } + self.dataTask = [session dataTaskWithRequest:self.request]; self.executing = YES; - self.dataTask = [self.session dataTaskWithRequest:self.request]; self.thread = [NSThread currentThread]; } @@ -213,9 +213,9 @@ NSString *const SDWebImageDownloadFinishNotification = @"SDWebImageDownloadFinis self.dataTask = nil; self.imageData = nil; self.thread = nil; - if (self.ownSession) { - [self.session invalidateAndCancel]; - self.session = nil; + if (self.ownedSession) { + [self.ownedSession invalidateAndCancel]; + self.ownedSession = nil; } }