From 799168929c72f5b68e5d6490b07533910f7b85fb Mon Sep 17 00:00:00 2001 From: Nick Lockwood Date: Mon, 12 Oct 2015 04:14:21 -0700 Subject: [PATCH] Unify XCAsset handling logic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: @​public This diff unifies the logic for detecting when images refer to XCAsset files into a single function (RCTXCAssetNameForURL) and uses it for both +[RCTConvert UIImage:] and RCTImageLoader. I've also tightened the definition of XCAssets so that it only applies to images inside .car files, not any image inside the main bundle. This avoids using the +[UIImage imageNamed:] when not strictly necessary, which is desirable since that method is not thread-safe, and has undocumented caching behavior that is difficult to reason about. Reviewed By: @javache Differential Revision: D2526400 fb-gh-sync-id: 7199c2a44f1d55ff236d2c38a0a9368739b993d5 --- .../Image/RCTImage.xcodeproj/project.pbxproj | 12 +++--- Libraries/Image/RCTImageDownloader.m | 12 +++--- Libraries/Image/RCTImageLoader.m | 8 ++-- ...eImageLoader.h => RCTXCAssetImageLoader.h} | 2 +- ...eImageLoader.m => RCTXCAssetImageLoader.m} | 39 ++++--------------- Libraries/Network/RCTHTTPRequestHandler.m | 7 +++- React/Base/RCTConvert.m | 39 +++++++------------ React/Base/RCTUtils.h | 7 ++++ React/Base/RCTUtils.m | 35 +++++++++++++++++ 9 files changed, 89 insertions(+), 72 deletions(-) rename Libraries/Image/{RCTAssetBundleImageLoader.h => RCTXCAssetImageLoader.h} (83%) rename Libraries/Image/{RCTAssetBundleImageLoader.m => RCTXCAssetImageLoader.m} (57%) diff --git a/Libraries/Image/RCTImage.xcodeproj/project.pbxproj b/Libraries/Image/RCTImage.xcodeproj/project.pbxproj index a6412bcc38f..0a1360c1a42 100644 --- a/Libraries/Image/RCTImage.xcodeproj/project.pbxproj +++ b/Libraries/Image/RCTImage.xcodeproj/project.pbxproj @@ -13,11 +13,11 @@ 134B00A21B54232B00EC8DFB /* RCTImageUtils.m in Sources */ = {isa = PBXBuildFile; fileRef = 134B00A11B54232B00EC8DFB /* RCTImageUtils.m */; }; 13EF7F0B1BC42D4E003F47DD /* RCTShadowVirtualImage.m in Sources */ = {isa = PBXBuildFile; fileRef = 13EF7F081BC42D4E003F47DD /* RCTShadowVirtualImage.m */; settings = {ASSET_TAGS = (); }; }; 13EF7F0C1BC42D4E003F47DD /* RCTVirtualImageManager.m in Sources */ = {isa = PBXBuildFile; fileRef = 13EF7F0A1BC42D4E003F47DD /* RCTVirtualImageManager.m */; settings = {ASSET_TAGS = (); }; }; + 13EF7F7F1BC825B1003F47DD /* RCTXCAssetImageLoader.m in Sources */ = {isa = PBXBuildFile; fileRef = 13EF7F7E1BC825B1003F47DD /* RCTXCAssetImageLoader.m */; settings = {ASSET_TAGS = (); }; }; 143879381AAD32A300F088A5 /* RCTImageLoader.m in Sources */ = {isa = PBXBuildFile; fileRef = 143879371AAD32A300F088A5 /* RCTImageLoader.m */; }; 35123E6B1B59C99D00EBAD80 /* RCTImageStoreManager.m in Sources */ = {isa = PBXBuildFile; fileRef = 35123E6A1B59C99D00EBAD80 /* RCTImageStoreManager.m */; }; 354631681B69857700AA0B86 /* RCTImageEditingManager.m in Sources */ = {isa = PBXBuildFile; fileRef = 354631671B69857700AA0B86 /* RCTImageEditingManager.m */; }; 58B5118F1A9E6BD600147676 /* RCTImageDownloader.m in Sources */ = {isa = PBXBuildFile; fileRef = 58B5118A1A9E6BD600147676 /* RCTImageDownloader.m */; }; - 83DDA1571B8DCA5800892A1C /* RCTAssetBundleImageLoader.m in Sources */ = {isa = PBXBuildFile; fileRef = 83DDA1561B8DCA5800892A1C /* RCTAssetBundleImageLoader.m */; }; /* End PBXBuildFile section */ /* Begin PBXCopyFilesBuildPhase section */ @@ -45,6 +45,8 @@ 13EF7F081BC42D4E003F47DD /* RCTShadowVirtualImage.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTShadowVirtualImage.m; sourceTree = ""; }; 13EF7F091BC42D4E003F47DD /* RCTVirtualImageManager.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RCTVirtualImageManager.h; sourceTree = ""; }; 13EF7F0A1BC42D4E003F47DD /* RCTVirtualImageManager.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTVirtualImageManager.m; sourceTree = ""; }; + 13EF7F7D1BC825B1003F47DD /* RCTXCAssetImageLoader.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RCTXCAssetImageLoader.h; sourceTree = ""; }; + 13EF7F7E1BC825B1003F47DD /* RCTXCAssetImageLoader.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTXCAssetImageLoader.m; sourceTree = ""; }; 143879361AAD32A300F088A5 /* RCTImageLoader.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RCTImageLoader.h; sourceTree = ""; }; 143879371AAD32A300F088A5 /* RCTImageLoader.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTImageLoader.m; sourceTree = ""; }; 35123E691B59C99D00EBAD80 /* RCTImageStoreManager.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RCTImageStoreManager.h; sourceTree = ""; }; @@ -54,8 +56,6 @@ 58B5115D1A9E6B3D00147676 /* libRCTImage.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = libRCTImage.a; sourceTree = BUILT_PRODUCTS_DIR; }; 58B511891A9E6BD600147676 /* RCTImageDownloader.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RCTImageDownloader.h; sourceTree = ""; }; 58B5118A1A9E6BD600147676 /* RCTImageDownloader.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTImageDownloader.m; sourceTree = ""; }; - 83DDA1551B8DCA5800892A1C /* RCTAssetBundleImageLoader.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RCTAssetBundleImageLoader.h; sourceTree = ""; }; - 83DDA1561B8DCA5800892A1C /* RCTAssetBundleImageLoader.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTAssetBundleImageLoader.m; sourceTree = ""; }; /* End PBXFileReference section */ /* Begin PBXFrameworksBuildPhase section */ @@ -72,8 +72,8 @@ 58B511541A9E6B3D00147676 = { isa = PBXGroup; children = ( - 83DDA1551B8DCA5800892A1C /* RCTAssetBundleImageLoader.h */, - 83DDA1561B8DCA5800892A1C /* RCTAssetBundleImageLoader.m */, + 13EF7F7D1BC825B1003F47DD /* RCTXCAssetImageLoader.h */, + 13EF7F7E1BC825B1003F47DD /* RCTXCAssetImageLoader.m */, 1304D5B01AA8C50D0002E2BE /* RCTGIFImageDecoder.h */, 1304D5B11AA8C50D0002E2BE /* RCTGIFImageDecoder.m */, 58B511891A9E6BD600147676 /* RCTImageDownloader.h */, @@ -173,8 +173,8 @@ 354631681B69857700AA0B86 /* RCTImageEditingManager.m in Sources */, 1304D5AB1AA8C4A30002E2BE /* RCTImageView.m in Sources */, 13EF7F0B1BC42D4E003F47DD /* RCTShadowVirtualImage.m in Sources */, + 13EF7F7F1BC825B1003F47DD /* RCTXCAssetImageLoader.m in Sources */, 134B00A21B54232B00EC8DFB /* RCTImageUtils.m in Sources */, - 83DDA1571B8DCA5800892A1C /* RCTAssetBundleImageLoader.m in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; diff --git a/Libraries/Image/RCTImageDownloader.m b/Libraries/Image/RCTImageDownloader.m index 5b8befb8b83..bdeae5df6c8 100644 --- a/Libraries/Image/RCTImageDownloader.m +++ b/Libraries/Image/RCTImageDownloader.m @@ -36,12 +36,12 @@ RCT_EXPORT_MODULE() - (BOOL)canLoadImageURL:(NSURL *)requestURL { - // Have to exclude 'file://' from the main bundle, otherwise this would conflict with RCTAssetBundleImageLoader - return - [requestURL.scheme compare:@"http" options:NSCaseInsensitiveSearch range:NSMakeRange(0, 4)] == NSOrderedSame || - ([requestURL.scheme caseInsensitiveCompare:@"file"] == NSOrderedSame && - ![requestURL.path hasPrefix:[NSBundle bundleForClass:[self class]].resourcePath]) || - [requestURL.scheme caseInsensitiveCompare:@"data"] == NSOrderedSame; + static NSSet *schemes = nil; + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ + schemes = [[NSSet alloc] initWithObjects:@"http", @"https", @"file", nil]; + }); + return [schemes containsObject:requestURL.scheme.lowercaseString]; } /** diff --git a/Libraries/Image/RCTImageLoader.m b/Libraries/Image/RCTImageLoader.m index d9109624317..e378bc4050f 100644 --- a/Libraries/Image/RCTImageLoader.m +++ b/Libraries/Image/RCTImageLoader.m @@ -96,14 +96,16 @@ RCT_EXPORT_MODULE() progressBlock:(RCTImageLoaderProgressBlock)progressBlock completionBlock:(RCTImageLoaderCompletionBlock)completionBlock { - if ([imageTag isEqualToString:@""]) { + if (imageTag.length == 0) { RCTLogWarn(@"source.uri should not be an empty string "); - return nil; + return ^{}; } + NSURL *requestURL = [RCTConvert NSURL:imageTag]; id loadHandler = [self imageURLLoaderForRequest:requestURL]; if (!loadHandler) { RCTLogError(@"No suitable image URL loader found for %@", imageTag); + return ^{}; } return [loadHandler loadImageForURL:requestURL size:size scale:scale resizeMode:resizeMode progressHandler:^(int64_t progress, int64_t total) { @@ -120,7 +122,7 @@ RCT_EXPORT_MODULE() } } completionHandler:^(NSError *error, UIImage *image) { RCTDispatchCallbackOnMainQueue(completionBlock, error, image); - }] ?: ^{}; + }]; } - (id)imageDecoderForRequest:(NSData *)imageData diff --git a/Libraries/Image/RCTAssetBundleImageLoader.h b/Libraries/Image/RCTXCAssetImageLoader.h similarity index 83% rename from Libraries/Image/RCTAssetBundleImageLoader.h rename to Libraries/Image/RCTXCAssetImageLoader.h index ff460e08e38..ca982bf0363 100644 --- a/Libraries/Image/RCTAssetBundleImageLoader.h +++ b/Libraries/Image/RCTXCAssetImageLoader.h @@ -9,6 +9,6 @@ #import "RCTImageLoader.h" -@interface RCTAssetBundleImageLoader : NSObject +@interface RCTXCAssetImageLoader : NSObject @end diff --git a/Libraries/Image/RCTAssetBundleImageLoader.m b/Libraries/Image/RCTXCAssetImageLoader.m similarity index 57% rename from Libraries/Image/RCTAssetBundleImageLoader.m rename to Libraries/Image/RCTXCAssetImageLoader.m index b407f876007..dd12b7c6a9b 100644 --- a/Libraries/Image/RCTAssetBundleImageLoader.m +++ b/Libraries/Image/RCTXCAssetImageLoader.m @@ -7,56 +7,33 @@ * of patent rights can be found in the PATENTS file in the same directory. */ -#import "RCTAssetBundleImageLoader.h" +#import "RCTXCAssetImageLoader.h" #import "RCTUtils.h" -@implementation RCTAssetBundleImageLoader +@implementation RCTXCAssetImageLoader RCT_EXPORT_MODULE() -- (NSString *)imageNameForRequestURL:(NSURL *)requestURL -{ - if (!requestURL.fileURL) { - return nil; - } - - NSString *resourcesPath = [NSBundle bundleForClass:[self class]].resourcePath; - NSString *requestPath = requestURL.absoluteURL.path; - if (requestPath.length < resourcesPath.length + 1) { - return nil; - } - - return [requestPath substringFromIndex:resourcesPath.length + 1]; -} - - (BOOL)canLoadImageURL:(NSURL *)requestURL { - NSString *imageName = [self imageNameForRequestURL:requestURL]; - if (!imageName.length) { - return NO; - } + return RCTIsXCAssetURL(requestURL); +} - NSBundle *bundle = [NSBundle bundleForClass:[self class]]; - if ([bundle URLForResource:imageName withExtension:nil] || - [bundle URLForResource:imageName withExtension:@"png"]) { - return YES; - } - - // Assume it's an image in the main asset catalog - return imageName.pathComponents.count == 1; +- (float)imageLoaderPriority +{ + return 100; // higher priority than any ordinary file loader } - (RCTImageLoaderCancellationBlock)loadImageForURL:(NSURL *)imageURL size:(CGSize)size scale:(CGFloat)scale resizeMode:(UIViewContentMode)resizeMode progressHandler:(RCTImageLoaderProgressBlock)progressHandler completionHandler:(RCTImageLoaderCompletionBlock)completionHandler { - NSString *imageName = [self imageNameForRequestURL:imageURL]; - __block BOOL cancelled = NO; dispatch_async(dispatch_get_main_queue(), ^{ if (cancelled) { return; } + NSString *imageName = RCTBundlePathForURL(imageURL); UIImage *image = [UIImage imageNamed:imageName]; if (image) { if (progressHandler) { diff --git a/Libraries/Network/RCTHTTPRequestHandler.m b/Libraries/Network/RCTHTTPRequestHandler.m index fe83e3f7c2b..4b4c403443c 100644 --- a/Libraries/Network/RCTHTTPRequestHandler.m +++ b/Libraries/Network/RCTHTTPRequestHandler.m @@ -47,7 +47,12 @@ RCT_EXPORT_MODULE() - (BOOL)canHandleRequest:(NSURLRequest *)request { - return [@[@"http", @"https", @"file"] containsObject:request.URL.scheme.lowercaseString]; + static NSSet *schemes = nil; + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ + schemes = [[NSSet alloc] initWithObjects:@"http", @"https", @"file", nil]; + }); + return [schemes containsObject:request.URL.scheme.lowercaseString]; } - (NSURLSessionDataTask *)sendRequest:(NSURLRequest *)request diff --git a/React/Base/RCTConvert.m b/React/Base/RCTConvert.m index 04721e37e9d..88e7e3f5198 100644 --- a/React/Base/RCTConvert.m +++ b/React/Base/RCTConvert.m @@ -12,6 +12,7 @@ #import #import "RCTDefines.h" +#import "RCTUtils.h" @implementation RCTConvert @@ -102,7 +103,10 @@ RCT_CUSTOM_CONVERTER(NSData *, NSData, [json dataUsingEncoding:NSUTF8StringEncod // Assume it's a resource path path = [[NSBundle bundleForClass:[self class]].resourcePath stringByAppendingPathComponent:path]; } - return [NSURL fileURLWithPath:path]; + if (!(URL = [NSURL fileURLWithPath:path])) { + RCTLogConvertError(json, @"a valid URL"); + } + return URL; } @catch (__unused NSException *e) { RCTLogConvertError(json, @"a valid URL"); @@ -417,7 +421,9 @@ RCT_CGSTRUCT_CONVERTER(CGAffineTransform, (@[ if ([json isKindOfClass:[NSString class]]) { path = json; } else if ([json isKindOfClass:[NSDictionary class]]) { - path = [self NSString:json[@"uri"]]; + if (!(path = [self NSString:json[@"uri"]])) { + return nil; + } scale = [self CGFloat:json[@"scale"]]; isPackagerAsset = [self BOOL:json[@"__packager_asset"]]; } else { @@ -427,36 +433,21 @@ RCT_CGSTRUCT_CONVERTER(CGAffineTransform, (@[ NSURL *URL = [self NSURL:path]; NSString *scheme = URL.scheme.lowercaseString; - if (URL && [scheme isEqualToString:@"file"]) { - if ([URL.path hasPrefix:[NSBundle mainBundle].resourcePath]) { - RCTAssertMainThread(); - + if ([scheme isEqualToString:@"file"]) { + if (RCTIsXCAssetURL(URL)) { // Image may reside inside a .car file, in which case we have no choice // but to use +[UIImage imageNamed] - but this method isn't thread safe - static NSMutableDictionary *XCAssetMap = nil; - if (!XCAssetMap) { - XCAssetMap = [NSMutableDictionary new]; - } - NSNumber *isAsset = XCAssetMap[path]; - if (!isAsset || isAsset.boolValue) { - image = [UIImage imageNamed:path]; - } - if (!isAsset) { - // Avoid calling `+imageNamed` again in future if it's not needed. - XCAssetMap[path] = @(image != nil); - } - } - - if (!image) { - NSString *filePath = URL.path; - + RCTAssertMainThread(); + NSString *assetName = RCTBundlePathForURL(URL); + image = [UIImage imageNamed:assetName]; + } else { // Attempt to load from the file system + NSString *filePath = URL.path; if (filePath.pathExtension.length == 0) { filePath = [filePath stringByAppendingPathExtension:@"png"]; } image = [UIImage imageWithContentsOfFile:filePath]; } - } else if ([scheme isEqualToString:@"data"]) { image = [UIImage imageWithData:[NSData dataWithContentsOfURL:URL]]; } else if ([scheme isEqualToString:@"http"] && isPackagerAsset) { diff --git a/React/Base/RCTUtils.h b/React/Base/RCTUtils.h index bc94d62f8e8..1f915e66d55 100644 --- a/React/Base/RCTUtils.h +++ b/React/Base/RCTUtils.h @@ -77,3 +77,10 @@ RCT_EXTERN NSURL *RCTDataURL(NSString *mimeType, NSData *data); // Gzip functionality - compression level in range 0 - 1 (-1 for default) RCT_EXTERN NSData *RCTGzipData(NSData *data, float level); + +// Returns the relative path within the main bundle for an absolute URL +// (or nil, if the URL does not specify a path within the main bundle) +RCT_EXTERN NSString *RCTBundlePathForURL(NSURL *URL); + +// Determines if a given image URL actually refers to an XCAsset +RCT_EXTERN BOOL RCTIsXCAssetURL(NSURL *imageURL); diff --git a/React/Base/RCTUtils.m b/React/Base/RCTUtils.m index c3e59b06f39..d262c2ced13 100644 --- a/React/Base/RCTUtils.m +++ b/React/Base/RCTUtils.m @@ -458,3 +458,38 @@ NSData *RCTGzipData(NSData *input, float level) return output; } + +NSString *RCTBundlePathForURL(NSURL *URL) +{ + if (!URL.fileURL) { + // Not a file path + return nil; + } + NSString *path = URL.path; + NSString *bundlePath = [[NSBundle mainBundle] resourcePath]; + if (![path hasPrefix:bundlePath]) { + // Not a bundle-relative file + return nil; + } + return [path substringFromIndex:bundlePath.length + 1]; +} + +BOOL RCTIsXCAssetURL(NSURL *imageURL) +{ + NSString *name = RCTBundlePathForURL(imageURL); + if (name.pathComponents.count != 1) { + // URL is invalid, or is a file path, not an XCAsset identifier + return NO; + } + NSString *extension = [name pathExtension]; + if (extension.length && ![extension isEqualToString:@"png"]) { + // Not a png + return NO; + } + extension = extension.length ? nil : @"png"; + if ([[NSBundle mainBundle] pathForResource:name ofType:extension]) { + // File actually exists in bundle, so is not an XCAsset + return NO; + } + return YES; +}