Skip to content

Commit ac4d466

Browse files
committed
Ignore stale image callbacks after an Image view is recycled on iOS
RCTImageResponseObserverProxy dispatches image, progress and failure callbacks to the main queue. RCTImageComponentView used one proxy for its whole lifetime and only checked that it still had a state, so a callback that was already queued when the view was recycled and reused for another <Image> was applied to the new image. With a cached new source (applied synchronously on subscribe) the late old image stayed on screen. Create a new proxy per subscription and ignore callbacks from any other proxy. The queued blocks keep their proxy alive, so a freed proxy's address cannot be reused while its callbacks are still pending.
1 parent b71d466 commit ac4d466

3 files changed

Lines changed: 23 additions & 12 deletions

File tree

‎packages/react-native/React/Fabric/Mounting/ComponentViews/Image/RCTImageComponentView.mm‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,6 @@ - (instancetype)initWithFrame:(CGRect)frame
3838
_imageView.layer.minificationFilter = kCAFilterTrilinear;
3939
_imageView.layer.magnificationFilter = kCAFilterTrilinear;
4040

41-
_imageResponseObserverProxy = std::make_shared<RCTImageResponseObserverProxy>(self);
42-
4341
self.contentView = _imageView;
4442
}
4543

@@ -124,6 +122,9 @@ - (void)_setStateAndResubscribeImageResponseObserver:(const ImageShadowNode::Con
124122
_state = state;
125123

126124
if (_state) {
125+
// A new observer per subscription: callbacks of a previous request can still be queued on the
126+
// main queue (e.g. after this view was recycled and reused), and must not be applied here.
127+
_imageResponseObserverProxy = std::make_shared<RCTImageResponseObserverProxy>(self);
127128
auto &observerCoordinator = _state->getData().getImageRequest().getObserverCoordinator();
128129
observerCoordinator.addObserver(_imageResponseObserverProxy);
129130
}
@@ -140,8 +141,9 @@ - (void)prepareForRecycle
140141

141142
- (void)didReceiveImage:(UIImage *)image metadata:(id)metadata fromObserver:(const void *)observer
142143
{
143-
if (!_eventEmitter || !_state) {
144-
// Notifications are delivered asynchronously and might arrive after the view is already recycled.
144+
if (!_eventEmitter || !_state || observer != _imageResponseObserverProxy.get()) {
145+
// Notifications are delivered asynchronously and might arrive after the view is already recycled,
146+
// or after it has been reused for another image.
145147
// In the future, we should incorporate an `EventEmitter` into a separate object owned by `ImageRequest` or `State`.
146148
// See for more info: T46311063.
147149
return;
@@ -187,7 +189,7 @@ - (void)didReceiveProgress:(float)progress
187189
total:(int64_t)total
188190
fromObserver:(const void *)observer
189191
{
190-
if (!_eventEmitter) {
192+
if (!_eventEmitter || observer != _imageResponseObserverProxy.get()) {
191193
return;
192194
}
193195

@@ -196,6 +198,10 @@ - (void)didReceiveProgress:(float)progress
196198

197199
- (void)didReceiveFailure:(NSError *)error fromObserver:(const void *)observer
198200
{
201+
if (observer != _imageResponseObserverProxy.get()) {
202+
return;
203+
}
204+
199205
_imageView.image = nil;
200206

201207
if (!_eventEmitter) {

‎packages/react-native/React/Fabric/RCTImageResponseObserverProxy.h‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,16 @@
1111

1212
#include <react/renderer/imagemanager/ImageResponseObserver.h>
1313

14+
#include <memory>
15+
1416
NS_ASSUME_NONNULL_BEGIN
1517

1618
namespace facebook::react {
1719

18-
class RCTImageResponseObserverProxy final : public ImageResponseObserver {
20+
// Must be owned by a std::shared_ptr: callbacks queued to the main queue keep the
21+
// proxy alive, so its address can identify the subscription they belong to.
22+
class RCTImageResponseObserverProxy final : public ImageResponseObserver,
23+
public std::enable_shared_from_this<RCTImageResponseObserverProxy> {
1924
public:
2025
RCTImageResponseObserverProxy(id<RCTImageResponseDelegate> delegate = nil);
2126

‎packages/react-native/React/Fabric/RCTImageResponseObserverProxy.mm‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,28 +24,28 @@
2424
UIImage *image = (UIImage *)unwrapManagedObject(imageResponse.getImage());
2525
id metadata = unwrapManagedObject(imageResponse.getMetadata());
2626
id<RCTImageResponseDelegate> delegate = delegate_;
27-
auto this_ = this;
27+
auto strongThis = shared_from_this();
2828
RCTExecuteOnMainQueue(^{
29-
[delegate didReceiveImage:image metadata:metadata fromObserver:this_];
29+
[delegate didReceiveImage:image metadata:metadata fromObserver:strongThis.get()];
3030
});
3131
}
3232

3333
void RCTImageResponseObserverProxy::didReceiveProgress(float progress, int64_t loaded, int64_t total) const
3434
{
35-
auto this_ = this;
35+
auto strongThis = shared_from_this();
3636
id<RCTImageResponseDelegate> delegate = delegate_;
3737
RCTExecuteOnMainQueue(^{
38-
[delegate didReceiveProgress:progress loaded:loaded total:total fromObserver:this_];
38+
[delegate didReceiveProgress:progress loaded:loaded total:total fromObserver:strongThis.get()];
3939
});
4040
}
4141

4242
void RCTImageResponseObserverProxy::didReceiveFailure(const ImageLoadError &errorResponse) const
4343
{
44-
auto this_ = this;
44+
auto strongThis = shared_from_this();
4545
NSError *error = (NSError *)unwrapManagedObject(errorResponse.getError());
4646
id<RCTImageResponseDelegate> delegate = delegate_;
4747
RCTExecuteOnMainQueue(^{
48-
[delegate didReceiveFailure:error fromObserver:this_];
48+
[delegate didReceiveFailure:error fromObserver:strongThis.get()];
4949
});
5050
}
5151

0 commit comments

Comments
 (0)