Fix the case when user cancel the image loading for same URL in sequence cause placeholder mass

This PR introduce 2 API changes:
1. Cache query now return cache token and callback sync when called from main queue, unlike NSOperation always callback async
2. Expose the set image operation, and update the documentation behavior about that `SDWebImageDelayPlaceholder`
This commit is contained in:
DreamPiggy 2022-06-26 19:51:51 +08:00
parent d58a1006c2
commit ad953cdcc5
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];