Skip to content

Commit ca7d792

Browse files
committed
Drop enable_shared_from_this from RCTImageResponseObserverProxy
Review feedback: the address comparison in RCTImageComponentView is enough on its own. The new proxy is allocated before the previous one is released, so consecutive subscriptions never share an address. Reuse is only possible after two resubscriptions while a callback from the first one is still queued. This leaves RCTImageResponseObserverProxy and the C++ API snapshots unchanged.
1 parent 1000f1b commit ca7d792

6 files changed

Lines changed: 12 additions & 15 deletions

File tree

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,8 @@ - (void)_setStateAndResubscribeImageResponseObserver:(const ImageShadowNode::Con
124124
if (_state) {
125125
// A new observer per subscription: callbacks of a previous request can still be queued on the
126126
// main queue (e.g. after this view was recycled and reused), and must not be applied here.
127+
// The callbacks are matched by the proxy's address. The new proxy is allocated before the
128+
// previous one is released, so two consecutive subscriptions never share an address.
127129
_imageResponseObserverProxy = std::make_shared<RCTImageResponseObserverProxy>(self);
128130
auto &observerCoordinator = _state->getData().getImageRequest().getObserverCoordinator();
129131
observerCoordinator.addObserver(_imageResponseObserverProxy);

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

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

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

14-
#include <memory>
15-
1614
NS_ASSUME_NONNULL_BEGIN
1715

1816
namespace facebook::react {
1917

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> {
18+
class RCTImageResponseObserverProxy final : public ImageResponseObserver {
2419
public:
2520
RCTImageResponseObserverProxy(id<RCTImageResponseDelegate> delegate = nil);
2621

‎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 strongThis = shared_from_this();
27+
auto this_ = this;
2828
RCTExecuteOnMainQueue(^{
29-
[delegate didReceiveImage:image metadata:metadata fromObserver:strongThis.get()];
29+
[delegate didReceiveImage:image metadata:metadata fromObserver:this_];
3030
});
3131
}
3232

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

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

‎scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6396,7 +6396,7 @@ class facebook::react::RCTHermesInstance : public facebook::react::JSRuntimeFact
63966396
public ~RCTHermesInstance() override;
63976397
}
63986398

6399-
class facebook::react::RCTImageResponseObserverProxy : public facebook::react::ImageResponseObserver, public std::enable_shared_from_this<facebook::react::RCTImageResponseObserverProxy> {
6399+
class facebook::react::RCTImageResponseObserverProxy : public facebook::react::ImageResponseObserver {
64006400
public RCTImageResponseObserverProxy(id<RCTImageResponseDelegate> delegate = nil);
64016401
public virtual void didReceiveFailure(const facebook::react::ImageLoadError& error) const override;
64026402
public virtual void didReceiveImage(const facebook::react::ImageResponse& imageResponse) const override;

‎scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6272,7 +6272,7 @@ class facebook::react::RCTHermesInstance : public facebook::react::JSRuntimeFact
62726272
public ~RCTHermesInstance() override;
62736273
}
62746274

6275-
class facebook::react::RCTImageResponseObserverProxy : public facebook::react::ImageResponseObserver, public std::enable_shared_from_this<facebook::react::RCTImageResponseObserverProxy> {
6275+
class facebook::react::RCTImageResponseObserverProxy : public facebook::react::ImageResponseObserver {
62766276
public RCTImageResponseObserverProxy(id<RCTImageResponseDelegate> delegate = nil);
62776277
public virtual void didReceiveFailure(const facebook::react::ImageLoadError& error) const override;
62786278
public virtual void didReceiveImage(const facebook::react::ImageResponse& imageResponse) const override;

‎scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6393,7 +6393,7 @@ class facebook::react::RCTHermesInstance : public facebook::react::JSRuntimeFact
63936393
public ~RCTHermesInstance() override;
63946394
}
63956395

6396-
class facebook::react::RCTImageResponseObserverProxy : public facebook::react::ImageResponseObserver, public std::enable_shared_from_this<facebook::react::RCTImageResponseObserverProxy> {
6396+
class facebook::react::RCTImageResponseObserverProxy : public facebook::react::ImageResponseObserver {
63976397
public RCTImageResponseObserverProxy(id<RCTImageResponseDelegate> delegate = nil);
63986398
public virtual void didReceiveFailure(const facebook::react::ImageLoadError& error) const override;
63996399
public virtual void didReceiveImage(const facebook::react::ImageResponse& imageResponse) const override;

0 commit comments

Comments
 (0)