Fabric: Replace ImageLoader promise implementation with observer model

Summary: Folly promises/futures have been replaced by an observer model which keeps track of loading state. This resolves at least one crash that I can no longer repro and simplifies the code a bit (IMO).

Reviewed By: shergin

Differential Revision: D13743393

fbshipit-source-id: 2b650841525db98b2f67add85f2097f24259c6cf

React/Fabric/Mounting/ComponentViews/Image/RCTImageComponentView.h

@@ -11,10 +11,18 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+@protocol RCTImageResponseDelegate <NSObject>
+
+- (void)didReceiveImage:(UIImage *)image fromObserver:(void*)observer;
+- (void)didReceiveProgress:(float)progress fromObserver:(void*)observer;
+- (void)didReceiveFailureFromObserver:(void*)observer;
+
+@end
+
 /**
  * UIView class for root <Image> component.
  */
-@interface RCTImageComponentView : RCTViewComponentView
+@interface RCTImageComponentView : RCTViewComponentView <RCTImageResponseDelegate>
 
 @end
 

React/Fabric/Mounting/ComponentViews/Image/RCTImageComponentView.mm

@@ -13,6 +13,7 @@
 #import <react/components/image/ImageShadowNode.h>
 #import <react/imagemanager/ImageRequest.h>
 #import <react/imagemanager/ImageResponse.h>
+#import <react/imagemanager/ImageResponseObserver.h>
 #import <react/imagemanager/RCTImagePrimitivesConversions.h>
 
 #import "RCTConversions.h"
@@ -20,9 +21,40 @@
 
 using namespace facebook::react;
 
+class ImageResponseObserverProxy: public ImageResponseObserver {
+public:
+    ImageResponseObserverProxy(void* delegate): delegate_((__bridge id<RCTImageResponseDelegate>)delegate) {}
+
+    void didReceiveImage(const ImageResponse &imageResponse) override {
+      UIImage *image = (__bridge UIImage *)imageResponse.getImage().get();
+      void *this_ = this;
+      dispatch_async(dispatch_get_main_queue(), ^{
+        [delegate_ didReceiveImage:image fromObserver:this_];
+      });
+    }
+
+    void didReceiveProgress (float p) override {
+      void *this_ = this;
+      dispatch_async(dispatch_get_main_queue(), ^{
+        [delegate_ didReceiveProgress:p fromObserver:this_];
+      });
+    }
+    void didReceiveFailure() override {
+      void *this_ = this;
+      dispatch_async(dispatch_get_main_queue(), ^{
+        [delegate_ didReceiveFailureFromObserver:this_];
+      });
+    }
+
+private:
+  id<RCTImageResponseDelegate> delegate_;
+};
+
 @implementation RCTImageComponentView {
   UIImageView *_imageView;
   SharedImageLocalData _imageLocalData;
+  std::shared_ptr<const ImageResponseObserverCoordinator> _coordinator;
+  std::unique_ptr<ImageResponseObserverProxy> _imageResponseObserverProxy;
 }
 
 - (instancetype)initWithFrame:(CGRect)frame
@@ -35,6 +67,8 @@ using namespace facebook::react;
     _imageView.clipsToBounds = YES;
 
     _imageView.contentMode = (UIViewContentMode)RCTResizeModeFromImageResizeMode(defaultProps->resizeMode);
+      
+    _imageResponseObserverProxy = std::make_unique<ImageResponseObserverProxy>((__bridge void *)self);
 
     self.contentView = _imageView;
   }
@@ -78,23 +112,42 @@ using namespace facebook::react;
 {
   _imageLocalData = std::static_pointer_cast<const ImageLocalData>(localData);
   assert(_imageLocalData);
-  auto future = _imageLocalData->getImageRequest().getResponseFuture();
-  future.via(&MainQueueExecutor::instance()).thenValue([self](ImageResponse &&imageResponse) {
-    self.image = (__bridge UIImage *)imageResponse.getImage().get();
-  });
+  self.coordinator = _imageLocalData->getImageRequest().getObserverCoordinator();
+  
+  // Loading actually starts a little before this
+  std::static_pointer_cast<const ImageEventEmitter>(_eventEmitter)->onLoadStart();
+}
+
+- (void)setCoordinator:(std::shared_ptr<const ImageResponseObserverCoordinator>)coordinator {
+  if (_coordinator) {
+    _coordinator->removeObserver(_imageResponseObserverProxy.get());
+  }
+  _coordinator = coordinator;
+  if (_coordinator != nullptr) {
+    _coordinator->addObserver(_imageResponseObserverProxy.get());
+  }
 }
 
 - (void)prepareForRecycle
 {
   [super prepareForRecycle];
+  self.coordinator = nullptr;
   _imageView.image = nil;
   _imageLocalData.reset();
 }
 
-#pragma mark - Other
-
-- (void)setImage:(UIImage *)image
+-(void)dealloc
 {
+  self.coordinator = nullptr;
+  _imageResponseObserverProxy.reset();
+}
+
+#pragma mark - RCTImageResponseDelegate
+
+- (void)didReceiveImage:(UIImage *)image fromObserver:(void*)observer
+{
+  std::static_pointer_cast<const ImageEventEmitter>(_eventEmitter)->onLoad();
+
   const auto &imageProps = *std::static_pointer_cast<const ImageProps>(_props);
 
   if (imageProps.tintColor) {
@@ -110,11 +163,22 @@ using namespace facebook::react;
                                   resizingMode:UIImageResizingModeStretch];
   }
 
-  _imageView.image = image;
-
+  self->_imageView.image = image;
+  
   // Apply trilinear filtering to smooth out mis-sized images.
-  _imageView.layer.minificationFilter = kCAFilterTrilinear;
-  _imageView.layer.magnificationFilter = kCAFilterTrilinear;
+  self->_imageView.layer.minificationFilter = kCAFilterTrilinear;
+  self->_imageView.layer.magnificationFilter = kCAFilterTrilinear;
+
+  std::static_pointer_cast<const ImageEventEmitter>(self->_eventEmitter)->onLoadEnd();
 }
 
+- (void)didReceiveProgress:(float)progress fromObserver:(void*)observer {
+  std::static_pointer_cast<const ImageEventEmitter>(_eventEmitter)->onProgress(progress);
+}
+
+- (void)didReceiveFailureFromObserver:(void*)observer {
+  std::static_pointer_cast<const ImageEventEmitter>(_eventEmitter)->onError();
+}
+
+
 @end

React/Fabric/RCTScheduler.mm

@@ -47,11 +47,11 @@ private:
   std::shared_ptr<SchedulerDelegateProxy> _delegateProxy;
 }
 
-- (instancetype)initWithContextContainer:(std::shared_ptr<void>)contextContatiner
+- (instancetype)initWithContextContainer:(std::shared_ptr<void>)contextContainer
 {
   if (self = [super init]) {
     _delegateProxy = std::make_shared<SchedulerDelegateProxy>((__bridge void *)self);
-    _scheduler = std::make_shared<Scheduler>(std::static_pointer_cast<ContextContainer>(contextContatiner), getDefaultComponentRegistryFactory());
+    _scheduler = std::make_shared<Scheduler>(std::static_pointer_cast<ContextContainer>(contextContainer), getDefaultComponentRegistryFactory());
     _scheduler->setDelegate(_delegateProxy.get());
   }
 

ReactCommon/fabric/components/image/ImageEventEmitter.cpp

@@ -22,8 +22,12 @@ void ImageEventEmitter::onLoadEnd() const {
   dispatchEvent("loadEnd");
 }
 
-void ImageEventEmitter::onProgress() const {
-  dispatchEvent("progress");
+void ImageEventEmitter::onProgress(double progress) const {
+  dispatchEvent("progress", [=](jsi::Runtime &runtime) {
+    auto payload = jsi::Object(runtime);
+    payload.setProperty(runtime, "progress", progress);
+    return payload;
+  });
 }
 
 void ImageEventEmitter::onError() const {

ReactCommon/fabric/components/image/ImageEventEmitter.h

@@ -18,7 +18,7 @@ class ImageEventEmitter : public ViewEventEmitter {
   void onLoadStart() const;
   void onLoad() const;
   void onLoadEnd() const;
-  void onProgress() const;
+  void onProgress(double) const;
   void onError() const;
   void onPartialLoad() const;
 };

ReactCommon/fabric/imagemanager/ImageRequest.h

@@ -7,11 +7,9 @@
 
 #pragma once
 
-#include <mutex>
-
-#include <folly/futures/Future.h>
-#include <folly/futures/FutureSplitter.h>
 #include <react/imagemanager/ImageResponse.h>
+#include <react/imagemanager/ImageResponseObserver.h>
+#include <react/imagemanager/ImageResponseObserverCoordinator.h>
 #include <react/imagemanager/primitives.h>
 
 namespace facebook {
@@ -22,7 +20,6 @@ namespace react {
  * The separate object must be constructed for every single separate
  * image request. The object cannot be copied because it would make managing of
  * event listeners hard and inefficient; the object can be moved though.
- * To subscribe for notifications use `getResponseFuture()` method.
  * Destroy to cancel the underlying request.
  */
 class ImageRequest final {
@@ -33,15 +30,10 @@ class ImageRequest final {
    */
   class ImageNoLongerNeededException;
 
-  ImageRequest();
-
   /*
-   * `ImageRequest` is constructed with `ImageSource` and
-   * `ImageResponse` future which must be moved in inside the object.
+   * The default constructor
    */
-  ImageRequest(
-      const ImageSource &imageSource,
-      folly::Future<ImageResponse> &&responseFuture);
+  ImageRequest(const ImageSource &imageSource);
 
   /*
    * The move constructor.
@@ -51,32 +43,36 @@ class ImageRequest final {
   /*
    * `ImageRequest` does not support copying by design.
    */
-  ImageRequest(const ImageRequest &) = delete;
+  ImageRequest(const ImageRequest &other) = delete;
 
   ~ImageRequest();
 
-  /*
-   * Creates and returns a *new* future object with promised `ImageResponse`
-   * result. Multiple consumers can call this method many times to create
-   * their own subscriptions to promised value.
+  /**
+   * Set cancelation function.
    */
-  folly::Future<ImageResponse> getResponseFuture() const;
+  void setCancelationFunction(std::function<void(void)> cancelationFunction);
+
+  /*
+   * Get observer coordinator.
+   */
+  std::shared_ptr<const ImageResponseObserverCoordinator>
+  getObserverCoordinator() const;
 
  private:
-  /*
-   * Mutext to protect an access to the future.
-   */
-  mutable std::mutex mutex_;
-
   /*
    * Image source assosiated with the request.
    */
   ImageSource imageSource_;
 
   /*
-   * Future splitter powers factory-like `getResponseFuture()` method.
+   * Event coordinator associated with the reqest.
    */
-  mutable folly::FutureSplitter<ImageResponse> responseFutureSplitter_;
+  std::shared_ptr<const ImageResponseObserverCoordinator> coordinator_{};
+
+  /*
+   * Function we can call to cancel image request (see destructor).
+   */
+  std::function<void(void)> cancelRequest_;
 
   /*
    * Indicates that the object was moved and hence cannot be used anymore.

ReactCommon/fabric/imagemanager/ImageResponse.h

@@ -5,6 +5,8 @@
  * LICENSE file in the root directory of this source tree.
  */
 
+#pragma once
+
 #include <memory>
 
 namespace facebook {
@@ -15,12 +17,18 @@ namespace react {
  */
 class ImageResponse final {
  public:
+  enum class Status {
+    Loading,
+    Completed,
+    Failed,
+  };
+
   ImageResponse(const std::shared_ptr<void> &image);
 
   std::shared_ptr<void> getImage() const;
 
  private:
-  std::shared_ptr<void> image_;
+  std::shared_ptr<void> image_{};
 };
 
 } // namespace react

ReactCommon/fabric/imagemanager/ImageResponseObserver.h

@@ -0,0 +1,27 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#pragma once
+
+#include <react/imagemanager/ImageResponse.h>
+
+namespace facebook {
+namespace react {
+
+/*
+ * Represents any observer of ImageResponse progression, completion, or failure.
+ */
+class ImageResponseObserver {
+ public:
+  virtual void didReceiveProgress(float) = 0;
+  virtual void didReceiveImage(const ImageResponse &imageResponse) = 0;
+  virtual void didReceiveFailure() = 0;
+  virtual ~ImageResponseObserver() noexcept = default;
+};
+
+} // namespace react
+} // namespace facebook

ReactCommon/fabric/imagemanager/ImageResponseObserverCoordinator.cpp

@@ -0,0 +1,103 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#include "ImageResponseObserverCoordinator.h"
+
+#include <algorithm>
+
+namespace facebook {
+namespace react {
+
+ImageResponseObserverCoordinator::ImageResponseObserverCoordinator() {
+  status_ = ImageResponse::Status::Loading;
+}
+
+ImageResponseObserverCoordinator::~ImageResponseObserverCoordinator() {}
+
+void ImageResponseObserverCoordinator::addObserver(
+    ImageResponseObserver *observer) const {
+  ImageResponse::Status status = [this] {
+    std::shared_lock<folly::SharedMutex> read(mutex_);
+    return status_;
+  }();
+
+  if (status == ImageResponse::Status::Loading) {
+    std::unique_lock<folly::SharedMutex> write(mutex_);
+    observers_.push_back(observer);
+  } else if (status == ImageResponse::Status::Completed) {
+    ImageResponse imageResponseCopy = [this] {
+      std::unique_lock<folly::SharedMutex> read(mutex_);
+      return ImageResponse(imageData_);
+    }();
+    observer->didReceiveImage(imageResponseCopy);
+  } else {
+    observer->didReceiveFailure();
+  }
+}
+
+void ImageResponseObserverCoordinator::removeObserver(
+    ImageResponseObserver *observer) const {
+  std::unique_lock<folly::SharedMutex> write(mutex_);
+
+  auto position = std::find(observers_.begin(), observers_.end(), observer);
+  if (position != observers_.end()) {
+    observers_.erase(position, observers_.end());
+  }
+}
+
+void ImageResponseObserverCoordinator::nativeImageResponseProgress(
+    float progress) const {
+  std::vector<ImageResponseObserver *> observersCopy = [this] {
+    std::shared_lock<folly::SharedMutex> read(mutex_);
+    return observers_;
+  }();
+
+  for (auto observer : observersCopy) {
+    observer->didReceiveProgress(progress);
+  }
+}
+
+void ImageResponseObserverCoordinator::nativeImageResponseComplete(
+    const ImageResponse &imageResponse) const {
+  {
+    std::unique_lock<folly::SharedMutex> write(mutex_);
+    imageData_ = imageResponse.getImage();
+    status_ = ImageResponse::Status::Completed;
+  }
+
+  std::vector<ImageResponseObserver *> observersCopy = [this] {
+    std::shared_lock<folly::SharedMutex> read(mutex_);
+    return observers_;
+  }();
+
+  for (auto observer : observersCopy) {
+    ImageResponse imageResponseCopy = [this] {
+      std::unique_lock<folly::SharedMutex> read(mutex_);
+      return ImageResponse(imageData_);
+    }();
+    observer->didReceiveImage(imageResponseCopy);
+  }
+}
+
+void ImageResponseObserverCoordinator::nativeImageResponseFailed() const {
+  {
+    std::unique_lock<folly::SharedMutex> write(mutex_);
+    status_ = ImageResponse::Status::Failed;
+  }
+
+  std::vector<ImageResponseObserver *> observersCopy = [this] {
+    std::shared_lock<folly::SharedMutex> read(mutex_);
+    return observers_;
+  }();
+
+  for (auto observer : observersCopy) {
+    observer->didReceiveFailure();
+  }
+}
+
+} // namespace react
+} // namespace facebook

ReactCommon/fabric/imagemanager/ImageResponseObserverCoordinator.h

@@ -0,0 +1,91 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#pragma once
+
+#include <react/imagemanager/ImageResponse.h>
+#include <react/imagemanager/ImageResponseObserver.h>
+
+#include <folly/SharedMutex.h>
+#include <shared_mutex>
+#include <vector>
+
+namespace facebook {
+namespace react {
+
+/*
+ * The ImageResponseObserverCoordinator receives events and completed image
+ * data from native image loaders and sends events to any observers attached
+ * to the coordinator. The Coordinator also keeps track of response status
+ * and caches completed images.
+ */
+class ImageResponseObserverCoordinator {
+ public:
+  /*
+   * Default constructor.
+   */
+  ImageResponseObserverCoordinator();
+
+  /*
+   * Default destructor.
+   */
+  ~ImageResponseObserverCoordinator();
+
+  /*
+   * Interested parties may observe the image response.
+   */
+  void addObserver(ImageResponseObserver *observer) const;
+
+  /*
+   * Interested parties may stop observing the image response.
+   */
+  void removeObserver(ImageResponseObserver *observer) const;
+
+  /*
+   * Platform-specific image loader will call this method with progress updates.
+   */
+  void nativeImageResponseProgress(float) const;
+
+  /*
+   * Platform-specific image loader will call this method with a completed image
+   * response.
+   */
+  void nativeImageResponseComplete(const ImageResponse &imageResponse) const;
+
+  /*
+   * Platform-specific image loader will call this method in case of any
+   * failures.
+   */
+  void nativeImageResponseFailed() const;
+
+ private:
+  /*
+   * List of observers.
+   * Mutable: protected by mutex_.
+   */
+  mutable std::vector<ImageResponseObserver *> observers_;
+
+  /*
+   * Current status of image loading.
+   * Mutable: protected by mutex_.
+   */
+  mutable ImageResponse::Status status_;
+
+  /*
+   * Cache image data.
+   * Mutable: protected by mutex_.
+   */
+  mutable std::shared_ptr<void> imageData_{};
+
+  /*
+   * Observer and data mutex.
+   */
+  mutable folly::SharedMutex mutex_;
+};
+
+} // namespace react
+} // namespace facebook

ReactCommon/fabric/imagemanager/platform/android/ImageManager.cpp

@@ -22,7 +22,7 @@ ImageManager::~ImageManager() {
 
 ImageRequest ImageManager::requestImage(const ImageSource &imageSource) const {
   // Not implemented.
-  return {};
+  return ImageRequest(imageSource);
 }
 
 } // namespace react

ReactCommon/fabric/imagemanager/platform/android/ImageRequest.cpp

@@ -10,17 +10,14 @@
 namespace facebook {
 namespace react {
 
-ImageRequest::ImageRequest() {}
-
-ImageRequest::ImageRequest(
-    const ImageSource &imageSource,
-    folly::Future<ImageResponse> &&responseFuture) {
+ImageRequest::ImageRequest(const ImageSource &imageSource)
+    : imageSource_(imageSource) {
   // Not implemented.
 }
 
 ImageRequest::ImageRequest(ImageRequest &&other) noexcept
     : imageSource_(std::move(other.imageSource_)),
-      responseFutureSplitter_(std::move(other.responseFutureSplitter_)) {
+      coordinator_(std::move(other.coordinator_)) {
   // Not implemented.
 }
 
@@ -28,8 +25,9 @@ ImageRequest::~ImageRequest() {
   // Not implemented.
 }
 
-folly::Future<ImageResponse> ImageRequest::getResponseFuture() const {
-  // Not implemented.
+std::shared_ptr<const ImageResponseObserverCoordinator>
+ImageRequest::getObserverCoordinator() const {
+  // Not implemented
   abort();
 }
 

ReactCommon/fabric/imagemanager/platform/ios/ImageRequest.cpp

@@ -16,34 +16,33 @@ class ImageRequest::ImageNoLongerNeededException : public std::logic_error {
       : std::logic_error("Image no longer needed.") {}
 };
 
-ImageRequest::ImageRequest(
-    const ImageSource &imageSource,
-    folly::Future<ImageResponse> &&responseFuture)
-    : imageSource_(imageSource),
-      responseFutureSplitter_(folly::splitFuture(std::move(responseFuture))) {}
+ImageRequest::ImageRequest(const ImageSource &imageSource)
+    : imageSource_(imageSource) {
+  coordinator_ = std::make_shared<ImageResponseObserverCoordinator>();
+}
 
 ImageRequest::ImageRequest(ImageRequest &&other) noexcept
     : imageSource_(std::move(other.imageSource_)),
-      responseFutureSplitter_(std::move(other.responseFutureSplitter_)) {
+      coordinator_(std::move(other.coordinator_)) {
   other.moved_ = true;
-};
+  other.coordinator_ = nullptr;
+  other.cancelRequest_ = nullptr;
+}
 
 ImageRequest::~ImageRequest() {
-  if (!moved_) {
-    auto future = responseFutureSplitter_.getFuture();
-    if (!future.isReady()) {
-      future.raise(ImageNoLongerNeededException());
-    }
+  if (cancelRequest_) {
+    cancelRequest_();
   }
 }
 
-folly::Future<ImageResponse> ImageRequest::getResponseFuture() const {
-  if (moved_) {
-    abort();
-  }
+void ImageRequest::setCancelationFunction(
+    std::function<void(void)> cancelationFunction) {
+  cancelRequest_ = cancelationFunction;
+}
 
-  std::lock_guard<std::mutex> lock(mutex_);
-  return responseFutureSplitter_.getFuture();
+std::shared_ptr<const ImageResponseObserverCoordinator>
+ImageRequest::getObserverCoordinator() const {
+  return coordinator_;
 }
 
 } // namespace react

ReactCommon/fabric/imagemanager/platform/ios/RCTImageManager.mm

@@ -7,10 +7,9 @@
 
 #import "RCTImageManager.h"
 
-#import <folly/futures/Future.h>
-#import <folly/futures/Promise.h>
-
 #import <React/RCTImageLoader.h>
+#import <react/imagemanager/ImageResponse.h>
+#import <react/imagemanager/ImageResponseObserver.h>
 
 #import "RCTImagePrimitivesConversions.h"
 
@@ -29,23 +28,28 @@ using namespace facebook::react;
 }
 
 - (ImageRequest)requestImage:(const ImageSource &)imageSource {
-  __block auto promise = folly::Promise<ImageResponse>();
+  auto imageRequest = ImageRequest(imageSource);
+
+  auto observerCoordinator = imageRequest.getObserverCoordinator();
 
   NSURLRequest *request = NSURLRequestFromImageSource(imageSource);
 
   auto completionBlock = ^(NSError *error, UIImage *image) {
-    auto imageResponse = ImageResponse(
-        std::shared_ptr<void>((__bridge_retained void *)image, CFRelease));
-    promise.setValue(std::move(imageResponse));
-  };
-
-  auto interruptBlock = ^(const folly::exception_wrapper &exceptionWrapper) {
-    if (!promise.isFulfilled()) {
-      promise.setException(exceptionWrapper);
+    if (image && !error) {
+      auto imageResponse = ImageResponse(
+          std::shared_ptr<void>((__bridge_retained void *)image, CFRelease));
+      observerCoordinator->nativeImageResponseComplete(
+          std::move(imageResponse));
+    } else {
+      observerCoordinator->nativeImageResponseFailed();
     }
   };
 
-  RCTImageLoaderCancellationBlock cancellationBlock =
+  auto progressBlock = ^(int64_t progress, int64_t total) {
+    observerCoordinator->nativeImageResponseProgress(progress / (float)total);
+  };
+
+  RCTImageLoaderCancellationBlock cancelationBlock =
       [_imageLoader loadImageWithURLRequest:request
                                        size:CGSizeMake(
                                                 imageSource.size.width,
@@ -53,18 +57,17 @@ using namespace facebook::react;
                                       scale:imageSource.scale
                                     clipped:YES
                                  resizeMode:RCTResizeModeStretch
-                              progressBlock:nil
+                              progressBlock:progressBlock
                            partialLoadBlock:nil
                             completionBlock:completionBlock];
 
-  promise.setInterruptHandler(
-      [cancellationBlock,
-       interruptBlock](const folly::exception_wrapper &exceptionWrapper) {
-        cancellationBlock();
-        interruptBlock(exceptionWrapper);
-      });
+  std::function<void(void)> cancelationFunction = [cancelationBlock](void) {
+    cancelationBlock();
+  };
 
-  return ImageRequest(imageSource, promise.getFuture());
+  imageRequest.setCancelationFunction(cancelationFunction);
+
+  return imageRequest;
 }
 
 @end