From eed78e37e8664ded1047b1261244cf30a67356e5 Mon Sep 17 00:00:00 2001 From: Olivier Poitrey Date: Tue, 12 Mar 2013 16:35:35 +0100 Subject: [PATCH] Ensure image isn't decoded twice if not necessary when SDWebImageRefreshCached flag is used #326 --- SDWebImage/SDWebImageDownloader.h | 11 +++++- SDWebImage/SDWebImageDownloader.m | 2 +- SDWebImage/SDWebImageDownloaderOperation.m | 39 ++++++++++++++-------- SDWebImage/SDWebImageManager.h | 8 +++-- SDWebImage/SDWebImageManager.m | 16 +++++++-- 5 files changed, 55 insertions(+), 21 deletions(-) diff --git a/SDWebImage/SDWebImageDownloader.h b/SDWebImage/SDWebImageDownloader.h index 1fab7e4a..3fdf9480 100644 --- a/SDWebImage/SDWebImageDownloader.h +++ b/SDWebImage/SDWebImageDownloader.h @@ -14,7 +14,16 @@ typedef enum { SDWebImageDownloaderLowPriority = 1 << 0, SDWebImageDownloaderProgressiveDownload = 1 << 1, - SDWebImageDownloaderEnableNSURLCache = 1 << 2 + /** + * By default, request prevent the of NSURLCache. With this flag, NSURLCache + * is used with default policies. + */ + SDWebImageDownloaderUseNSURLCache = 1 << 2, + /** + * Call completion block with nil image/imageData if the image was read from NSURLCache + * (to be combined with `SDWebImageDownloaderUseNSURLCache`). + */ + SDWebImageDownloaderIgnoreCachedResponse = 1 << 3 } SDWebImageDownloaderOptions; typedef enum diff --git a/SDWebImage/SDWebImageDownloader.m b/SDWebImage/SDWebImageDownloader.m index 8e776fd0..ae8c9eb0 100644 --- a/SDWebImage/SDWebImageDownloader.m +++ b/SDWebImage/SDWebImageDownloader.m @@ -120,7 +120,7 @@ static NSString *const kCompletedCallbackKey = @"completed"; [self addProgressCallback:progressBlock andCompletedBlock:completedBlock forURL:url createCallback:^ { // In order to prevent from potential duplicate caching (NSURLCache + SDImageCache) we disable the cache for image requests if told otherwise - NSMutableURLRequest *request = [NSMutableURLRequest.alloc initWithURL:url cachePolicy:(options & SDWebImageDownloaderEnableNSURLCache ? NSURLRequestUseProtocolCachePolicy : NSURLRequestReloadIgnoringLocalCacheData) timeoutInterval:15]; + NSMutableURLRequest *request = [NSMutableURLRequest.alloc initWithURL:url cachePolicy:(options & SDWebImageDownloaderUseNSURLCache ? NSURLRequestUseProtocolCachePolicy : NSURLRequestReloadIgnoringLocalCacheData) timeoutInterval:15]; request.HTTPShouldHandleCookies = NO; request.HTTPShouldUsePipelining = YES; request.allHTTPHeaderFields = wself.HTTPHeaders; diff --git a/SDWebImage/SDWebImageDownloaderOperation.m b/SDWebImage/SDWebImageDownloaderOperation.m index cbe45d84..07b3ad53 100644 --- a/SDWebImage/SDWebImageDownloaderOperation.m +++ b/SDWebImage/SDWebImageDownloaderOperation.m @@ -28,6 +28,7 @@ @implementation SDWebImageDownloaderOperation { size_t width, height; + BOOL responseFromCached; } - (id)initWithRequest:(NSURLRequest *)request queue:(dispatch_queue_t)queue options:(SDWebImageDownloaderOptions)options progress:(void (^)(NSUInteger, long long))progressBlock completed:(void (^)(UIImage *, NSData *, NSError *, BOOL))completedBlock cancelled:(void (^)())cancelBlock @@ -43,6 +44,7 @@ _executing = NO; _finished = NO; _expectedSize = 0; + responseFromCached = YES; // Initially wrong until `connection:willCacheResponse:` is called or not called } return self; } @@ -271,25 +273,35 @@ [[NSNotificationCenter defaultCenter] postNotificationName:SDWebImageDownloadStopNotification object:nil]; SDWebImageDownloaderCompletedBlock completionBlock = self.completedBlock; + if (completionBlock) { - dispatch_async(self.queue, ^ + if (self.options & SDWebImageDownloaderIgnoreCachedResponse && responseFromCached) { - UIImage *image = [UIImage decodedImageWithImage:SDScaledImageForPath(self.request.URL.absoluteString, self.imageData)]; - dispatch_async(dispatch_get_main_queue(), ^ + completionBlock(nil, nil, nil, YES); + self.completionBlock = nil; + [self done]; + } + else + { + dispatch_async(self.queue, ^ { - if (CGSizeEqualToSize(image.size, CGSizeZero)) + UIImage *image = [UIImage decodedImageWithImage:SDScaledImageForPath(self.request.URL.absoluteString, self.imageData)]; + dispatch_async(dispatch_get_main_queue(), ^ { - completionBlock(nil, nil, [NSError errorWithDomain:@"SDWebImageErrorDomain" code:0 userInfo:@{NSLocalizedDescriptionKey: @"Downloaded image has 0 pixels"}], YES); - } - else - { - completionBlock(image, self.imageData, nil, YES); - } - self.completionBlock = nil; - [self done]; + if (CGSizeEqualToSize(image.size, CGSizeZero)) + { + completionBlock(nil, nil, [NSError errorWithDomain:@"SDWebImageErrorDomain" code:0 userInfo:@{NSLocalizedDescriptionKey: @"Downloaded image has 0 pixels"}], YES); + } + else + { + completionBlock(image, self.imageData, nil, YES); + } + self.completionBlock = nil; + [self done]; + }); }); - }); + } } else { @@ -311,6 +323,7 @@ - (NSCachedURLResponse *)connection:(NSURLConnection *)connection willCacheResponse:(NSCachedURLResponse *)cachedResponse { + responseFromCached = NO; // If this method is called, it means the response wasn't read from cache if (self.request.cachePolicy == NSURLRequestReloadIgnoringLocalCacheData) { // Prevents caching of responses diff --git a/SDWebImage/SDWebImageManager.h b/SDWebImage/SDWebImageManager.h index a686978c..a6e39ace 100644 --- a/SDWebImage/SDWebImageManager.h +++ b/SDWebImage/SDWebImageManager.h @@ -33,10 +33,12 @@ typedef enum */ SDWebImageProgressiveDownload = 1 << 3, /** - * Even if the image is cached, fetch the URL again anyway. When set, NSURLCache is enabled in the downloader. - * NSURLCache will handle the protocol caching logic while SDWebImage remains useful for offline images. + * Even if the image is cached, respect the HTTP response cache control, and refresh the image from remote location if needed. + * The disk caching will be handled by NSURLCache instead of SDWebImage leading to slight performance degradation. * This option helps deal with images changing behind the same request URL, e.g. Facebook graph api profile pics. - * If a cached image exists, the completion block is called once with the cached image and again with the final image. + * If a cached image is refreshed, the completion block is called once with the cached image and again with the final image. + * + * Use this flag only if you can't make your URLs static with embeded cache busting parameter. */ SDWebImageRefreshCached = 1 << 4 } SDWebImageOptions; diff --git a/SDWebImage/SDWebImageManager.m b/SDWebImage/SDWebImageManager.m index c43f132f..8bf5cf98 100644 --- a/SDWebImage/SDWebImageManager.m +++ b/SDWebImage/SDWebImageManager.m @@ -107,8 +107,14 @@ SDWebImageDownloaderOptions downloaderOptions = 0; if (options & SDWebImageLowPriority) downloaderOptions |= SDWebImageDownloaderLowPriority; if (options & SDWebImageProgressiveDownload) downloaderOptions |= SDWebImageDownloaderProgressiveDownload; - if (options & SDWebImageRefreshCached) downloaderOptions |= SDWebImageDownloaderEnableNSURLCache; - if (image && options & SDWebImageRefreshCached) downloaderOptions ^= SDWebImageDownloaderProgressiveDownload; // force progressive off if image is refreshing + if (options & SDWebImageRefreshCached) downloaderOptions |= SDWebImageDownloaderUseNSURLCache; + if (image && options & SDWebImageRefreshCached) + { + // force progressive off if image already cached but forced refreshing + downloaderOptions ^= SDWebImageDownloaderProgressiveDownload; + // ignore image read from NSURLCache if image if cached but force refreshing + downloaderOptions |= SDWebImageDownloaderIgnoreCachedResponse; + } __block id subOperation = [self.imageDownloader downloadImageWithURL:url options:downloaderOptions progress:progressBlock completed:^(UIImage *downloadedImage, NSData *data, NSError *error, BOOL finished) { if (weakOperation.cancelled) @@ -138,7 +144,11 @@ cacheOnDisk = NO; } - if (downloadedImage && [self.delegate respondsToSelector:@selector(imageManager:transformDownloadedImage:withURL:)]) + if (options & SDWebImageRefreshCached && image && !downloadedImage) + { + // Image refresh hit the NSURLCache cache, do not call the completion block + } + else if (downloadedImage && [self.delegate respondsToSelector:@selector(imageManager:transformDownloadedImage:withURL:)]) { dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0), ^ {