Merge pull request #3363 from dreampiggy/fix_multiple_cancel_cache_callback_order

Fix the case when user cancel the image loading for same URL in sequence cause placeholder mass
This commit is contained in:
DreamPiggy 2022-06-26 23:52:55 +08:00 committed by GitHub
commit 16cf157658
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 191 additions and 38 deletions

View File

@ -55,6 +55,20 @@ typedef NS_OPTIONS(NSUInteger, SDImageCacheOptions) {
SDImageCacheMatchAnimatedImageClass = 1 << 7,
};
@interface SDImageCacheToken : NSObject <SDWebImageOperation>
/**
Cancel the current cache query.
*/
- (void)cancel;
/**
The query's cache key.
*/
@property (nonatomic, strong, nullable, readonly) NSString *key;
@end
/**
* SDImageCache maintains a memory cache and a disk cache. Disk cache write operations are performed
* asynchronous so it doesnt add unnecessary latency to the UI.
@ -270,9 +284,9 @@ typedef NS_OPTIONS(NSUInteger, SDImageCacheOptions) {
* @param key The unique key used to store the wanted image. If you want transformed or thumbnail image, calculate the key with `SDTransformedKeyForKey`, `SDThumbnailedKeyForKey`, or generate the cache key from url with `cacheKeyForURL:context:`.
* @param doneBlock The completion block. Will not get called if the operation is cancelled
*
* @return a NSOperation instance containing the cache op
* @return a SDImageCacheToken instance containing the cache operation, will callback immediately when cancelled
*/
- (nullable NSOperation *)queryCacheOperationForKey:(nullable NSString *)key done:(nullable SDImageCacheQueryCompletionBlock)doneBlock;
- (nullable SDImageCacheToken *)queryCacheOperationForKey:(nullable NSString *)key done:(nullable SDImageCacheQueryCompletionBlock)doneBlock;
/**
* Asynchronously queries the cache with operation and call the completion when done.
@ -281,9 +295,9 @@ typedef NS_OPTIONS(NSUInteger, SDImageCacheOptions) {
* @param options A mask to specify options to use for this cache query
* @param doneBlock The completion block. Will not get called if the operation is cancelled
*
* @return a NSOperation instance containing the cache op
* @return a SDImageCacheToken instance containing the cache operation, will callback immediately when cancelled
*/
- (nullable NSOperation *)queryCacheOperationForKey:(nullable NSString *)key options:(SDImageCacheOptions)options done:(nullable SDImageCacheQueryCompletionBlock)doneBlock;
- (nullable SDImageCacheToken *)queryCacheOperationForKey:(nullable NSString *)key options:(SDImageCacheOptions)options done:(nullable SDImageCacheQueryCompletionBlock)doneBlock;
/**
* Asynchronously queries the cache with operation and call the completion when done.
@ -293,9 +307,9 @@ typedef NS_OPTIONS(NSUInteger, SDImageCacheOptions) {
* @param context A context contains different options to perform specify changes or processes, see `SDWebImageContextOption`. This hold the extra objects which `options` enum can not hold.
* @param doneBlock The completion block. Will not get called if the operation is cancelled
*
* @return a NSOperation instance containing the cache op
* @return a SDImageCacheToken instance containing the cache operation, will callback immediately when cancellederation, will callback immediately when cancelled
*/
- (nullable NSOperation *)queryCacheOperationForKey:(nullable NSString *)key options:(SDImageCacheOptions)options context:(nullable SDWebImageContext *)context done:(nullable SDImageCacheQueryCompletionBlock)doneBlock;
- (nullable SDImageCacheToken *)queryCacheOperationForKey:(nullable NSString *)key options:(SDImageCacheOptions)options context:(nullable SDWebImageContext *)context done:(nullable SDImageCacheQueryCompletionBlock)doneBlock;
/**
* Asynchronously queries the cache with operation and call the completion when done.
@ -306,9 +320,9 @@ typedef NS_OPTIONS(NSUInteger, SDImageCacheOptions) {
* @param queryCacheType Specify where to query the cache from. By default we use `.all`, which means both memory cache and disk cache. You can choose to query memory only or disk only as well. Pass `.none` is invalid and callback with nil immediately.
* @param doneBlock The completion block. Will not get called if the operation is cancelled
*
* @return a NSOperation instance containing the cache op
* @return a SDImageCacheToken instance containing the cache operation, will callback immediately when cancelled
*/
- (nullable NSOperation *)queryCacheOperationForKey:(nullable NSString *)key options:(SDImageCacheOptions)options context:(nullable SDWebImageContext *)context cacheType:(SDImageCacheType)queryCacheType done:(nullable SDImageCacheQueryCompletionBlock)doneBlock;
- (nullable SDImageCacheToken *)queryCacheOperationForKey:(nullable NSString *)key options:(SDImageCacheOptions)options context:(nullable SDWebImageContext *)context cacheType:(SDImageCacheType)queryCacheType done:(nullable SDImageCacheQueryCompletionBlock)doneBlock;
/**
* Synchronously query the memory cache.

View File

@ -15,6 +15,42 @@
#import "UIImage+Metadata.h"
#import "UIImage+ExtendedCacheData.h"
@interface SDImageCacheToken ()
@property (nonatomic, strong, nullable, readwrite) NSString *key;
@property (nonatomic, assign, getter=isCancelled) BOOL cancelled;
@property (nonatomic, copy, nullable) SDImageCacheQueryCompletionBlock doneBlock;
@end
@implementation SDImageCacheToken
-(instancetype)initWithDoneBlock:(nullable SDImageCacheQueryCompletionBlock)doneBlock {
self = [super init];
if (self) {
self.doneBlock = doneBlock;
}
return self;
}
- (void)cancel {
@synchronized (self) {
if (self.isCancelled) {
return;
}
self.cancelled = YES;
dispatch_main_async_safe(^{
if (self.doneBlock) {
self.doneBlock(nil, nil, SDImageCacheTypeNone);
self.doneBlock = nil;
}
});
}
}
@end
static NSString * _defaultDiskCacheDirectory;
@interface SDImageCache ()
@ -493,19 +529,19 @@ static NSString * _defaultDiskCacheDirectory;
image.sd_extendedObject = extendedObject;
}
- (nullable NSOperation *)queryCacheOperationForKey:(NSString *)key done:(SDImageCacheQueryCompletionBlock)doneBlock {
- (nullable SDImageCacheToken *)queryCacheOperationForKey:(NSString *)key done:(SDImageCacheQueryCompletionBlock)doneBlock {
return [self queryCacheOperationForKey:key options:0 done:doneBlock];
}
- (nullable NSOperation *)queryCacheOperationForKey:(NSString *)key options:(SDImageCacheOptions)options done:(SDImageCacheQueryCompletionBlock)doneBlock {
- (nullable SDImageCacheToken *)queryCacheOperationForKey:(NSString *)key options:(SDImageCacheOptions)options done:(SDImageCacheQueryCompletionBlock)doneBlock {
return [self queryCacheOperationForKey:key options:options context:nil done:doneBlock];
}
- (nullable NSOperation *)queryCacheOperationForKey:(nullable NSString *)key options:(SDImageCacheOptions)options context:(nullable SDWebImageContext *)context done:(nullable SDImageCacheQueryCompletionBlock)doneBlock {
- (nullable SDImageCacheToken *)queryCacheOperationForKey:(nullable NSString *)key options:(SDImageCacheOptions)options context:(nullable SDWebImageContext *)context done:(nullable SDImageCacheQueryCompletionBlock)doneBlock {
return [self queryCacheOperationForKey:key options:options context:context cacheType:SDImageCacheTypeAll done:doneBlock];
}
- (nullable NSOperation *)queryCacheOperationForKey:(nullable NSString *)key options:(SDImageCacheOptions)options context:(nullable SDWebImageContext *)context cacheType:(SDImageCacheType)queryCacheType done:(nullable SDImageCacheQueryCompletionBlock)doneBlock {
- (nullable SDImageCacheToken *)queryCacheOperationForKey:(nullable NSString *)key options:(SDImageCacheOptions)options context:(nullable SDWebImageContext *)context cacheType:(SDImageCacheType)queryCacheType done:(nullable SDImageCacheQueryCompletionBlock)doneBlock {
if (!key) {
if (doneBlock) {
doneBlock(nil, nil, SDImageCacheTypeNone);
@ -556,23 +592,28 @@ static NSString * _defaultDiskCacheDirectory;
}
// Second check the disk cache...
NSOperation *operation = [NSOperation new];
SDImageCacheToken *operation = [[SDImageCacheToken alloc] initWithDoneBlock:doneBlock];
operation.key = key;
// Check whether we need to synchronously query disk
// 1. in-memory cache hit & memoryDataSync
// 2. in-memory cache miss & diskDataSync
BOOL shouldQueryDiskSync = ((image && options & SDImageCacheQueryMemoryDataSync) ||
(!image && options & SDImageCacheQueryDiskDataSync));
NSData* (^queryDiskDataBlock)(void) = ^NSData* {
if (operation.isCancelled) {
return nil;
@synchronized (operation) {
if (operation.isCancelled) {
return nil;
}
}
return [self diskImageDataBySearchingAllPathsForKey:key];
};
UIImage* (^queryDiskImageBlock)(NSData*) = ^UIImage*(NSData* diskData) {
if (operation.isCancelled) {
return nil;
@synchronized (operation) {
if (operation.isCancelled) {
return nil;
}
}
UIImage *diskImage;
@ -614,6 +655,11 @@ static NSString * _defaultDiskCacheDirectory;
dispatch_async(self.ioQueue, ^{
NSData* diskData = queryDiskDataBlock();
UIImage* diskImage = queryDiskImageBlock(diskData);
@synchronized (operation) {
if (operation.isCancelled) {
return;
}
}
if (doneBlock) {
dispatch_async(dispatch_get_main_queue(), ^{
doneBlock(diskImage, diskData, SDImageCacheTypeDisk);

View File

@ -105,6 +105,8 @@ typedef NS_OPTIONS(NSUInteger, SDWebImageOptions) {
/**
* By default, placeholder images are loaded while the image is loading. This flag will delay the loading
* of the placeholder image until after the image has finished loading.
* @note This is used to treate placeholder as an **Error Placeholder** but not **Loading Placeholder** by defaults. if the image loading is cancelled or error, the placeholder will be always set.
* @note Therefore, if you want both **Error Placeholder** and **Loading Placeholder** exist, use `SDWebImageAvoidAutoSetImage` to manually set the two placeholders and final loaded image by your hand depends on loading result.
*/
SDWebImageDelayPlaceholder = 1 << 8,

View File

@ -11,8 +11,14 @@
/// A protocol represents cancelable operation.
@protocol SDWebImageOperation <NSObject>
/// Cancel the operation
- (void)cancel;
@optional
/// Whether the operation has been cancelled.
@property (nonatomic, assign, readonly, getter=isCancelled) BOOL cancelled;
@end
/// NSOperation conform to `SDWebImageOperation`.

View File

@ -71,14 +71,15 @@ typedef void(^SDSetImageBlock)(UIImage * _Nullable image, NSData * _Nullable ima
* block is called a last time with the full image and the last parameter set to YES.
*
* The last parameter is the original image URL
* @return The returned operation for cancelling cache and download operation, typically type is `SDWebImageCombinedOperation`
*/
- (void)sd_internalSetImageWithURL:(nullable NSURL *)url
placeholderImage:(nullable UIImage *)placeholder
options:(SDWebImageOptions)options
context:(nullable SDWebImageContext *)context
setImageBlock:(nullable SDSetImageBlock)setImageBlock
progress:(nullable SDImageLoaderProgressBlock)progressBlock
completed:(nullable SDInternalCompletionBlock)completedBlock;
- (nullable id<SDWebImageOperation>)sd_internalSetImageWithURL:(nullable NSURL *)url
placeholderImage:(nullable UIImage *)placeholder
options:(SDWebImageOptions)options
context:(nullable SDWebImageContext *)context
setImageBlock:(nullable SDSetImageBlock)setImageBlock
progress:(nullable SDImageLoaderProgressBlock)progressBlock
completed:(nullable SDInternalCompletionBlock)completedBlock;
/**
* Cancel the current image load

View File

@ -47,13 +47,13 @@ const int64_t SDWebImageProgressUnitCountUnknown = 1LL;
objc_setAssociatedObject(self, @selector(sd_imageProgress), sd_imageProgress, OBJC_ASSOCIATION_RETAIN_NONATOMIC);
}
- (void)sd_internalSetImageWithURL:(nullable NSURL *)url
placeholderImage:(nullable UIImage *)placeholder
options:(SDWebImageOptions)options
context:(nullable SDWebImageContext *)context
setImageBlock:(nullable SDSetImageBlock)setImageBlock
progress:(nullable SDImageLoaderProgressBlock)progressBlock
completed:(nullable SDInternalCompletionBlock)completedBlock {
- (nullable id<SDWebImageOperation>)sd_internalSetImageWithURL:(nullable NSURL *)url
placeholderImage:(nullable UIImage *)placeholder
options:(SDWebImageOptions)options
context:(nullable SDWebImageContext *)context
setImageBlock:(nullable SDSetImageBlock)setImageBlock
progress:(nullable SDImageLoaderProgressBlock)progressBlock
completed:(nullable SDInternalCompletionBlock)completedBlock {
if (context) {
// copy to avoid mutable object
context = [context copy];
@ -99,6 +99,8 @@ const int64_t SDWebImageProgressUnitCountUnknown = 1LL;
});
}
id <SDWebImageOperation> operation = nil;
if (url) {
// reset the progress
NSProgress *imageProgress = objc_getAssociatedObject(self, @selector(sd_imageProgress));
@ -135,7 +137,7 @@ const int64_t SDWebImageProgressUnitCountUnknown = 1LL;
}
};
@weakify(self);
id <SDWebImageOperation> operation = [manager loadImageWithURL:url options:options context:context progress:combinedProgressBlock completed:^(UIImage *image, NSData *data, NSError *error, SDImageCacheType cacheType, BOOL finished, NSURL *imageURL) {
operation = [manager loadImageWithURL:url options:options context:context progress:combinedProgressBlock completed:^(UIImage *image, NSData *data, NSError *error, SDImageCacheType cacheType, BOOL finished, NSURL *imageURL) {
@strongify(self);
if (!self) { return; }
// if the progress not been updated, mark it to complete state
@ -234,6 +236,8 @@ const int64_t SDWebImageProgressUnitCountUnknown = 1LL;
}
});
}
return operation;
}
- (void)sd_cancelCurrentImageLoad {

View File

@ -32,14 +32,14 @@
- (void)sd_setImageLoadOperation:(nullable id<SDWebImageOperation>)operation forKey:(nullable NSString *)key;
/**
* Cancel all operations for the current UIView and key
* Cancel the operation for the current UIView and key
*
* @param key key for identifying the operations
*/
- (void)sd_cancelImageLoadOperationWithKey:(nullable NSString *)key;
/**
* Just remove the operations corresponding to the current UIView and key without cancelling them
* Just remove the operation corresponding to the current UIView and key without cancelling them
*
* @param key key for identifying the operations
*/

View File

@ -9,8 +9,6 @@
#import "UIView+WebCacheOperation.h"
#import "objc/runtime.h"
static char loadOperationKey;
// key is strong, value is weak because operation instance is retained by SDWebImageManager's runningOperations property
// we should use lock to keep thread-safe because these method may not be accessed from main queue
typedef NSMapTable<NSString *, id<SDWebImageOperation>> SDOperationsDictionary;
@ -19,12 +17,12 @@ typedef NSMapTable<NSString *, id<SDWebImageOperation>> SDOperationsDictionary;
- (SDOperationsDictionary *)sd_operationDictionary {
@synchronized(self) {
SDOperationsDictionary *operations = objc_getAssociatedObject(self, &loadOperationKey);
SDOperationsDictionary *operations = objc_getAssociatedObject(self, @selector(sd_operationDictionary));
if (operations) {
return operations;
}
operations = [[NSMapTable alloc] initWithKeyOptions:NSPointerFunctionsStrongMemory valueOptions:NSPointerFunctionsWeakMemory capacity:0];
objc_setAssociatedObject(self, &loadOperationKey, operations, OBJC_ASSOCIATION_RETAIN_NONATOMIC);
objc_setAssociatedObject(self, @selector(sd_operationDictionary), operations, OBJC_ASSOCIATION_RETAIN_NONATOMIC);
return operations;
}
}

View File

@ -224,6 +224,88 @@
[self waitForExpectationsWithCommonTimeout];
}
- (void)testUIViewCancelWithDelayPlaceholderShouldCallbackOnceBeforeSecond {
XCTestExpectation *expectation1 = [self expectationWithDescription:@"UIView internalSetImageWithURL call 1"];
XCTestExpectation *expectation2 = [self expectationWithDescription:@"UIView internalSetImageWithURL call 2"];
UIView *imageView = [[UIView alloc] init];
NSURL *originalImageURL = [NSURL URLWithString:kTestJPEGURL];
[SDImageCache.sharedImageCache removeImageFromDiskForKey:kTestJPEGURL];
[SDImageCache.sharedImageCache removeImageFromMemoryForKey:kTestJPEGURL];
__block NSUInteger calledSetImageTimes = 0;
__block NSUInteger calledSetImageTimes2 = 0;
NSString *operationKey = NSUUID.UUID.UUIDString;
UIImage *placeholder1 = UIImage.new;
id<SDWebImageOperation> op1 = [imageView sd_internalSetImageWithURL:originalImageURL placeholderImage:placeholder1 options:SDWebImageDelayPlaceholder context:@{ SDWebImageContextSetImageOperationKey:operationKey} setImageBlock:^(UIImage * _Nullable image, NSData * _Nullable imageData, SDImageCacheType cacheType, NSURL * _Nullable imageURL) {
// Should called before second query (We changed the cache callback in sync when cancelled)
expect(calledSetImageTimes2).equal(0);
calledSetImageTimes++;
} progress:nil completed:^(UIImage * _Nullable image, NSData * _Nullable data, NSError * _Nullable error, SDImageCacheType cacheType, BOOL finished, NSURL * _Nullable imageURL) {
if (calledSetImageTimes == 1) {
[expectation1 fulfill];
}
}];
[op1 cancel];
UIImage *placeholder2 = UIImage.new;
[imageView sd_internalSetImageWithURL:originalImageURL placeholderImage:placeholder2 options:0 context:@{ SDWebImageContextSetImageOperationKey:operationKey} setImageBlock:^(UIImage * _Nullable image, NSData * _Nullable imageData, SDImageCacheType cacheType, NSURL * _Nullable imageURL) {
if (calledSetImageTimes2 == 0) {
expect(image).equal(placeholder2);
} else {
expect(image).notTo.beNil();
}
calledSetImageTimes2++;
} progress:nil completed:^(UIImage * _Nullable image, NSData * _Nullable data, NSError * _Nullable error, SDImageCacheType cacheType, BOOL finished, NSURL * _Nullable imageURL) {
if (calledSetImageTimes2 == 2) {
[expectation2 fulfill];
}
}];
[self waitForExpectationsWithCommonTimeout];
}
- (void)testUIViewCancelWithoutDelayPlaceholderShouldCallbackOnceBeforeSecond {
XCTestExpectation *expectation1 = [self expectationWithDescription:@"UIView internalSetImageWithURL call 1"];
XCTestExpectation *expectation2 = [self expectationWithDescription:@"UIView internalSetImageWithURL call 2"];
UIView *imageView = [[UIView alloc] init];
NSURL *originalImageURL = [NSURL URLWithString:kTestJPEGURL];
[SDImageCache.sharedImageCache removeImageFromDiskForKey:kTestJPEGURL];
[SDImageCache.sharedImageCache removeImageFromMemoryForKey:kTestJPEGURL];
__block NSUInteger calledSetImageTimes = 0;
__block NSUInteger calledSetImageTimes2 = 0;
NSString *operationKey = NSUUID.UUID.UUIDString;
UIImage *placeholder1 = UIImage.new;
id<SDWebImageOperation> op1 = [imageView sd_internalSetImageWithURL:originalImageURL placeholderImage:placeholder1 options:0 context:@{ SDWebImageContextSetImageOperationKey:operationKey} setImageBlock:^(UIImage * _Nullable image, NSData * _Nullable imageData, SDImageCacheType cacheType, NSURL * _Nullable imageURL) {
// Should called before second query (We changed the cache callback in sync when cancelled)
expect(calledSetImageTimes2).equal(0);
calledSetImageTimes++;
} progress:nil completed:^(UIImage * _Nullable image, NSData * _Nullable data, NSError * _Nullable error, SDImageCacheType cacheType, BOOL finished, NSURL * _Nullable imageURL) {
if (calledSetImageTimes == 1) {
[expectation1 fulfill];
}
}];
[op1 cancel];
UIImage *placeholder2 = UIImage.new;
[imageView sd_internalSetImageWithURL:originalImageURL placeholderImage:placeholder2 options:0 context:@{ SDWebImageContextSetImageOperationKey:operationKey} setImageBlock:^(UIImage * _Nullable image, NSData * _Nullable imageData, SDImageCacheType cacheType, NSURL * _Nullable imageURL) {
if (calledSetImageTimes2 == 0) {
expect(image).equal(placeholder2);
} else {
expect(image).notTo.beNil();
}
calledSetImageTimes2++;
} progress:nil completed:^(UIImage * _Nullable image, NSData * _Nullable data, NSError * _Nullable error, SDImageCacheType cacheType, BOOL finished, NSURL * _Nullable imageURL) {
if (calledSetImageTimes2 == 2) {
[expectation2 fulfill];
}
}];
[self waitForExpectationsWithCommonTimeout];
}
- (void)testUIViewCancelCurrentImageLoad {
UIView *imageView = [[UIView alloc] init];
NSURL *originalImageURL = [NSURL URLWithString:kTestJPEGURL];