diff --git a/React/Fabric/Mounting/ComponentViews/Image/RCTImageComponentView.mm b/React/Fabric/Mounting/ComponentViews/Image/RCTImageComponentView.mm index bbede51c049..a350d81d730 100644 --- a/React/Fabric/Mounting/ComponentViews/Image/RCTImageComponentView.mm +++ b/React/Fabric/Mounting/ComponentViews/Image/RCTImageComponentView.mm @@ -20,14 +20,14 @@ @implementation RCTImageComponentView { UIImageView *_imageView; SharedImageLocalData _imageLocalData; - const ImageResponseObserverCoordinator *_coordinator; + ImageResponseObserverCoordinator const *_coordinator; std::unique_ptr _imageResponseObserverProxy; } - (instancetype)initWithFrame:(CGRect)frame { if (self = [super initWithFrame:frame]) { - static const auto defaultProps = std::make_shared(); + static auto const defaultProps = std::make_shared(); _props = defaultProps; _imageView = [[UIImageView alloc] initWithFrame:self.bounds]; @@ -52,8 +52,8 @@ - (void)updateProps:(Props::Shared const &)props oldProps:(Props::Shared const &)oldProps { - const auto &oldImageProps = *std::static_pointer_cast(_props); - const auto &newImageProps = *std::static_pointer_cast(props); + auto const &oldImageProps = *std::static_pointer_cast(_props); + auto const &newImageProps = *std::static_pointer_cast(props); // `resizeMode` if (oldImageProps.resizeMode != newImageProps.resizeMode) { @@ -76,7 +76,7 @@ - (void)updateLocalData:(SharedLocalData)localData oldLocalData:(SharedLocalData)oldLocalData { - auto imageLocalData = std::static_pointer_cast(localData); + auto imageLocalData = std::static_pointer_cast(localData); // This call (setting `coordinator`) must be unconditional (at the same block as setting `LocalData`) // because the setter stores a raw pointer to object that `LocalData` owns. @@ -96,18 +96,18 @@ if (!havePreviousData || _imageLocalData->getImageSource() != previousData->getImageSource()) { // Loading actually starts a little before this, but this is the first time we know // the image is loading and can fire an event from this component - std::static_pointer_cast(_eventEmitter)->onLoadStart(); + std::static_pointer_cast(_eventEmitter)->onLoadStart(); } } -- (void)setCoordinator:(const ImageResponseObserverCoordinator *)coordinator +- (void)setCoordinator:(ImageResponseObserverCoordinator const *)coordinator { if (_coordinator) { - _coordinator->removeObserver(_imageResponseObserverProxy.get()); + _coordinator->removeObserver(*_imageResponseObserverProxy); } _coordinator = coordinator; if (_coordinator != nullptr) { - _coordinator->addObserver(_imageResponseObserverProxy.get()); + _coordinator->addObserver(*_imageResponseObserverProxy); } } @@ -127,7 +127,7 @@ #pragma mark - RCTImageResponseDelegate -- (void)didReceiveImage:(UIImage *)image fromObserver:(void *)observer +- (void)didReceiveImage:(UIImage *)image fromObserver:(void const *)observer { if (!_eventEmitter) { // Notifications are delivered asynchronously and might arrive after the view is already recycled. @@ -136,9 +136,9 @@ return; } - std::static_pointer_cast(_eventEmitter)->onLoad(); + std::static_pointer_cast(_eventEmitter)->onLoad(); - const auto &imageProps = *std::static_pointer_cast(_props); + const auto &imageProps = *std::static_pointer_cast(_props); if (imageProps.tintColor) { image = [image imageWithRenderingMode:UIImageRenderingModeAlwaysTemplate]; @@ -159,19 +159,19 @@ self->_imageView.layer.minificationFilter = kCAFilterTrilinear; self->_imageView.layer.magnificationFilter = kCAFilterTrilinear; - std::static_pointer_cast(self->_eventEmitter)->onLoadEnd(); + std::static_pointer_cast(self->_eventEmitter)->onLoadEnd(); } -- (void)didReceiveProgress:(float)progress fromObserver:(void *)observer +- (void)didReceiveProgress:(float)progress fromObserver:(void const *)observer { if (!_eventEmitter) { return; } - std::static_pointer_cast(_eventEmitter)->onProgress(progress); + std::static_pointer_cast(_eventEmitter)->onProgress(progress); } -- (void)didReceiveFailureFromObserver:(void *)observer +- (void)didReceiveFailureFromObserver:(void const *)observer { _imageView.image = nil; @@ -179,7 +179,7 @@ return; } - std::static_pointer_cast(_eventEmitter)->onError(); + std::static_pointer_cast(_eventEmitter)->onError(); } @end diff --git a/React/Fabric/Mounting/ComponentViews/Slider/RCTSliderComponentView.mm b/React/Fabric/Mounting/ComponentViews/Slider/RCTSliderComponentView.mm index cb0bb3757b6..72a6c2ebfe4 100644 --- a/React/Fabric/Mounting/ComponentViews/Slider/RCTSliderComponentView.mm +++ b/React/Fabric/Mounting/ComponentViews/Slider/RCTSliderComponentView.mm @@ -177,44 +177,44 @@ using namespace facebook::react; - (void)setTrackImageCoordinator:(const ImageResponseObserverCoordinator *)coordinator { if (_trackImageCoordinator) { - _trackImageCoordinator->removeObserver(_trackImageResponseObserverProxy.get()); + _trackImageCoordinator->removeObserver(*_trackImageResponseObserverProxy); } _trackImageCoordinator = coordinator; if (_trackImageCoordinator) { - _trackImageCoordinator->addObserver(_trackImageResponseObserverProxy.get()); + _trackImageCoordinator->addObserver(*_trackImageResponseObserverProxy); } } - (void)setMinimumTrackImageCoordinator:(const ImageResponseObserverCoordinator *)coordinator { if (_minimumTrackImageCoordinator) { - _minimumTrackImageCoordinator->removeObserver(_minimumTrackImageResponseObserverProxy.get()); + _minimumTrackImageCoordinator->removeObserver(*_minimumTrackImageResponseObserverProxy); } _minimumTrackImageCoordinator = coordinator; if (_minimumTrackImageCoordinator) { - _minimumTrackImageCoordinator->addObserver(_minimumTrackImageResponseObserverProxy.get()); + _minimumTrackImageCoordinator->addObserver(*_minimumTrackImageResponseObserverProxy); } } - (void)setMaximumTrackImageCoordinator:(const ImageResponseObserverCoordinator *)coordinator { if (_maximumTrackImageCoordinator) { - _maximumTrackImageCoordinator->removeObserver(_maximumTrackImageResponseObserverProxy.get()); + _maximumTrackImageCoordinator->removeObserver(*_maximumTrackImageResponseObserverProxy); } _maximumTrackImageCoordinator = coordinator; if (_maximumTrackImageCoordinator) { - _maximumTrackImageCoordinator->addObserver(_maximumTrackImageResponseObserverProxy.get()); + _maximumTrackImageCoordinator->addObserver(*_maximumTrackImageResponseObserverProxy); } } - (void)setThumbImageCoordinator:(const ImageResponseObserverCoordinator *)coordinator { if (_thumbImageCoordinator) { - _thumbImageCoordinator->removeObserver(_thumbImageResponseObserverProxy.get()); + _thumbImageCoordinator->removeObserver(*_thumbImageResponseObserverProxy); } _thumbImageCoordinator = coordinator; if (_thumbImageCoordinator) { - _thumbImageCoordinator->addObserver(_thumbImageResponseObserverProxy.get()); + _thumbImageCoordinator->addObserver(*_thumbImageResponseObserverProxy); } } diff --git a/React/Fabric/RCTImageResponseDelegate.h b/React/Fabric/RCTImageResponseDelegate.h index db313305f44..126ccee4a6c 100644 --- a/React/Fabric/RCTImageResponseDelegate.h +++ b/React/Fabric/RCTImageResponseDelegate.h @@ -11,9 +11,9 @@ NS_ASSUME_NONNULL_BEGIN @protocol RCTImageResponseDelegate -- (void)didReceiveImage:(UIImage *)image fromObserver:(void*)observer; -- (void)didReceiveProgress:(float)progress fromObserver:(void*)observer; -- (void)didReceiveFailureFromObserver:(void*)observer; +- (void)didReceiveImage:(UIImage *)image fromObserver:(void const *)observer; +- (void)didReceiveProgress:(float)progress fromObserver:(void const *)observer; +- (void)didReceiveFailureFromObserver:(void const *)observer; @end diff --git a/React/Fabric/RCTImageResponseObserverProxy.h b/React/Fabric/RCTImageResponseObserverProxy.h index a595dc53750..a8f50b06400 100644 --- a/React/Fabric/RCTImageResponseObserverProxy.h +++ b/React/Fabric/RCTImageResponseObserverProxy.h @@ -15,16 +15,19 @@ NS_ASSUME_NONNULL_BEGIN namespace facebook { namespace react { -class RCTImageResponseObserverProxy : public ImageResponseObserver { + +class RCTImageResponseObserverProxy final : public ImageResponseObserver { public: RCTImageResponseObserverProxy(void *delegate); - void didReceiveImage(const ImageResponse &imageResponse) override; - void didReceiveProgress(float p) override; - void didReceiveFailure() override; + + void didReceiveImage(ImageResponse const &imageResponse) const override; + void didReceiveProgress(float progress) const override; + void didReceiveFailure() const override; private: __weak id delegate_; }; + } // namespace react } // namespace facebook diff --git a/React/Fabric/RCTImageResponseObserverProxy.mm b/React/Fabric/RCTImageResponseObserverProxy.mm index 9a532d354a0..ce4e2253cc2 100644 --- a/React/Fabric/RCTImageResponseObserverProxy.mm +++ b/React/Fabric/RCTImageResponseObserverProxy.mm @@ -18,26 +18,26 @@ RCTImageResponseObserverProxy::RCTImageResponseObserverProxy(void *delegate) { } -void RCTImageResponseObserverProxy::didReceiveImage(const ImageResponse &imageResponse) +void RCTImageResponseObserverProxy::didReceiveImage(ImageResponse const &imageResponse) const { UIImage *image = (__bridge UIImage *)imageResponse.getImage().get(); - void *this_ = this; + auto this_ = this; dispatch_async(dispatch_get_main_queue(), ^{ [delegate_ didReceiveImage:image fromObserver:this_]; }); } -void RCTImageResponseObserverProxy::didReceiveProgress(float p) +void RCTImageResponseObserverProxy::didReceiveProgress(float progress) const { - void *this_ = this; + auto this_ = this; dispatch_async(dispatch_get_main_queue(), ^{ - [delegate_ didReceiveProgress:p fromObserver:this_]; + [delegate_ didReceiveProgress:progress fromObserver:this_]; }); } -void RCTImageResponseObserverProxy::didReceiveFailure() +void RCTImageResponseObserverProxy::didReceiveFailure() const { - void *this_ = this; + auto this_ = this; dispatch_async(dispatch_get_main_queue(), ^{ [delegate_ didReceiveFailureFromObserver:this_]; }); diff --git a/ReactCommon/fabric/imagemanager/ImageResponseObserver.h b/ReactCommon/fabric/imagemanager/ImageResponseObserver.h index 55fe4f64f50..104cf667515 100644 --- a/ReactCommon/fabric/imagemanager/ImageResponseObserver.h +++ b/ReactCommon/fabric/imagemanager/ImageResponseObserver.h @@ -14,13 +14,15 @@ namespace react { /* * Represents any observer of ImageResponse progression, completion, or failure. + * All methods must be thread-safe. */ class ImageResponseObserver { public: - virtual void didReceiveProgress(float) = 0; - virtual void didReceiveImage(const ImageResponse &imageResponse) = 0; - virtual void didReceiveFailure() = 0; virtual ~ImageResponseObserver() noexcept = default; + + virtual void didReceiveProgress(float progress) const = 0; + virtual void didReceiveImage(ImageResponse const &imageResponse) const = 0; + virtual void didReceiveFailure() const = 0; }; } // namespace react diff --git a/ReactCommon/fabric/imagemanager/ImageResponseObserverCoordinator.cpp b/ReactCommon/fabric/imagemanager/ImageResponseObserverCoordinator.cpp index df90128c117..aa976221f5e 100644 --- a/ReactCommon/fabric/imagemanager/ImageResponseObserverCoordinator.cpp +++ b/ReactCommon/fabric/imagemanager/ImageResponseObserverCoordinator.cpp @@ -14,34 +14,34 @@ namespace facebook { namespace react { void ImageResponseObserverCoordinator::addObserver( - ImageResponseObserver *observer) const { + ImageResponseObserver const &observer) const { mutex_.lock(); switch (status_) { case ImageResponse::Status::Loading: { - observers_.push_back(observer); + observers_.push_back(&observer); mutex_.unlock(); break; } case ImageResponse::Status::Completed: { auto imageData = imageData_; mutex_.unlock(); - observer->didReceiveImage(ImageResponse{imageData}); + observer.didReceiveImage(ImageResponse{imageData}); break; } case ImageResponse::Status::Failed: { mutex_.unlock(); - observer->didReceiveFailure(); + observer.didReceiveFailure(); break; } } } void ImageResponseObserverCoordinator::removeObserver( - ImageResponseObserver *observer) const { + ImageResponseObserver const &observer) const { std::lock_guard lock(mutex_); // We remove only one element to maintain a balance between add/remove calls. - auto position = std::find(observers_.begin(), observers_.end(), observer); + auto position = std::find(observers_.begin(), observers_.end(), &observer); if (position != observers_.end()) { observers_.erase(position, observers_.end()); } @@ -60,7 +60,7 @@ void ImageResponseObserverCoordinator::nativeImageResponseProgress( } void ImageResponseObserverCoordinator::nativeImageResponseComplete( - const ImageResponse &imageResponse) const { + ImageResponse const &imageResponse) const { mutex_.lock(); imageData_ = imageResponse.getImage(); assert(status_ == ImageResponse::Status::Loading); diff --git a/ReactCommon/fabric/imagemanager/ImageResponseObserverCoordinator.h b/ReactCommon/fabric/imagemanager/ImageResponseObserverCoordinator.h index 3cb046b32af..66542642763 100644 --- a/ReactCommon/fabric/imagemanager/ImageResponseObserverCoordinator.h +++ b/ReactCommon/fabric/imagemanager/ImageResponseObserverCoordinator.h @@ -30,23 +30,23 @@ class ImageResponseObserverCoordinator { * If the current image request status is not equal to `Loading`, the observer * will be called immediately. */ - void addObserver(ImageResponseObserver *observer) const; + void addObserver(ImageResponseObserver const &observer) const; /* * Interested parties may stop observing the image response. */ - void removeObserver(ImageResponseObserver *observer) const; + void removeObserver(ImageResponseObserver const &observer) const; /* * Platform-specific image loader will call this method with progress updates. */ - void nativeImageResponseProgress(float) const; + void nativeImageResponseProgress(float progress) const; /* * Platform-specific image loader will call this method with a completed image * response. */ - void nativeImageResponseComplete(const ImageResponse &imageResponse) const; + void nativeImageResponseComplete(ImageResponse const &imageResponse) const; /* * Platform-specific image loader will call this method in case of any @@ -59,7 +59,7 @@ class ImageResponseObserverCoordinator { * List of observers. * Mutable: protected by mutex_. */ - mutable better::small_vector observers_; + mutable better::small_vector observers_; /* * Current status of image loading.