Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "[Fabric] Fix image component reference cycle",
"packageName": "react-native-windows",
"email": "30809111+acoates-ms@users.noreply.github.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,9 @@ void ComponentViewRegistry::enqueueComponentViewWithComponentHandle(
ComponentViewDescriptor componentViewDescriptor) noexcept {
assert(m_registry.find(tag) != m_registry.end());

winrt::get_self<winrt::Microsoft::ReactNative::implementation::ComponentView>(componentViewDescriptor.view)
->prepareForRecycle();

m_registry.erase(tag);
}
} // namespace Microsoft::ReactNative
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ extern "C" HRESULT WINAPI WICCreateImagingFactory_Proxy(UINT SDKVersion, IWICIma

namespace winrt::Microsoft::ReactNative::Composition::implementation {

ImageComponentView::WindowsImageResponseObserver::WindowsImageResponseObserver(ImageComponentView &image) {
m_image.copy_from(&image);
}
ImageComponentView::WindowsImageResponseObserver::WindowsImageResponseObserver(
winrt::weak_ref<winrt::Microsoft::ReactNative::Composition::implementation::ImageComponentView> wkImage)
: m_wkImage(std::move(wkImage)) {}

void ImageComponentView::WindowsImageResponseObserver::didReceiveProgress(float progress, int64_t loaded, int64_t total)
const {
Expand All @@ -41,14 +41,21 @@ void ImageComponentView::WindowsImageResponseObserver::didReceiveProgress(float

void ImageComponentView::WindowsImageResponseObserver::didReceiveImage(
facebook::react::ImageResponse const &imageResponse) const {
auto imageResponseImage = std::static_pointer_cast<ImageResponseImage>(imageResponse.getImage());
m_image->m_reactContext.UIDispatcher().Post(
[imageResponseImage, image = m_image]() { image->didReceiveImage(imageResponseImage); });
if (auto imgComponentView{m_wkImage.get()}) {
auto imageResponseImage = std::static_pointer_cast<ImageResponseImage>(imageResponse.getImage());
imgComponentView->m_reactContext.UIDispatcher().Post([imageResponseImage, wkImage = m_wkImage]() {
if (auto image{wkImage.get()}) {
image->didReceiveImage(imageResponseImage);
}
});
}
}

void ImageComponentView::WindowsImageResponseObserver::didReceiveFailure(
facebook::react::ImageLoadError const &error) const {
m_image->didReceiveFailureFromObserver(error);
if (auto imgComponentView{m_wkImage.get()}) {
imgComponentView->didReceiveFailureFromObserver(error);
}
}

facebook::react::SharedViewProps ImageComponentView::defaultProps() noexcept {
Expand Down Expand Up @@ -146,14 +153,7 @@ void ImageComponentView::updateState(
auto newImageState = std::static_pointer_cast<facebook::react::ImageShadowNode::ConcreteState const>(state);

if (!m_imageResponseObserver) {
// Should ViewComponents enable_shared_from_this? then we don't need this dance to get a shared_ptr
std::shared_ptr<::Microsoft::ReactNative::FabricUIManager> fabricuiManager =
::Microsoft::ReactNative::FabricUIManager::FromProperties(m_reactContext.Properties());
auto componentViewDescriptor = fabricuiManager->GetViewRegistry().componentViewDescriptorWithTag(m_tag);

m_imageResponseObserver = std::make_shared<WindowsImageResponseObserver>(
*componentViewDescriptor.view
.as<winrt::Microsoft::ReactNative::Composition::implementation::ImageComponentView>());
m_imageResponseObserver = std::make_shared<WindowsImageResponseObserver>(get_weak());
}

setStateAndResubscribeImageResponseObserver(newImageState);
Expand Down Expand Up @@ -182,6 +182,10 @@ void ImageComponentView::setStateAndResubscribeImageResponseObserver(
}
}

void ImageComponentView::prepareForRecycle() noexcept {
setStateAndResubscribeImageResponseObserver(nullptr);
}

winrt::Microsoft::ReactNative::ImageProps ImageComponentView::ImageProps() noexcept {
// We do not currently support custom ImageComponentView's
// If we did we would need to create a AbiImageProps and possibly return them here
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ struct ImageComponentView : ImageComponentViewT<ImageComponentView, ViewComponen
override;
void updateState(facebook::react::State::Shared const &state, facebook::react::State::Shared const &oldState) noexcept
override;
void prepareForRecycle() noexcept override;
void OnRenderingDeviceLost() noexcept override;
void onThemeChanged() noexcept override;

Expand All @@ -69,13 +70,14 @@ struct ImageComponentView : ImageComponentViewT<ImageComponentView, ViewComponen
private:
struct WindowsImageResponseObserver : facebook::react::ImageResponseObserver {
public:
WindowsImageResponseObserver(ImageComponentView &image);
WindowsImageResponseObserver(
winrt::weak_ref<winrt::Microsoft::ReactNative::Composition::implementation::ImageComponentView> wkImage);
void didReceiveProgress(float progress, int64_t loaded, int64_t total) const override;
void didReceiveImage(const facebook::react::ImageResponse &imageResponse) const override;
void didReceiveFailure(const facebook::react::ImageLoadError &error) const override;

private:
winrt::com_ptr<ImageComponentView> m_image;
winrt::weak_ref<winrt::Microsoft::ReactNative::Composition::implementation::ImageComponentView> m_wkImage;
};

void ensureDrawingSurface() noexcept;
Expand Down
24 changes: 19 additions & 5 deletions vnext/Microsoft.ReactNative/Fabric/FabricUIManagerModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -200,11 +200,25 @@ void FabricUIManager::RCTPerformMountInstructions(
}

case facebook::react::ShadowViewMutation::Delete: {
auto &oldChildShadowView = mutation.oldChildShadowView;
auto &oldChildViewDescriptor = m_registry.componentViewDescriptorWithTag(oldChildShadowView.tag);
// observerCoordinator.unregisterViewComponentDescriptor(oldChildViewDescriptor, surfaceId);
m_registry.enqueueComponentViewWithComponentHandle(
oldChildShadowView.componentHandle, oldChildShadowView.tag, oldChildViewDescriptor);
// #define DETECT_COMPONENT_OUTLIVE_DELETE_MUTATION
#ifdef DETECT_COMPONENT_OUTLIVE_DELETE_MUTATION
winrt::weak_ref<winrt::Microsoft::ReactNative::ComponentView> wkView;
#endif
{
auto &oldChildShadowView = mutation.oldChildShadowView;
auto &oldChildViewDescriptor = m_registry.componentViewDescriptorWithTag(oldChildShadowView.tag);
// observerCoordinator.unregisterViewComponentDescriptor(oldChildViewDescriptor, surfaceId);
#ifdef DETECT_COMPONENT_OUTLIVE_DELETE_MUTATION
wkView = winrt::make_weak(oldChildViewDescriptor.view);
#endif
m_registry.enqueueComponentViewWithComponentHandle(
oldChildShadowView.componentHandle, oldChildShadowView.tag, oldChildViewDescriptor);
}
#ifdef DETECT_COMPONENT_OUTLIVE_DELETE_MUTATION
// After handling a delete mutation, nothing should be holding on to the view. If there is thats an indication
// of a leak, or at least something holding on to a view longer than it should
assert(!wkView.get());
#endif
break;
}

Expand Down
5 changes: 3 additions & 2 deletions vnext/Microsoft.ReactNative/ReactHost/ReactInstanceWin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ void ReactInstanceWin::LoadModules(
};

#ifdef USE_FABRIC
if (!m_options.UseWebDebugger()) {
if (Microsoft::ReactNative::IsFabricEnabled(m_reactContext->Properties())) {
registerTurboModule(
L"FabricUIManagerBinding",
winrt::Microsoft::ReactNative::MakeModuleProvider<::Microsoft::ReactNative::FabricUIManager>());
Expand Down Expand Up @@ -602,7 +602,8 @@ void ReactInstanceWin::InitializeBridgeless() noexcept {

m_jsMessageThread.Load()->runOnQueueSync([&]() {
::SetThreadDescription(GetCurrentThread(), L"React-Native JavaScript Thread");
auto timerRegistry = ::Microsoft::ReactNative::TimerRegistry::CreateTimerRegistry(m_options.Properties);
auto timerRegistry =
::Microsoft::ReactNative::TimerRegistry::CreateTimerRegistry(m_reactContext->Properties());
auto timerRegistryRaw = timerRegistry.get();

auto timerManager = std::make_shared<facebook::react::TimerManager>(std::move(timerRegistry));
Expand Down