From acda29945395aa343cea320359f69ba4f81289bf Mon Sep 17 00:00:00 2001 From: Paige Sun Date: Fri, 11 Sep 2020 15:02:53 -0700 Subject: [PATCH] 6/6 Log image-rendered for Fabric image logging Reviewed By: fkgozali Differential Revision: D23450649 fbshipit-source-id: 58265a2c7855a2f4371d68637f09a07921821adf --- Libraries/Image/RCTImageLoader.mm | 40 ++++++++++--------- .../RCTImageLoaderWithAttributionProtocol.h | 2 +- Libraries/Image/RCTImageURLLoader.h | 3 ++ .../Image/RCTImageURLLoaderWithAttribution.h | 2 +- Libraries/Image/RCTImageView.mm | 2 +- .../Image/RCTImageComponentView.mm | 2 +- .../Slider/RCTSliderComponentView.mm | 2 +- React/Fabric/RCTImageResponseDelegate.h | 2 +- React/Fabric/RCTImageResponseObserverProxy.mm | 3 +- .../renderer/imagemanager/ImageResponse.cpp | 10 ++++- .../renderer/imagemanager/ImageResponse.h | 8 +++- .../ImageResponseObserverCoordinator.cpp | 4 +- .../ImageResponseObserverCoordinator.h | 6 +++ .../platform/ios/RCTImageManager.mm | 5 ++- .../platform/ios/RCTSyncImageManager.mm | 5 ++- 15 files changed, 63 insertions(+), 33 deletions(-) diff --git a/Libraries/Image/RCTImageLoader.mm b/Libraries/Image/RCTImageLoader.mm index 300442d4402..875c6c58dd2 100644 --- a/Libraries/Image/RCTImageLoader.mm +++ b/Libraries/Image/RCTImageLoader.mm @@ -375,7 +375,9 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, attribution:{} progressBlock:progressBlock partialLoadBlock:partialLoadBlock - completionBlock:completionBlock]; + completionBlock:^(NSError *error, UIImage *image, id metadata) { + completionBlock(error, image); + }]; return ^{ [request cancel]; }; @@ -457,7 +459,7 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, attribution:(const ImageURLLoaderAttribution &)attribution progressBlock:(RCTImageLoaderProgressBlock)progressHandler partialLoadBlock:(RCTImageLoaderPartialLoadBlock)partialLoadHandler - completionBlock:(void (^)(NSError *error, id imageOrData, BOOL cacheResult, NSURLResponse *response))completionBlock + completionBlock:(void (^)(NSError *error, id imageOrData, id imageMetadata, BOOL cacheResult, NSURLResponse *response))completionBlock { { NSMutableURLRequest *mutableRequest = [request mutableCopy]; @@ -491,7 +493,7 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, __block NSLock *cancelLoadLock = [NSLock new]; NSString *requestId = [NSString stringWithFormat:@"%@-%llu",[[NSUUID UUID] UUIDString], monotonicTimeGetCurrentNanoseconds()]; - void (^completionHandler)(NSError *, id, NSURLResponse *) = ^(NSError *error, id imageOrData, NSURLResponse *response) { + void (^completionHandler)(NSError *, id, id, NSURLResponse *) = ^(NSError *error, id imageOrData, id imageMetadata, NSURLResponse *response) { [cancelLoadLock lock]; cancelLoad = nil; [cancelLoadLock unlock]; @@ -503,11 +505,11 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, // expecting it, and may do expensive post-processing in the callback dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ if (!std::atomic_load(cancelled.get())) { - completionBlock(error, imageOrData, cacheResult, response); + completionBlock(error, imageOrData, imageMetadata, cacheResult, response); } }); } else if (!std::atomic_load(cancelled.get())) { - completionBlock(error, imageOrData, cacheResult, response); + completionBlock(error, imageOrData, imageMetadata, cacheResult, response); } }; @@ -524,8 +526,8 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, attribution:attributionCopy progressHandler:progressHandler partialLoadHandler:partialLoadHandler - completionHandler:^(NSError *error, UIImage *image) { - completionHandler(error, image, nil); + completionHandler:^(NSError *error, UIImage *image, id metadata) { + completionHandler(error, image, metadata, nil); }]; } RCTImageLoaderCancellationBlock cb = [loadHandler loadImageForURL:request.URL @@ -535,7 +537,7 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, progressHandler:progressHandler partialLoadHandler:partialLoadHandler completionHandler:^(NSError *error, UIImage *image) { - completionHandler(error, image, nil); + completionHandler(error, image, nil, nil); }]; return [[RCTImageURLLoaderRequest alloc] initWithRequestId:nil imageURL:request.URL cancellationBlock:cb]; } @@ -564,8 +566,8 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, attribution:attributionCopy progressHandler:progressHandler partialLoadHandler:partialLoadHandler - completionHandler:^(NSError *error, UIImage *image) { - completionHandler(error, image, nil); + completionHandler:^(NSError *error, UIImage *image, id metadata) { + completionHandler(error, image, metadata, nil); }]; cancelLoadLocal = loaderRequest.cancellationBlock; } else { @@ -576,7 +578,7 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, progressHandler:progressHandler partialLoadHandler:partialLoadHandler completionHandler:^(NSError *error, UIImage *image) { - completionHandler(error, image, nil); + completionHandler(error, image, nil, nil); }]; } [cancelLoadLock lock]; @@ -592,12 +594,14 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, } if (image) { - completionHandler(nil, image, nil); + completionHandler(nil, image, nil, nil); } else { // Use networking module to load image dispatch_block_t cancelLoadLocal = [strongSelf _loadURLRequest:request progressBlock:progressHandler - completionBlock:completionHandler]; + completionBlock:^(NSError *error, id imageOrData, NSURLResponse *response) { + completionHandler(error, imageOrData, nil, response); + }]; [cancelLoadLock lock]; cancelLoad = cancelLoadLocal; [cancelLoadLock unlock]; @@ -746,7 +750,7 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, attribution:(const ImageURLLoaderAttribution &)attribution progressBlock:(RCTImageLoaderProgressBlock)progressBlock partialLoadBlock:(RCTImageLoaderPartialLoadBlock)partialLoadBlock - completionBlock:(RCTImageLoaderCompletionBlock)completionBlock + completionBlock:(RCTImageLoaderCompletionBlockWithMetadata)completionBlock { auto cancelled = std::make_shared>(0); __block dispatch_block_t cancelLoad = nil; @@ -766,7 +770,7 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, }; __weak RCTImageLoader *weakSelf = self; - void (^completionHandler)(NSError *, id, BOOL, NSURLResponse *) = ^(NSError *error, id imageOrData, BOOL cacheResult, NSURLResponse *response) { + void (^completionHandler)(NSError *, id, id, BOOL, NSURLResponse *) = ^(NSError *error, id imageOrData, id imageMetadata, BOOL cacheResult, NSURLResponse *response) { __typeof(self) strongSelf = weakSelf; if (std::atomic_load(cancelled.get()) || !strongSelf) { return; @@ -776,7 +780,7 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, [cancelLoadLock lock]; cancelLoad = nil; [cancelLoadLock unlock]; - completionBlock(error, imageOrData); + completionBlock(error, imageOrData, imageMetadata); return; } @@ -793,7 +797,7 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, [cancelLoadLock lock]; cancelLoad = nil; [cancelLoadLock unlock]; - completionBlock(error_, image); + completionBlock(error_, image, nil); }; dispatch_block_t cancelLoadLocal = [strongSelf decodeImageData:imageOrData size:size @@ -987,7 +991,7 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, - (RCTImageLoaderCancellationBlock)getImageSizeForURLRequest:(NSURLRequest *)imageURLRequest block:(void(^)(NSError *error, CGSize size))callback { - void (^completion)(NSError *, id, BOOL, NSURLResponse *) = ^(NSError *error, id imageOrData, BOOL cacheResult, NSURLResponse *response) { + void (^completion)(NSError *, id, id, BOOL, NSURLResponse *) = ^(NSError *error, id imageOrData, id imageMetadata, BOOL cacheResult, NSURLResponse *response) { CGSize size; if ([imageOrData isKindOfClass:[NSData class]]) { NSDictionary *meta = RCTGetImageMetadata(imageOrData); diff --git a/Libraries/Image/RCTImageLoaderWithAttributionProtocol.h b/Libraries/Image/RCTImageLoaderWithAttributionProtocol.h index 4334d074c70..bd26221531b 100644 --- a/Libraries/Image/RCTImageLoaderWithAttributionProtocol.h +++ b/Libraries/Image/RCTImageLoaderWithAttributionProtocol.h @@ -33,7 +33,7 @@ RCT_EXTERN void RCTEnableImageLoadingPerfInstrumentation(BOOL enabled); attribution:(const facebook::react::ImageURLLoaderAttribution &)attribution progressBlock:(RCTImageLoaderProgressBlock)progressBlock partialLoadBlock:(RCTImageLoaderPartialLoadBlock)partialLoadBlock - completionBlock:(RCTImageLoaderCompletionBlock)completionBlock; + completionBlock:(RCTImageLoaderCompletionBlockWithMetadata)completionBlock; #endif /** diff --git a/Libraries/Image/RCTImageURLLoader.h b/Libraries/Image/RCTImageURLLoader.h index 866ae9ada61..e49ed1118f3 100644 --- a/Libraries/Image/RCTImageURLLoader.h +++ b/Libraries/Image/RCTImageURLLoader.h @@ -15,6 +15,9 @@ NS_ASSUME_NONNULL_BEGIN typedef void (^RCTImageLoaderProgressBlock)(int64_t progress, int64_t total); typedef void (^RCTImageLoaderPartialLoadBlock)(UIImage *image); typedef void (^RCTImageLoaderCompletionBlock)(NSError * _Nullable error, UIImage * _Nullable image); +// Metadata is passed as a id in an additional parameter because there are forks of RN without this parameter, +// and the complexity of RCTImageLoader would make using protocols here difficult to typecheck. +typedef void (^RCTImageLoaderCompletionBlockWithMetadata)(NSError * _Nullable error, UIImage * _Nullable image, id _Nullable metadata); typedef dispatch_block_t RCTImageLoaderCancellationBlock; /** diff --git a/Libraries/Image/RCTImageURLLoaderWithAttribution.h b/Libraries/Image/RCTImageURLLoaderWithAttribution.h index 23d269bee33..b05ebfba8ad 100644 --- a/Libraries/Image/RCTImageURLLoaderWithAttribution.h +++ b/Libraries/Image/RCTImageURLLoaderWithAttribution.h @@ -56,7 +56,7 @@ struct ImageURLLoaderAttribution { attribution:(const facebook::react::ImageURLLoaderAttribution &)attribution progressHandler:(RCTImageLoaderProgressBlock)progressHandler partialLoadHandler:(RCTImageLoaderPartialLoadBlock)partialLoadHandler - completionHandler:(RCTImageLoaderCompletionBlock)completionHandler; + completionHandler:(RCTImageLoaderCompletionBlockWithMetadata)completionHandler; #endif /** diff --git a/Libraries/Image/RCTImageView.mm b/Libraries/Image/RCTImageView.mm index c0b23f78f5e..2b5fce147f6 100644 --- a/Libraries/Image/RCTImageView.mm +++ b/Libraries/Image/RCTImageView.mm @@ -332,7 +332,7 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithFrame:(CGRect)frame) imageScale = source.scale; } - RCTImageLoaderCompletionBlock completionHandler = ^(NSError *error, UIImage *loadedImage) { + RCTImageLoaderCompletionBlockWithMetadata completionHandler = ^(NSError *error, UIImage *loadedImage, id metadata) { [weakSelf imageLoaderLoadedImage:loadedImage error:error forImageSource:source partial:NO]; }; diff --git a/React/Fabric/Mounting/ComponentViews/Image/RCTImageComponentView.mm b/React/Fabric/Mounting/ComponentViews/Image/RCTImageComponentView.mm index 1420cb87f44..a80c9914ea7 100644 --- a/React/Fabric/Mounting/ComponentViews/Image/RCTImageComponentView.mm +++ b/React/Fabric/Mounting/ComponentViews/Image/RCTImageComponentView.mm @@ -128,7 +128,7 @@ using namespace facebook::react; #pragma mark - RCTImageResponseDelegate -- (void)didReceiveImage:(UIImage *)image fromObserver:(void const *)observer +- (void)didReceiveImage:(UIImage *)image metadata:(id)metadata fromObserver:(void const *)observer { if (!_eventEmitter || !_stateTeller.isValid()) { // Notifications are delivered asynchronously and might arrive after the view is already recycled. diff --git a/React/Fabric/Mounting/ComponentViews/Slider/RCTSliderComponentView.mm b/React/Fabric/Mounting/ComponentViews/Slider/RCTSliderComponentView.mm index fea74e18c14..e28a7eb5da9 100644 --- a/React/Fabric/Mounting/ComponentViews/Slider/RCTSliderComponentView.mm +++ b/React/Fabric/Mounting/ComponentViews/Slider/RCTSliderComponentView.mm @@ -320,7 +320,7 @@ using namespace facebook::react; #pragma mark - RCTImageResponseDelegate -- (void)didReceiveImage:(UIImage *)image fromObserver:(void const *)observer +- (void)didReceiveImage:(UIImage *)image metadata:(id)metadata fromObserver:(void const *)observer { if (observer == &_trackImageResponseObserverProxy) { self.trackImage = image; diff --git a/React/Fabric/RCTImageResponseDelegate.h b/React/Fabric/RCTImageResponseDelegate.h index eea1fabf953..29f64151804 100644 --- a/React/Fabric/RCTImageResponseDelegate.h +++ b/React/Fabric/RCTImageResponseDelegate.h @@ -11,7 +11,7 @@ NS_ASSUME_NONNULL_BEGIN @protocol RCTImageResponseDelegate -- (void)didReceiveImage:(UIImage *)image fromObserver:(void const *)observer; +- (void)didReceiveImage:(UIImage *)image metadata:(id)metadata fromObserver:(void const *)observer; - (void)didReceiveProgress:(float)progress fromObserver:(void const *)observer; - (void)didReceiveFailureFromObserver:(void const *)observer; diff --git a/React/Fabric/RCTImageResponseObserverProxy.mm b/React/Fabric/RCTImageResponseObserverProxy.mm index 4c74e2ea42f..ffb68fbdef7 100644 --- a/React/Fabric/RCTImageResponseObserverProxy.mm +++ b/React/Fabric/RCTImageResponseObserverProxy.mm @@ -23,10 +23,11 @@ RCTImageResponseObserverProxy::RCTImageResponseObserverProxy(id delegate = delegate_; auto this_ = this; RCTExecuteOnMainQueue(^{ - [delegate didReceiveImage:image fromObserver:this_]; + [delegate didReceiveImage:image metadata:metadata fromObserver:this_]; }); } diff --git a/ReactCommon/react/renderer/imagemanager/ImageResponse.cpp b/ReactCommon/react/renderer/imagemanager/ImageResponse.cpp index ab874617d7d..361e5fe7f88 100644 --- a/ReactCommon/react/renderer/imagemanager/ImageResponse.cpp +++ b/ReactCommon/react/renderer/imagemanager/ImageResponse.cpp @@ -10,12 +10,18 @@ namespace facebook { namespace react { -ImageResponse::ImageResponse(const std::shared_ptr &image) - : image_(image) {} +ImageResponse::ImageResponse( + const std::shared_ptr &image, + const std::shared_ptr &metadata) + : image_(image), metadata_(metadata) {} std::shared_ptr ImageResponse::getImage() const { return image_; } +std::shared_ptr ImageResponse::getMetadata() const { + return metadata_; +} + } // namespace react } // namespace facebook diff --git a/ReactCommon/react/renderer/imagemanager/ImageResponse.h b/ReactCommon/react/renderer/imagemanager/ImageResponse.h index 52efffd9760..23cc49f65bf 100644 --- a/ReactCommon/react/renderer/imagemanager/ImageResponse.h +++ b/ReactCommon/react/renderer/imagemanager/ImageResponse.h @@ -23,12 +23,18 @@ class ImageResponse final { Failed, }; - ImageResponse(const std::shared_ptr &image); + ImageResponse( + const std::shared_ptr &image, + const std::shared_ptr &metadata); std::shared_ptr getImage() const; + std::shared_ptr getMetadata() const; + private: std::shared_ptr image_{}; + + std::shared_ptr metadata_{}; }; } // namespace react diff --git a/ReactCommon/react/renderer/imagemanager/ImageResponseObserverCoordinator.cpp b/ReactCommon/react/renderer/imagemanager/ImageResponseObserverCoordinator.cpp index d50469818fc..3f3fd6a5ffb 100644 --- a/ReactCommon/react/renderer/imagemanager/ImageResponseObserverCoordinator.cpp +++ b/ReactCommon/react/renderer/imagemanager/ImageResponseObserverCoordinator.cpp @@ -24,8 +24,9 @@ void ImageResponseObserverCoordinator::addObserver( } case ImageResponse::Status::Completed: { auto imageData = imageData_; + auto imageMetadata = imageMetadata_; mutex_.unlock(); - observer.didReceiveImage(ImageResponse{imageData}); + observer.didReceiveImage(ImageResponse{imageData, imageMetadata}); break; } case ImageResponse::Status::Failed: { @@ -63,6 +64,7 @@ void ImageResponseObserverCoordinator::nativeImageResponseComplete( ImageResponse const &imageResponse) const { mutex_.lock(); imageData_ = imageResponse.getImage(); + imageMetadata_ = imageResponse.getMetadata(); assert(status_ == ImageResponse::Status::Loading); status_ = ImageResponse::Status::Completed; auto observers = observers_; diff --git a/ReactCommon/react/renderer/imagemanager/ImageResponseObserverCoordinator.h b/ReactCommon/react/renderer/imagemanager/ImageResponseObserverCoordinator.h index f28e7e5805c..6b1f8301588 100644 --- a/ReactCommon/react/renderer/imagemanager/ImageResponseObserverCoordinator.h +++ b/ReactCommon/react/renderer/imagemanager/ImageResponseObserverCoordinator.h @@ -73,6 +73,12 @@ class ImageResponseObserverCoordinator { */ mutable std::shared_ptr imageData_; + /* + * Cache image metadata. + * Mutable: protected by mutex_. + */ + mutable std::shared_ptr imageMetadata_; + /* * Observer and data mutex. */ diff --git a/ReactCommon/react/renderer/imagemanager/platform/ios/RCTImageManager.mm b/ReactCommon/react/renderer/imagemanager/platform/ios/RCTImageManager.mm index 9b85d9a5121..b719be1158f 100644 --- a/ReactCommon/react/renderer/imagemanager/platform/ios/RCTImageManager.mm +++ b/ReactCommon/react/renderer/imagemanager/platform/ios/RCTImageManager.mm @@ -70,14 +70,15 @@ using namespace facebook::react; std::string([moduleName UTF8String], [moduleName lengthOfBytesUsingEncoding:NSUTF8StringEncoding]); telemetry->setLoaderModuleName(moduleCString); - auto completionBlock = ^(NSError *error, UIImage *image) { + auto completionBlock = ^(NSError *error, UIImage *image, id metadata) { auto observerCoordinator = weakObserverCoordinator.lock(); if (!observerCoordinator) { return; } if (image && !error) { - observerCoordinator->nativeImageResponseComplete(ImageResponse(wrapManagedObject(image))); + auto wrappedMetadata = metadata ? wrapManagedObject(metadata) : nullptr; + observerCoordinator->nativeImageResponseComplete(ImageResponse(wrapManagedObject(image), wrappedMetadata)); } else { observerCoordinator->nativeImageResponseFailed(); } diff --git a/ReactCommon/react/renderer/imagemanager/platform/ios/RCTSyncImageManager.mm b/ReactCommon/react/renderer/imagemanager/platform/ios/RCTSyncImageManager.mm index c4e2deb8d5c..df89da51cec 100644 --- a/ReactCommon/react/renderer/imagemanager/platform/ios/RCTSyncImageManager.mm +++ b/ReactCommon/react/renderer/imagemanager/platform/ios/RCTSyncImageManager.mm @@ -50,14 +50,15 @@ using namespace facebook::react; NSURLRequest *request = NSURLRequestFromImageSource(imageSource); - auto completionBlock = ^(NSError *error, UIImage *image) { + auto completionBlock = ^(NSError *error, UIImage *image, id metadata) { auto observerCoordinator = weakObserverCoordinator.lock(); if (!observerCoordinator) { return; } if (image && !error) { - observerCoordinator->nativeImageResponseComplete(ImageResponse(wrapManagedObject(image))); + auto wrappedMetadata = metadata ? wrapManagedObject(metadata) : nullptr; + observerCoordinator->nativeImageResponseComplete(ImageResponse(wrapManagedObject(image), wrappedMetadata)); } else { observerCoordinator->nativeImageResponseFailed(); }