Merge pull request #3572 from Mervin1024/redundant_url_requests

Fix redundant requests for the same url during decoding time

SDWebImage/Core/SDWebImageDownloader.m

@@ -21,22 +21,6 @@ NSNotificationName const SDWebImageDownloadStopNotification = @"SDWebImageDownlo
 NSNotificationName const SDWebImageDownloadFinishNotification = @"SDWebImageDownloadFinishNotification";
 
 static void * SDWebImageDownloaderContext = &SDWebImageDownloaderContext;
-static void * SDWebImageDownloaderOperationKey = &SDWebImageDownloaderOperationKey;
-
-BOOL SDWebImageDownloaderOperationGetCompleted(id<SDWebImageDownloaderOperation> operation) {
-    NSCParameterAssert(operation);
-    NSNumber *value = objc_getAssociatedObject(operation, SDWebImageDownloaderOperationKey);
-    if (value != nil) {
-        return value.boolValue;
-    } else {
-        return NO;
-    }
-}
-
-void SDWebImageDownloaderOperationSetCompleted(id<SDWebImageDownloaderOperation> operation, BOOL isCompleted) {
-    NSCParameterAssert(operation);
-    objc_setAssociatedObject(operation, SDWebImageDownloaderOperationKey, @(isCompleted), OBJC_ASSOCIATION_RETAIN);
-}
 
 @interface SDWebImageDownloadToken ()
 
@@ -239,7 +223,7 @@ void SDWebImageDownloaderOperationSetCompleted(id<SDWebImageDownloaderOperation>
     BOOL shouldNotReuseOperation;
     if (operation) {
         @synchronized (operation) {
-            shouldNotReuseOperation = operation.isFinished || operation.isCancelled || SDWebImageDownloaderOperationGetCompleted(operation);
+            shouldNotReuseOperation = operation.isFinished || operation.isCancelled;
         }
     } else {
         shouldNotReuseOperation = YES;
@@ -518,12 +502,6 @@ didReceiveResponse:(NSURLResponse *)response
     
     // Identify the operation that runs this task and pass it the delegate method
     NSOperation<SDWebImageDownloaderOperation> *dataOperation = [self operationWithTask:task];
-    if (dataOperation) {
-        @synchronized (dataOperation) {
-            // Mark the downloader operation `isCompleted = YES`, no longer re-use this operation when new request comes in
-            SDWebImageDownloaderOperationSetCompleted(dataOperation, YES);
-        }
-    }
     if ([dataOperation respondsToSelector:@selector(URLSession:task:didCompleteWithError:)]) {
         [dataOperation URLSession:session task:task didCompleteWithError:error];
     }

SDWebImage/Core/SDWebImageDownloaderOperation.m

@@ -14,8 +14,6 @@
 #import "SDImageCacheDefine.h"
 #import "SDCallbackQueue.h"
 
-BOOL SDWebImageDownloaderOperationGetCompleted(id<SDWebImageDownloaderOperation> operation); // Private currently, mark open if needed
-
 // A handler to represent individual request
 @interface SDWebImageDownloaderOperationToken : NSObject
 
@@ -62,6 +60,8 @@ BOOL SDWebImageDownloaderOperationGetCompleted(id<SDWebImageDownloaderOperation>
 @property (strong, nonatomic, nullable) NSError *responseError;
 @property (assign, nonatomic) double previousProgress; // previous progress percent
 
+@property (assign, nonatomic, getter = isDownloadCompleted) BOOL downloadCompleted;
+
 @property (strong, nonatomic, nullable) id<SDWebImageDownloaderResponseModifier> responseModifier; // modify original URLResponse
 @property (strong, nonatomic, nullable) id<SDWebImageDownloaderDecryptor> decryptor; // decrypt image data
 
@@ -112,6 +112,7 @@ BOOL SDWebImageDownloaderOperationGetCompleted(id<SDWebImageDownloaderOperation>
         _finished = NO;
         _expectedSize = 0;
         _unownedSession = session;
+        _downloadCompleted = NO;
         _coderQueue = [[NSOperationQueue alloc] init];
         _coderQueue.maxConcurrentOperationCount = 1;
         _coderQueue.name = @"com.hackemist.SDWebImageDownloaderOperation.coderQueue";
@@ -338,6 +339,90 @@ BOOL SDWebImageDownloaderOperationGetCompleted(id<SDWebImageDownloaderOperation>
     return YES;
 }
 
+// Check for unprocessed tokens.
+// if all tokens have been processed call [self done].
+- (void)checkDoneWithImageData:(NSData *)imageData
+                finishedTokens:(NSArray<SDWebImageDownloaderOperationToken *> *)finishedTokens {
+    @synchronized (self) {
+        NSMutableArray<SDWebImageDownloaderOperationToken *> *tokens = [self.callbackTokens mutableCopy];
+        [finishedTokens enumerateObjectsUsingBlock:^(SDWebImageDownloaderOperationToken * _Nonnull obj, NSUInteger idx, BOOL * _Nonnull stop) {
+            [tokens removeObjectIdenticalTo:obj];
+        }];
+        if (tokens.count == 0) {
+            [self done];
+        } else {
+            // If there are new tokens added during the decoding operation, the decoding operation is supplemented with these new tokens.
+            [self startCoderOperationWithImageData:imageData pendingTokens:tokens finishedTokens:finishedTokens];
+        }
+    }
+}
+
+- (void)startCoderOperationWithImageData:(NSData *)imageData
+                           pendingTokens:(NSArray<SDWebImageDownloaderOperationToken *> *)pendingTokens
+                          finishedTokens:(NSArray<SDWebImageDownloaderOperationToken *> *)finishedTokens {
+    @weakify(self);
+    for (SDWebImageDownloaderOperationToken *token in pendingTokens) {
+        [self.coderQueue addOperationWithBlock:^{
+            @strongify(self);
+            if (!self) {
+                return;
+            }
+            UIImage *image;
+            // check if we already decode this variant of image for current callback
+            if (token.decodeOptions) {
+                image = [self.imageMap objectForKey:token.decodeOptions];
+            }
+            if (!image) {
+                // check if we already use progressive decoding, use that to produce faster decoding
+                id<SDProgressiveImageCoder> progressiveCoder = SDImageLoaderGetProgressiveCoder(self);
+                SDWebImageOptions options = [[self class] imageOptionsFromDownloaderOptions:self.options];
+                SDWebImageContext *context;
+                if (token.decodeOptions) {
+                    SDWebImageMutableContext *mutableContext = [NSMutableDictionary dictionaryWithDictionary:self.context];
+                    SDSetDecodeOptionsToContext(mutableContext, &options, token.decodeOptions);
+                    context = [mutableContext copy];
+                } else {
+                    context = self.context;
+                }
+                if (progressiveCoder) {
+                    image = SDImageLoaderDecodeProgressiveImageData(imageData, self.request.URL, YES, self, options, context);
+                } else {
+                    image = SDImageLoaderDecodeImageData(imageData, self.request.URL, options, context);
+                }
+                if (image && token.decodeOptions) {
+                    [self.imageMap setObject:image forKey:token.decodeOptions];
+                }
+            }
+            CGSize imageSize = image.size;
+            if (imageSize.width == 0 || imageSize.height == 0) {
+                NSString *description = image == nil ? @"Downloaded image decode failed" : @"Downloaded image has 0 pixels";
+                NSError *error = [NSError errorWithDomain:SDWebImageErrorDomain code:SDWebImageErrorBadImageData userInfo:@{NSLocalizedDescriptionKey : description}];
+                [self callCompletionBlockWithToken:token image:nil imageData:nil error:error finished:YES];
+            } else {
+                [self callCompletionBlockWithToken:token image:image imageData:imageData error:nil finished:YES];
+            }
+        }];
+    }
+    // call [self done] after all completed block was dispatched
+    dispatch_block_t doneBlock = ^{
+        @strongify(self);
+        if (!self) {
+            return;
+        }
+        // Check for new tokens added during the decode operation.
+        [self checkDoneWithImageData:imageData
+                      finishedTokens:[finishedTokens arrayByAddingObjectsFromArray:pendingTokens]];
+    };
+    if (@available(iOS 13, tvOS 13, macOS 10.15, watchOS 6, *)) {
+        // seems faster than `addOperationWithBlock`
+        [self.coderQueue addBarrierBlock:doneBlock];
+    } else {
+        // serial queue, this does the same effect in semantics
+        [self.coderQueue addOperationWithBlock:doneBlock];
+    }
+
+}
+
 #pragma mark NSURLSessionDataDelegate
 
 - (void)URLSession:(NSURLSession *)session
@@ -478,7 +563,7 @@ didReceiveResponse:(NSURLResponse *)response
                 }
                 // When cancelled or transfer finished (`didCompleteWithError`), cancel the progress callback, only completed block is called and enough
                 @synchronized (self) {
-                    if (self.isCancelled || SDWebImageDownloaderOperationGetCompleted(self)) {
+                    if (self.isCancelled || self.isDownloadCompleted) {
                         return;
                     }
                 }
@@ -521,6 +606,8 @@ didReceiveResponse:(NSURLResponse *)response
     // If we already cancel the operation or anything mark the operation finished, don't callback twice
     if (self.isFinished) return;
     
+    self.downloadCompleted = YES;
+    
     NSArray<SDWebImageDownloaderOperationToken *> *tokens;
     @synchronized (self) {
         tokens = [self.callbackTokens copy];
@@ -565,64 +652,9 @@ didReceiveResponse:(NSURLResponse *)response
                 } else {
                     // decode the image in coder queue, cancel all previous decoding process
                     [self.coderQueue cancelAllOperations];
-                    @weakify(self);
-                    for (SDWebImageDownloaderOperationToken *token in tokens) {
-                        [self.coderQueue addOperationWithBlock:^{
-                            @strongify(self);
-                            if (!self) {
-                                return;
-                            }
-                            UIImage *image;
-                            // check if we already decode this variant of image for current callback
-                            if (token.decodeOptions) {
-                                image = [self.imageMap objectForKey:token.decodeOptions];
-                            }
-                            if (!image) {
-                                // check if we already use progressive decoding, use that to produce faster decoding
-                                id<SDProgressiveImageCoder> progressiveCoder = SDImageLoaderGetProgressiveCoder(self);
-                                SDWebImageOptions options = [[self class] imageOptionsFromDownloaderOptions:self.options];
-                                SDWebImageContext *context;
-                                if (token.decodeOptions) {
-                                    SDWebImageMutableContext *mutableContext = [NSMutableDictionary dictionaryWithDictionary:self.context];
-                                    SDSetDecodeOptionsToContext(mutableContext, &options, token.decodeOptions);
-                                    context = [mutableContext copy];
-                                } else {
-                                    context = self.context;
-                                }
-                                if (progressiveCoder) {
-                                    image = SDImageLoaderDecodeProgressiveImageData(imageData, self.request.URL, YES, self, options, context);
-                                } else {
-                                    image = SDImageLoaderDecodeImageData(imageData, self.request.URL, options, context);
-                                }
-                                if (image && token.decodeOptions) {
-                                    [self.imageMap setObject:image forKey:token.decodeOptions];
-                                }
-                            }
-                            CGSize imageSize = image.size;
-                            if (imageSize.width == 0 || imageSize.height == 0) {
-                                NSString *description = image == nil ? @"Downloaded image decode failed" : @"Downloaded image has 0 pixels";
-                                NSError *error = [NSError errorWithDomain:SDWebImageErrorDomain code:SDWebImageErrorBadImageData userInfo:@{NSLocalizedDescriptionKey : description}];
-                                [self callCompletionBlockWithToken:token image:nil imageData:nil error:error finished:YES];
-                            } else {
-                                [self callCompletionBlockWithToken:token image:image imageData:imageData error:nil finished:YES];
-                            }
-                        }];
-                    }
-                    // call [self done] after all completed block was dispatched
-                    dispatch_block_t doneBlock = ^{
-                        @strongify(self);
-                        if (!self) {
-                            return;
-                        }
-                        [self done];
-                    };
-                    if (@available(iOS 13, tvOS 13, macOS 10.15, watchOS 6, *)) {
-                        // seems faster than `addOperationWithBlock`
-                        [self.coderQueue addBarrierBlock:doneBlock];
-                    } else {
-                        // serial queue, this does the same effect in semantics
-                        [self.coderQueue addOperationWithBlock:doneBlock];
-                    }
+                    [self startCoderOperationWithImageData:imageData
+                                             pendingTokens:tokens
+                                            finishedTokens:@[]];
                 }
             } else {
                 [self callCompletionBlocksWithError:[NSError errorWithDomain:SDWebImageErrorDomain code:SDWebImageErrorBadImageData userInfo:@{NSLocalizedDescriptionKey : @"Image data is nil"}]];