Skip to content

Commit b47b39f

Browse files
acoates-msacoates-ms
committed
[Fabric] Fix ScrollViewComponentView object leak (microsoft#13633)
* [Fabric] Fix ScrollViewComponentView object leak * Change files * fix * fix * Update CompositionEventHandler.cpp * fix
1 parent dbb032f commit b47b39f

File tree

8 files changed

+354
-282
lines changed

8 files changed

+354
-282
lines changed

‎change/react-native-windows-d8175a4a-46c8-40d0-b42b-097f50a633c1.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "prerelease",
3+
"comment": "[Fabric] Fix ScrollViewComponentView object leak",
4+
"packageName": "react-native-windows",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

‎packages/playground/windows/playground-composition/Playground-Composition.cpp

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ struct WindowData {
123123

124124
std::wstring m_bundleFile;
125125
winrt::Microsoft::ReactNative::ReactNativeIsland m_compRootView{nullptr};
126+
winrt::Microsoft::UI::Content::DesktopChildSiteBridge m_bridge{nullptr};
126127
winrt::Microsoft::ReactNative::ReactNativeHost m_host{nullptr};
127128
winrt::Microsoft::ReactNative::ReactInstanceSettings m_instanceSettings{nullptr};
128129
bool m_useLiftedComposition{true};
@@ -248,13 +249,13 @@ struct WindowData {
248249
// Register ellipse:// uri hander for images
249250
host.PackageProviders().Append(winrt::make<EllipseReactPackageProvider>());
250251

251-
auto bridge = winrt::Microsoft::UI::Content::DesktopChildSiteBridge::Create(
252+
m_bridge = winrt::Microsoft::UI::Content::DesktopChildSiteBridge::Create(
252253
g_liftedCompositor, winrt::Microsoft::UI::GetWindowIdFromWindow(hwnd));
253254

254255
auto appContent = m_compRootView.Island();
255256

256-
bridge.Connect(appContent);
257-
bridge.Show();
257+
m_bridge.Connect(appContent);
258+
m_bridge.Show();
258259

259260
m_compRootView.ScaleFactor(ScaleFactor(hwnd));
260261
winrt::Microsoft::ReactNative::LayoutConstraints constraints;
@@ -287,7 +288,7 @@ struct WindowData {
287288
}
288289
m_compRootView.Arrange(constraints, {0, 0});
289290

290-
bridge.ResizePolicy(winrt::Microsoft::UI::Content::ContentSizePolicy::ResizeContentToParentWindow);
291+
m_bridge.ResizePolicy(winrt::Microsoft::UI::Content::ContentSizePolicy::ResizeContentToParentWindow);
291292

292293
} else if (!m_target) {
293294
// General users of RNW should never set CompositionContext - this is an advanced usage to inject another
@@ -359,6 +360,23 @@ struct WindowData {
359360
case IDM_SETTINGS:
360361
DialogBoxParam(s_instance, MAKEINTRESOURCE(IDD_SETTINGSBOX), hwnd, &Settings, reinterpret_cast<INT_PTR>(this));
361362
break;
363+
case IDM_UNLOAD: {
364+
auto async = Host().UnloadInstance();
365+
async.Completed([&, uidispatch = InstanceSettings().UIDispatcher()](
366+
auto asyncInfo, winrt::Windows::Foundation::AsyncStatus asyncStatus) {
367+
asyncStatus;
368+
OutputDebugStringA("Instance Unload completed\n");
369+
370+
uidispatch.Post([&]() {
371+
m_bridge.Close();
372+
m_bridge = nullptr;
373+
});
374+
assert(asyncStatus == winrt::Windows::Foundation::AsyncStatus::Completed);
375+
});
376+
m_compRootView = nullptr;
377+
m_instanceSettings = nullptr;
378+
m_host = nullptr;
379+
} break;
362380
}
363381

364382
return 0;
@@ -561,11 +579,21 @@ LRESULT CALLBACK WndProc(HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam)
561579
bool shouldPostQuitMessage = true;
562580
if (data->m_host) {
563581
shouldPostQuitMessage = false;
582+
583+
winrt::Microsoft::ReactNative::ReactPropertyBag properties(data->m_host.InstanceSettings().Properties());
584+
585+
properties.Remove(winrt::Microsoft::ReactNative::ReactPropertyId<
586+
winrt::Microsoft::ReactNative::Composition::Experimental::ICompositionContext>{
587+
L"ReactNative.Composition", L"CompositionContext"});
588+
564589
auto async = data->m_host.UnloadInstance();
565590
async.Completed([host = data->m_host](auto asyncInfo, winrt::Windows::Foundation::AsyncStatus asyncStatus) {
566591
assert(asyncStatus == winrt::Windows::Foundation::AsyncStatus::Completed);
567592
host.InstanceSettings().UIDispatcher().Post([]() { PostQuitMessage(0); });
568593
});
594+
data->m_compRootView = nullptr;
595+
data->m_instanceSettings = nullptr;
596+
data->m_host = nullptr;
569597
}
570598

571599
delete WindowData::GetFromWindow(hwnd);

‎packages/playground/windows/playground-composition/Playground-Composition.rc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ BEGIN
5757
MENUITEM "&Open Javascript File...\tCtrl+O", IDM_OPENJSFILE
5858
MENUITEM "&New Window\tCtrl+N", IDM_NEWWINDOW
5959
MENUITEM "&Refresh\tF5", IDM_REFRESH
60+
MENUITEM "&Unload", IDM_UNLOAD
6061
MENUITEM SEPARATOR
6162
MENUITEM "&Settings...\tAlt+S", IDM_SETTINGS
6263
MENUITEM SEPARATOR

‎packages/playground/windows/playground-composition/resource.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#define IDC_THEMELABEL 110
2525
#define IDC_JSENGINELABEL 111
2626
#define IDC_SIZETOCONTENT 112
27+
#define IDM_UNLOAD 113
2728
#define IDI_ICON1 1008
2829

2930
// Next default values for new objects

‎vnext/Microsoft.ReactNative/Fabric/Composition/CompositionEventHandler.cpp

Lines changed: 117 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -140,152 +140,170 @@ struct CompositionInputKeyboardSource : winrt::implements<
140140
CompositionEventHandler::CompositionEventHandler(
141141
const winrt::Microsoft::ReactNative::ReactContext &context,
142142
const winrt::Microsoft::ReactNative::ReactNativeIsland &reactNativeIsland)
143-
: m_context(context), m_wkRootView(reactNativeIsland) {
143+
: m_context(context), m_wkRootView(reactNativeIsland) {}
144+
145+
void CompositionEventHandler::Initialize() noexcept {
144146
#ifdef USE_WINUI3
145-
if (auto island = reactNativeIsland.Island()) {
147+
if (auto island = m_wkRootView.get().Island()) {
146148
auto pointerSource = winrt::Microsoft::UI::Input::InputPointerSource::GetForIsland(island);
147149

148150
m_pointerPressedToken =
149-
pointerSource.PointerPressed([this](
151+
pointerSource.PointerPressed([wkThis = weak_from_this()](
150152
winrt::Microsoft::UI::Input::InputPointerSource const &,
151153
winrt::Microsoft::UI::Input::PointerEventArgs const &args) {
152-
if (auto strongRootView = m_wkRootView.get()) {
153-
if (SurfaceId() == -1)
154-
return;
155-
156-
auto pp = winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::PointerPoint>(
157-
args.CurrentPoint(), strongRootView.ScaleFactor());
158-
onPointerPressed(pp, args.KeyModifiers());
154+
if (auto strongThis = wkThis.lock()) {
155+
if (auto strongRootView = strongThis->m_wkRootView.get()) {
156+
if (strongThis->SurfaceId() == -1)
157+
return;
158+
159+
auto pp = winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::PointerPoint>(
160+
args.CurrentPoint(), strongRootView.ScaleFactor());
161+
strongThis->onPointerPressed(pp, args.KeyModifiers());
162+
}
159163
}
160164
});
161165

162166
m_pointerReleasedToken =
163-
pointerSource.PointerReleased([this](
167+
pointerSource.PointerReleased([wkThis = weak_from_this()](
164168
winrt::Microsoft::UI::Input::InputPointerSource const &,
165169
winrt::Microsoft::UI::Input::PointerEventArgs const &args) {
166-
if (auto strongRootView = m_wkRootView.get()) {
167-
if (SurfaceId() == -1)
168-
return;
169-
170-
auto pp = winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::PointerPoint>(
171-
args.CurrentPoint(), strongRootView.ScaleFactor());
172-
onPointerReleased(pp, args.KeyModifiers());
170+
if (auto strongThis = wkThis.lock()) {
171+
if (auto strongRootView = strongThis->m_wkRootView.get()) {
172+
if (strongThis->SurfaceId() == -1)
173+
return;
174+
175+
auto pp = winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::PointerPoint>(
176+
args.CurrentPoint(), strongRootView.ScaleFactor());
177+
strongThis->onPointerReleased(pp, args.KeyModifiers());
178+
}
173179
}
174180
});
175181

176-
m_pointerMovedToken = pointerSource.PointerMoved([this](
182+
m_pointerMovedToken = pointerSource.PointerMoved([wkThis = weak_from_this()](
177183
winrt::Microsoft::UI::Input::InputPointerSource const &,
178184
winrt::Microsoft::UI::Input::PointerEventArgs const &args) {
179-
if (auto strongRootView = m_wkRootView.get()) {
180-
if (SurfaceId() == -1)
181-
return;
182-
183-
auto pp = winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::PointerPoint>(
184-
args.CurrentPoint(), strongRootView.ScaleFactor());
185-
onPointerMoved(pp, args.KeyModifiers());
185+
if (auto strongThis = wkThis.lock()) {
186+
if (auto strongRootView = strongThis->m_wkRootView.get()) {
187+
if (strongThis->SurfaceId() == -1)
188+
return;
189+
190+
auto pp = winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::PointerPoint>(
191+
args.CurrentPoint(), strongRootView.ScaleFactor());
192+
strongThis->onPointerMoved(pp, args.KeyModifiers());
193+
}
186194
}
187195
});
188196

189197
m_pointerCaptureLostToken =
190-
pointerSource.PointerCaptureLost([this](
198+
pointerSource.PointerCaptureLost([wkThis = weak_from_this()](
191199
winrt::Microsoft::UI::Input::InputPointerSource const &,
192200
winrt::Microsoft::UI::Input::PointerEventArgs const &args) {
193-
if (auto strongRootView = m_wkRootView.get()) {
194-
if (SurfaceId() == -1)
195-
return;
196-
197-
auto pp = winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::PointerPoint>(
198-
args.CurrentPoint(), strongRootView.ScaleFactor());
199-
onPointerCaptureLost(pp, args.KeyModifiers());
201+
if (auto strongThis = wkThis.lock()) {
202+
if (auto strongRootView = strongThis->m_wkRootView.get()) {
203+
if (strongThis->SurfaceId() == -1)
204+
return;
205+
206+
auto pp = winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::PointerPoint>(
207+
args.CurrentPoint(), strongRootView.ScaleFactor());
208+
strongThis->onPointerCaptureLost(pp, args.KeyModifiers());
209+
}
200210
}
201211
});
202212

203213
m_pointerWheelChangedToken =
204-
pointerSource.PointerWheelChanged([this](
214+
pointerSource.PointerWheelChanged([wkThis = weak_from_this()](
205215
winrt::Microsoft::UI::Input::InputPointerSource const &,
206216
winrt::Microsoft::UI::Input::PointerEventArgs const &args) {
207-
if (auto strongRootView = m_wkRootView.get()) {
208-
if (SurfaceId() == -1)
209-
return;
210-
211-
auto pp = winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::PointerPoint>(
212-
args.CurrentPoint(), strongRootView.ScaleFactor());
213-
onPointerWheelChanged(pp, args.KeyModifiers());
217+
if (auto strongThis = wkThis.lock()) {
218+
if (auto strongRootView = strongThis->m_wkRootView.get()) {
219+
if (strongThis->SurfaceId() == -1)
220+
return;
221+
222+
auto pp = winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::PointerPoint>(
223+
args.CurrentPoint(), strongRootView.ScaleFactor());
224+
strongThis->onPointerWheelChanged(pp, args.KeyModifiers());
225+
}
214226
}
215227
});
216228

217229
auto keyboardSource = winrt::Microsoft::UI::Input::InputKeyboardSource::GetForIsland(island);
218230

219-
m_keyDownToken = keyboardSource.KeyDown([this](
231+
m_keyDownToken = keyboardSource.KeyDown([wkThis = weak_from_this()](
220232
winrt::Microsoft::UI::Input::InputKeyboardSource const &source,
221233
winrt::Microsoft::UI::Input::KeyEventArgs const &args) {
222-
if (auto strongRootView = m_wkRootView.get()) {
223-
if (SurfaceId() == -1)
224-
return;
225-
226-
auto focusedComponent = RootComponentView().GetFocusedComponent();
227-
auto keyboardSource = winrt::make<CompositionInputKeyboardSource>(source);
228-
auto keyArgs =
229-
winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::KeyRoutedEventArgs>(
230-
focusedComponent
231-
? focusedComponent.Tag()
232-
: static_cast<facebook::react::Tag>(
233-
winrt::get_self<winrt::Microsoft::ReactNative::implementation::ReactNativeIsland>(
234-
strongRootView)
235-
->RootTag()),
236-
args,
237-
keyboardSource);
238-
onKeyDown(keyArgs);
239-
winrt::get_self<CompositionInputKeyboardSource>(keyboardSource)->Disconnect();
234+
if (auto strongThis = wkThis.lock()) {
235+
if (auto strongRootView = strongThis->m_wkRootView.get()) {
236+
if (strongThis->SurfaceId() == -1)
237+
return;
238+
239+
auto focusedComponent = strongThis->RootComponentView().GetFocusedComponent();
240+
auto keyboardSource = winrt::make<CompositionInputKeyboardSource>(source);
241+
auto keyArgs =
242+
winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::KeyRoutedEventArgs>(
243+
focusedComponent
244+
? focusedComponent.Tag()
245+
: static_cast<facebook::react::Tag>(
246+
winrt::get_self<winrt::Microsoft::ReactNative::implementation::ReactNativeIsland>(
247+
strongRootView)
248+
->RootTag()),
249+
args,
250+
keyboardSource);
251+
strongThis->onKeyDown(keyArgs);
252+
winrt::get_self<CompositionInputKeyboardSource>(keyboardSource)->Disconnect();
253+
}
240254
}
241255
});
242256

243-
m_keyUpToken = keyboardSource.KeyUp([this](
257+
m_keyUpToken = keyboardSource.KeyUp([wkThis = weak_from_this()](
244258
winrt::Microsoft::UI::Input::InputKeyboardSource const &source,
245259
winrt::Microsoft::UI::Input::KeyEventArgs const &args) {
246-
if (auto strongRootView = m_wkRootView.get()) {
247-
if (SurfaceId() == -1)
248-
return;
249-
250-
auto focusedComponent = RootComponentView().GetFocusedComponent();
251-
auto keyboardSource = winrt::make<CompositionInputKeyboardSource>(source);
252-
auto keyArgs =
253-
winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::KeyRoutedEventArgs>(
254-
focusedComponent
255-
? focusedComponent.Tag()
256-
: static_cast<facebook::react::Tag>(
257-
winrt::get_self<winrt::Microsoft::ReactNative::implementation::ReactNativeIsland>(
258-
strongRootView)
259-
->RootTag()),
260-
args,
261-
keyboardSource);
262-
onKeyUp(keyArgs);
263-
winrt::get_self<CompositionInputKeyboardSource>(keyboardSource)->Disconnect();
260+
if (auto strongThis = wkThis.lock()) {
261+
if (auto strongRootView = strongThis->m_wkRootView.get()) {
262+
if (strongThis->SurfaceId() == -1)
263+
return;
264+
265+
auto focusedComponent = strongThis->RootComponentView().GetFocusedComponent();
266+
auto keyboardSource = winrt::make<CompositionInputKeyboardSource>(source);
267+
auto keyArgs =
268+
winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::KeyRoutedEventArgs>(
269+
focusedComponent
270+
? focusedComponent.Tag()
271+
: static_cast<facebook::react::Tag>(
272+
winrt::get_self<winrt::Microsoft::ReactNative::implementation::ReactNativeIsland>(
273+
strongRootView)
274+
->RootTag()),
275+
args,
276+
keyboardSource);
277+
strongThis->onKeyUp(keyArgs);
278+
winrt::get_self<CompositionInputKeyboardSource>(keyboardSource)->Disconnect();
279+
}
264280
}
265281
});
266282

267283
m_characterReceivedToken =
268-
keyboardSource.CharacterReceived([this](
284+
keyboardSource.CharacterReceived([wkThis = weak_from_this()](
269285
winrt::Microsoft::UI::Input::InputKeyboardSource const &source,
270286
winrt::Microsoft::UI::Input::CharacterReceivedEventArgs const &args) {
271-
if (auto strongRootView = m_wkRootView.get()) {
272-
if (SurfaceId() == -1)
273-
return;
274-
275-
auto focusedComponent = RootComponentView().GetFocusedComponent();
276-
auto keyboardSource = winrt::make<CompositionInputKeyboardSource>(source);
277-
auto charArgs = winrt::make<
278-
winrt::Microsoft::ReactNative::Composition::Input::implementation::CharacterReceivedRoutedEventArgs>(
279-
focusedComponent
280-
? focusedComponent.Tag()
281-
: static_cast<facebook::react::Tag>(
282-
winrt::get_self<winrt::Microsoft::ReactNative::implementation::ReactNativeIsland>(
283-
strongRootView)
284-
->RootTag()),
285-
args,
286-
keyboardSource);
287-
onCharacterReceived(charArgs);
288-
winrt::get_self<CompositionInputKeyboardSource>(keyboardSource)->Disconnect();
287+
if (auto strongThis = wkThis.lock()) {
288+
if (auto strongRootView = strongThis->m_wkRootView.get()) {
289+
if (strongThis->SurfaceId() == -1)
290+
return;
291+
292+
auto focusedComponent = strongThis->RootComponentView().GetFocusedComponent();
293+
auto keyboardSource = winrt::make<CompositionInputKeyboardSource>(source);
294+
auto charArgs = winrt::make<
295+
winrt::Microsoft::ReactNative::Composition::Input::implementation::CharacterReceivedRoutedEventArgs>(
296+
focusedComponent
297+
? focusedComponent.Tag()
298+
: static_cast<facebook::react::Tag>(
299+
winrt::get_self<winrt::Microsoft::ReactNative::implementation::ReactNativeIsland>(
300+
strongRootView)
301+
->RootTag()),
302+
args,
303+
keyboardSource);
304+
strongThis->onCharacterReceived(charArgs);
305+
winrt::get_self<CompositionInputKeyboardSource>(keyboardSource)->Disconnect();
306+
}
289307
}
290308
});
291309
}

‎vnext/Microsoft.ReactNative/Fabric/Composition/CompositionEventHandler.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,14 @@ struct FabricUIManager;
2727
struct winrt::Microsoft::ReactNative::implementation::ComponentView;
2828
typedef int PointerId;
2929

30-
class CompositionEventHandler {
30+
class CompositionEventHandler : public std::enable_shared_from_this<CompositionEventHandler> {
3131
public:
3232
CompositionEventHandler(
3333
const winrt::Microsoft::ReactNative::ReactContext &context,
3434
const winrt::Microsoft::ReactNative::ReactNativeIsland &ReactNativeIsland);
3535
virtual ~CompositionEventHandler();
3636

37+
void Initialize() noexcept;
3738
int64_t SendMessage(HWND hwnd, uint32_t msg, uint64_t wParam, int64_t lParam) noexcept;
3839
void RemoveTouchHandlers();
3940
winrt::Microsoft::UI::Input::VirtualKeyStates GetKeyState(winrt::Windows::System::VirtualKey key) noexcept;

‎vnext/Microsoft.ReactNative/Fabric/Composition/ReactNativeIsland.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,7 @@ void ReactNativeIsland::InitRootView(
408408
m_context = winrt::Microsoft::ReactNative::ReactContext(std::move(context));
409409
m_reactViewOptions = std::move(viewOptions);
410410
m_CompositionEventHandler = std::make_shared<::Microsoft::ReactNative::CompositionEventHandler>(m_context, *this);
411+
m_CompositionEventHandler->Initialize();
411412

412413
UpdateRootViewInternal();
413414

0 commit comments

Comments
 (0)