[Fabric] Fix ScrollViewComponentView object leak (#13633)

acoates-ms · web-flow · commit 3486e573e051 · 2024-08-30T10:49:01.000-07:00
* [Fabric] Fix ScrollViewComponentView object leak

* Change files

* fix

* fix

* Update CompositionEventHandler.cpp

* fix
diff --git a/change/react-native-windows-d8175a4a-46c8-40d0-b42b-097f50a633c1.json b/change/react-native-windows-d8175a4a-46c8-40d0-b42b-097f50a633c1.json
@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "[Fabric] Fix ScrollViewComponentView object leak",
+  "packageName": "react-native-windows",
+  "email": "30809111+acoates-ms@users.noreply.github.com",
+  "dependentChangeType": "patch"
+}
diff --git a/packages/playground/windows/playground-composition/Playground-Composition.cpp b/packages/playground/windows/playground-composition/Playground-Composition.cpp
@@ -123,6 +123,7 @@ struct WindowData {
 
   std::wstring m_bundleFile;
   winrt::Microsoft::ReactNative::ReactNativeIsland m_compRootView{nullptr};
+  winrt::Microsoft::UI::Content::DesktopChildSiteBridge m_bridge{nullptr};
   winrt::Microsoft::ReactNative::ReactNativeHost m_host{nullptr};
   winrt::Microsoft::ReactNative::ReactInstanceSettings m_instanceSettings{nullptr};
   bool m_useLiftedComposition{true};
@@ -248,13 +249,13 @@ struct WindowData {
               // Register ellipse:// uri hander for images
               host.PackageProviders().Append(winrt::make<EllipseReactPackageProvider>());
 
-              auto bridge = winrt::Microsoft::UI::Content::DesktopChildSiteBridge::Create(
+              m_bridge = winrt::Microsoft::UI::Content::DesktopChildSiteBridge::Create(
                   g_liftedCompositor, winrt::Microsoft::UI::GetWindowIdFromWindow(hwnd));
 
               auto appContent = m_compRootView.Island();
 
-              bridge.Connect(appContent);
-              bridge.Show();
+              m_bridge.Connect(appContent);
+              m_bridge.Show();
 
               m_compRootView.ScaleFactor(ScaleFactor(hwnd));
               winrt::Microsoft::ReactNative::LayoutConstraints constraints;
@@ -287,7 +288,7 @@ struct WindowData {
               }
               m_compRootView.Arrange(constraints, {0, 0});
 
-              bridge.ResizePolicy(winrt::Microsoft::UI::Content::ContentSizePolicy::ResizeContentToParentWindow);
+              m_bridge.ResizePolicy(winrt::Microsoft::UI::Content::ContentSizePolicy::ResizeContentToParentWindow);
 
             } else if (!m_target) {
               // General users of RNW should never set CompositionContext - this is an advanced usage to inject another
@@ -359,6 +360,23 @@ struct WindowData {
       case IDM_SETTINGS:
         DialogBoxParam(s_instance, MAKEINTRESOURCE(IDD_SETTINGSBOX), hwnd, &Settings, reinterpret_cast<INT_PTR>(this));
         break;
+      case IDM_UNLOAD: {
+        auto async = Host().UnloadInstance();
+        async.Completed([&, uidispatch = InstanceSettings().UIDispatcher()](
+                            auto asyncInfo, winrt::Windows::Foundation::AsyncStatus asyncStatus) {
+          asyncStatus;
+          OutputDebugStringA("Instance Unload completed\n");
+
+          uidispatch.Post([&]() {
+            m_bridge.Close();
+            m_bridge = nullptr;
+          });
+          assert(asyncStatus == winrt::Windows::Foundation::AsyncStatus::Completed);
+        });
+        m_compRootView = nullptr;
+        m_instanceSettings = nullptr;
+        m_host = nullptr;
+      } break;
     }
 
     return 0;
@@ -561,12 +579,22 @@ LRESULT CALLBACK WndProc(HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam)
       bool shouldPostQuitMessage = true;
       if (data->m_host) {
         shouldPostQuitMessage = false;
+
+        winrt::Microsoft::ReactNative::ReactPropertyBag properties(data->m_host.InstanceSettings().Properties());
+
+        properties.Remove(winrt::Microsoft::ReactNative::ReactPropertyId<
+                          winrt::Microsoft::ReactNative::Composition::Experimental::ICompositionContext>{
+            L"ReactNative.Composition", L"CompositionContext"});
+
         auto async = data->m_host.UnloadInstance();
         async.Completed([host = data->m_host](auto asyncInfo, winrt::Windows::Foundation::AsyncStatus asyncStatus) {
           asyncStatus;
           assert(asyncStatus == winrt::Windows::Foundation::AsyncStatus::Completed);
           host.InstanceSettings().UIDispatcher().Post([]() { PostQuitMessage(0); });
         });
+        data->m_compRootView = nullptr;
+        data->m_instanceSettings = nullptr;
+        data->m_host = nullptr;
       }
 
       delete WindowData::GetFromWindow(hwnd);
diff --git a/packages/playground/windows/playground-composition/Playground-Composition.rc b/packages/playground/windows/playground-composition/Playground-Composition.rc
@@ -57,6 +57,7 @@ BEGIN
         MENUITEM "&Open Javascript File...\tCtrl+O", IDM_OPENJSFILE
         MENUITEM "&New Window\tCtrl+N",         IDM_NEWWINDOW
         MENUITEM "&Refresh\tF5",                IDM_REFRESH
+        MENUITEM "&Unload",                     IDM_UNLOAD
         MENUITEM SEPARATOR
         MENUITEM "&Settings...\tAlt+S",         IDM_SETTINGS
         MENUITEM SEPARATOR
diff --git a/packages/playground/windows/playground-composition/resource.h b/packages/playground/windows/playground-composition/resource.h
@@ -24,6 +24,7 @@
 #define IDC_THEMELABEL 110
 #define IDC_JSENGINELABEL 111
 #define IDC_SIZETOCONTENT 112
+#define IDM_UNLOAD 113
 #define IDI_ICON1 1008
 
 // Next default values for new objects
diff --git a/vnext/Microsoft.ReactNative/Fabric/Composition/CompositionEventHandler.cpp b/vnext/Microsoft.ReactNative/Fabric/Composition/CompositionEventHandler.cpp
@@ -140,152 +140,170 @@ struct CompositionInputKeyboardSource : winrt::implements<
 CompositionEventHandler::CompositionEventHandler(
     const winrt::Microsoft::ReactNative::ReactContext &context,
     const winrt::Microsoft::ReactNative::ReactNativeIsland &reactNativeIsland)
-    : m_context(context), m_wkRootView(reactNativeIsland) {
+    : m_context(context), m_wkRootView(reactNativeIsland) {}
+
+void CompositionEventHandler::Initialize() noexcept {
 #ifdef USE_WINUI3
-  if (auto island = reactNativeIsland.Island()) {
+  if (auto island = m_wkRootView.get().Island()) {
     auto pointerSource = winrt::Microsoft::UI::Input::InputPointerSource::GetForIsland(island);
 
     m_pointerPressedToken =
-        pointerSource.PointerPressed([this](
+        pointerSource.PointerPressed([wkThis = weak_from_this()](
                                          winrt::Microsoft::UI::Input::InputPointerSource const &,
                                          winrt::Microsoft::UI::Input::PointerEventArgs const &args) {
-          if (auto strongRootView = m_wkRootView.get()) {
-            if (SurfaceId() == -1)
-              return;
-
-            auto pp = winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::PointerPoint>(
-                args.CurrentPoint(), strongRootView.ScaleFactor());
-            onPointerPressed(pp, args.KeyModifiers());
+          if (auto strongThis = wkThis.lock()) {
+            if (auto strongRootView = strongThis->m_wkRootView.get()) {
+              if (strongThis->SurfaceId() == -1)
+                return;
+
+              auto pp = winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::PointerPoint>(
+                  args.CurrentPoint(), strongRootView.ScaleFactor());
+              strongThis->onPointerPressed(pp, args.KeyModifiers());
+            }
           }
         });
 
     m_pointerReleasedToken =
-        pointerSource.PointerReleased([this](
+        pointerSource.PointerReleased([wkThis = weak_from_this()](
                                           winrt::Microsoft::UI::Input::InputPointerSource const &,
                                           winrt::Microsoft::UI::Input::PointerEventArgs const &args) {
-          if (auto strongRootView = m_wkRootView.get()) {
-            if (SurfaceId() == -1)
-              return;
-
-            auto pp = winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::PointerPoint>(
-                args.CurrentPoint(), strongRootView.ScaleFactor());
-            onPointerReleased(pp, args.KeyModifiers());
+          if (auto strongThis = wkThis.lock()) {
+            if (auto strongRootView = strongThis->m_wkRootView.get()) {
+              if (strongThis->SurfaceId() == -1)
+                return;
+
+              auto pp = winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::PointerPoint>(
+                  args.CurrentPoint(), strongRootView.ScaleFactor());
+              strongThis->onPointerReleased(pp, args.KeyModifiers());
+            }
           }
         });
 
-    m_pointerMovedToken = pointerSource.PointerMoved([this](
+    m_pointerMovedToken = pointerSource.PointerMoved([wkThis = weak_from_this()](
                                                          winrt::Microsoft::UI::Input::InputPointerSource const &,
                                                          winrt::Microsoft::UI::Input::PointerEventArgs const &args) {
-      if (auto strongRootView = m_wkRootView.get()) {
-        if (SurfaceId() == -1)
-          return;
-
-        auto pp = winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::PointerPoint>(
-            args.CurrentPoint(), strongRootView.ScaleFactor());
-        onPointerMoved(pp, args.KeyModifiers());
+      if (auto strongThis = wkThis.lock()) {
+        if (auto strongRootView = strongThis->m_wkRootView.get()) {
+          if (strongThis->SurfaceId() == -1)
+            return;
+
+          auto pp = winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::PointerPoint>(
+              args.CurrentPoint(), strongRootView.ScaleFactor());
+          strongThis->onPointerMoved(pp, args.KeyModifiers());
+        }
       }
     });
 
     m_pointerCaptureLostToken =
-        pointerSource.PointerCaptureLost([this](
+        pointerSource.PointerCaptureLost([wkThis = weak_from_this()](
                                              winrt::Microsoft::UI::Input::InputPointerSource const &,
                                              winrt::Microsoft::UI::Input::PointerEventArgs const &args) {
-          if (auto strongRootView = m_wkRootView.get()) {
-            if (SurfaceId() == -1)
-              return;
-
-            auto pp = winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::PointerPoint>(
-                args.CurrentPoint(), strongRootView.ScaleFactor());
-            onPointerCaptureLost(pp, args.KeyModifiers());
+          if (auto strongThis = wkThis.lock()) {
+            if (auto strongRootView = strongThis->m_wkRootView.get()) {
+              if (strongThis->SurfaceId() == -1)
+                return;
+
+              auto pp = winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::PointerPoint>(
+                  args.CurrentPoint(), strongRootView.ScaleFactor());
+              strongThis->onPointerCaptureLost(pp, args.KeyModifiers());
+            }
           }
         });
 
     m_pointerWheelChangedToken =
-        pointerSource.PointerWheelChanged([this](
+        pointerSource.PointerWheelChanged([wkThis = weak_from_this()](
                                               winrt::Microsoft::UI::Input::InputPointerSource const &,
                                               winrt::Microsoft::UI::Input::PointerEventArgs const &args) {
-          if (auto strongRootView = m_wkRootView.get()) {
-            if (SurfaceId() == -1)
-              return;
-
-            auto pp = winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::PointerPoint>(
-                args.CurrentPoint(), strongRootView.ScaleFactor());
-            onPointerWheelChanged(pp, args.KeyModifiers());
+          if (auto strongThis = wkThis.lock()) {
+            if (auto strongRootView = strongThis->m_wkRootView.get()) {
+              if (strongThis->SurfaceId() == -1)
+                return;
+
+              auto pp = winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::PointerPoint>(
+                  args.CurrentPoint(), strongRootView.ScaleFactor());
+              strongThis->onPointerWheelChanged(pp, args.KeyModifiers());
+            }
           }
         });
 
     auto keyboardSource = winrt::Microsoft::UI::Input::InputKeyboardSource::GetForIsland(island);
 
-    m_keyDownToken = keyboardSource.KeyDown([this](
+    m_keyDownToken = keyboardSource.KeyDown([wkThis = weak_from_this()](
                                                 winrt::Microsoft::UI::Input::InputKeyboardSource const &source,
                                                 winrt::Microsoft::UI::Input::KeyEventArgs const &args) {
-      if (auto strongRootView = m_wkRootView.get()) {
-        if (SurfaceId() == -1)
-          return;
-
-        auto focusedComponent = RootComponentView().GetFocusedComponent();
-        auto keyboardSource = winrt::make<CompositionInputKeyboardSource>(source);
-        auto keyArgs =
-            winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::KeyRoutedEventArgs>(
-                focusedComponent
-                    ? focusedComponent.Tag()
-                    : static_cast<facebook::react::Tag>(
-                          winrt::get_self<winrt::Microsoft::ReactNative::implementation::ReactNativeIsland>(
-                              strongRootView)
-                              ->RootTag()),
-                args,
-                keyboardSource);
-        onKeyDown(keyArgs);
-        winrt::get_self<CompositionInputKeyboardSource>(keyboardSource)->Disconnect();
+      if (auto strongThis = wkThis.lock()) {
+        if (auto strongRootView = strongThis->m_wkRootView.get()) {
+          if (strongThis->SurfaceId() == -1)
+            return;
+
+          auto focusedComponent = strongThis->RootComponentView().GetFocusedComponent();
+          auto keyboardSource = winrt::make<CompositionInputKeyboardSource>(source);
+          auto keyArgs =
+              winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::KeyRoutedEventArgs>(
+                  focusedComponent
+                      ? focusedComponent.Tag()
+                      : static_cast<facebook::react::Tag>(
+                            winrt::get_self<winrt::Microsoft::ReactNative::implementation::ReactNativeIsland>(
+                                strongRootView)
+                                ->RootTag()),
+                  args,
+                  keyboardSource);
+          strongThis->onKeyDown(keyArgs);
+          winrt::get_self<CompositionInputKeyboardSource>(keyboardSource)->Disconnect();
+        }
       }
     });
 
-    m_keyUpToken = keyboardSource.KeyUp([this](
+    m_keyUpToken = keyboardSource.KeyUp([wkThis = weak_from_this()](
                                             winrt::Microsoft::UI::Input::InputKeyboardSource const &source,
                                             winrt::Microsoft::UI::Input::KeyEventArgs const &args) {
-      if (auto strongRootView = m_wkRootView.get()) {
-        if (SurfaceId() == -1)
-          return;
-
-        auto focusedComponent = RootComponentView().GetFocusedComponent();
-        auto keyboardSource = winrt::make<CompositionInputKeyboardSource>(source);
-        auto keyArgs =
-            winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::KeyRoutedEventArgs>(
-                focusedComponent
-                    ? focusedComponent.Tag()
-                    : static_cast<facebook::react::Tag>(
-                          winrt::get_self<winrt::Microsoft::ReactNative::implementation::ReactNativeIsland>(
-                              strongRootView)
-                              ->RootTag()),
-                args,
-                keyboardSource);
-        onKeyUp(keyArgs);
-        winrt::get_self<CompositionInputKeyboardSource>(keyboardSource)->Disconnect();
+      if (auto strongThis = wkThis.lock()) {
+        if (auto strongRootView = strongThis->m_wkRootView.get()) {
+          if (strongThis->SurfaceId() == -1)
+            return;
+
+          auto focusedComponent = strongThis->RootComponentView().GetFocusedComponent();
+          auto keyboardSource = winrt::make<CompositionInputKeyboardSource>(source);
+          auto keyArgs =
+              winrt::make<winrt::Microsoft::ReactNative::Composition::Input::implementation::KeyRoutedEventArgs>(
+                  focusedComponent
+                      ? focusedComponent.Tag()
+                      : static_cast<facebook::react::Tag>(
+                            winrt::get_self<winrt::Microsoft::ReactNative::implementation::ReactNativeIsland>(
+                                strongRootView)
+                                ->RootTag()),
+                  args,
+                  keyboardSource);
+          strongThis->onKeyUp(keyArgs);
+          winrt::get_self<CompositionInputKeyboardSource>(keyboardSource)->Disconnect();
+        }
       }
     });
 
     m_characterReceivedToken =
-        keyboardSource.CharacterReceived([this](
+        keyboardSource.CharacterReceived([wkThis = weak_from_this()](
                                              winrt::Microsoft::UI::Input::InputKeyboardSource const &source,
                                              winrt::Microsoft::UI::Input::CharacterReceivedEventArgs const &args) {
-          if (auto strongRootView = m_wkRootView.get()) {
-            if (SurfaceId() == -1)
-              return;
-
-            auto focusedComponent = RootComponentView().GetFocusedComponent();
-            auto keyboardSource = winrt::make<CompositionInputKeyboardSource>(source);
-            auto charArgs = winrt::make<
-                winrt::Microsoft::ReactNative::Composition::Input::implementation::CharacterReceivedRoutedEventArgs>(
-                focusedComponent
-                    ? focusedComponent.Tag()
-                    : static_cast<facebook::react::Tag>(
-                          winrt::get_self<winrt::Microsoft::ReactNative::implementation::ReactNativeIsland>(
-                              strongRootView)
-                              ->RootTag()),
-                args,
-                keyboardSource);
-            onCharacterReceived(charArgs);
-            winrt::get_self<CompositionInputKeyboardSource>(keyboardSource)->Disconnect();
+          if (auto strongThis = wkThis.lock()) {
+            if (auto strongRootView = strongThis->m_wkRootView.get()) {
+              if (strongThis->SurfaceId() == -1)
+                return;
+
+              auto focusedComponent = strongThis->RootComponentView().GetFocusedComponent();
+              auto keyboardSource = winrt::make<CompositionInputKeyboardSource>(source);
+              auto charArgs = winrt::make<
+                  winrt::Microsoft::ReactNative::Composition::Input::implementation::CharacterReceivedRoutedEventArgs>(
+                  focusedComponent
+                      ? focusedComponent.Tag()
+                      : static_cast<facebook::react::Tag>(
+                            winrt::get_self<winrt::Microsoft::ReactNative::implementation::ReactNativeIsland>(
+                                strongRootView)
+                                ->RootTag()),
+                  args,
+                  keyboardSource);
+              strongThis->onCharacterReceived(charArgs);
+              winrt::get_self<CompositionInputKeyboardSource>(keyboardSource)->Disconnect();
+            }
           }
         });
   }
diff --git a/vnext/Microsoft.ReactNative/Fabric/Composition/CompositionEventHandler.h b/vnext/Microsoft.ReactNative/Fabric/Composition/CompositionEventHandler.h
@@ -27,13 +27,14 @@ struct FabricUIManager;
 struct winrt::Microsoft::ReactNative::implementation::ComponentView;
 typedef int PointerId;
 
-class CompositionEventHandler {
+class CompositionEventHandler : public std::enable_shared_from_this<CompositionEventHandler> {
  public:
   CompositionEventHandler(
       const winrt::Microsoft::ReactNative::ReactContext &context,
       const winrt::Microsoft::ReactNative::ReactNativeIsland &ReactNativeIsland);
   virtual ~CompositionEventHandler();
 
+  void Initialize() noexcept;
   int64_t SendMessage(HWND hwnd, uint32_t msg, uint64_t wParam, int64_t lParam) noexcept;
   void RemoveTouchHandlers();
   winrt::Microsoft::UI::Input::VirtualKeyStates GetKeyState(winrt::Windows::System::VirtualKey key) noexcept;
diff --git a/vnext/Microsoft.ReactNative/Fabric/Composition/ReactNativeIsland.cpp b/vnext/Microsoft.ReactNative/Fabric/Composition/ReactNativeIsland.cpp
@@ -408,6 +408,7 @@ void ReactNativeIsland::InitRootView(
   m_context = winrt::Microsoft::ReactNative::ReactContext(std::move(context));
   m_reactViewOptions = std::move(viewOptions);
   m_CompositionEventHandler = std::make_shared<::Microsoft::ReactNative::CompositionEventHandler>(m_context, *this);
+  m_CompositionEventHandler->Initialize();
 
   UpdateRootViewInternal();
 
diff --git a/vnext/Microsoft.ReactNative/Fabric/Composition/ScrollViewComponentView.cpp b/vnext/Microsoft.ReactNative/Fabric/Composition/ScrollViewComponentView.cpp
