[0.74] Fix reference cycle between CompositionRootView and Compositio…

…nEventHandler (#13171)

* fix

* Rework custom resources API (#13013)

* Rework custom resources API

* Change files

* fix

* Fix a reference cycle between CompositionRootView and CompositionEventHandler (#13163)

* Fix a reference cycle between CompositionRootView and CompositionEventHandler

* Change files

* fix

* Fix close when instance is not running

* fix change files

change/react-native-windows-55134846-d638-4b46-8190-55b23cf11c3d.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Rework custom resources API",
+  "packageName": "react-native-windows",
+  "email": "30809111+acoates-ms@users.noreply.github.com",
+  "dependentChangeType": "patch"
+}

change/react-native-windows-73af8adf-3550-45d3-baed-c5e49c135395.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Fix a reference cycle between CompositionRootView and CompositionEventHandler",
+  "packageName": "react-native-windows",
+  "email": "30809111+acoates-ms@users.noreply.github.com",
+  "dependentChangeType": "patch"
+}

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

@@ -460,9 +460,23 @@ LRESULT CALLBACK WndProc(HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam)
           hwnd, LOWORD(wparam), reinterpret_cast<HWND>(lparam), HIWORD(wparam));
     }
     case WM_DESTROY: {
+      auto data = WindowData::GetFromWindow(hwnd);
+      // Before we shutdown the application - gracefully unload the ReactNativeHost instance
+      bool shouldPostQuitMessage = true;
+      if (data->m_host) {
+        shouldPostQuitMessage = false;
+        auto async = data->m_host.UnloadInstance();
+        async.Completed([host = data->m_host](auto asyncInfo, winrt::Windows::Foundation::AsyncStatus asyncStatus) {
+          assert(asyncStatus == winrt::Windows::Foundation::AsyncStatus::Completed);
+          host.InstanceSettings().UIDispatcher().Post([]() { PostQuitMessage(0); });
+        });
+      }
+
       delete WindowData::GetFromWindow(hwnd);
       SetProp(hwnd, WindowDataProperty, 0);
-      PostQuitMessage(0);
+      if (shouldPostQuitMessage) {
+        PostQuitMessage(0);
+      }
       return 0;
     }
     case WM_NCCREATE: {
@@ -531,6 +545,14 @@ int RunPlayground(int showCmd, bool useWebDebugger) {
 
   g_liftedDispatcherQueueController.DispatcherQueue().RunEventLoop();
 
+  // Rundown the DispatcherQueue. This drains the queue and raises events to let components
+  // know the message loop has finished.
+  g_liftedDispatcherQueueController.ShutdownQueue();
+
+  // Destroy all Composition objects
+  g_compositor.Close();
+  g_compositor = nullptr;
+
   return 0;
 }
 

vnext/Microsoft.ReactNative/CompositionRootView.idl

@@ -95,8 +95,10 @@ namespace Microsoft.ReactNative
 
     Object GetUiaProvider();
 
-    DOC_STRING("Theme used for Platform colors within this RootView")
-    Microsoft.ReactNative.Composition.Theme Theme;
+    DOC_STRING("Provides resources used for Platform colors within this RootView")
+    Microsoft.ReactNative.Composition.ICustomResourceLoader Resources;
+
+    Microsoft.ReactNative.Composition.Theme Theme { get; };
 
 #ifdef USE_WINUI3
     Microsoft.UI.Content.ContentIsland Island { get; };

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

@@ -154,7 +154,7 @@ class CompositionEventHandler {
   PointerId m_touchId = 0;
 
   std::map<PointerId, std::vector<ReactTaggedView>> m_currentlyHoveredViewsPerPointer;
-  winrt::Microsoft::ReactNative::CompositionRootView m_compRootView{nullptr};
+  winrt::weak_ref<winrt::Microsoft::ReactNative::CompositionRootView> m_wkRootView;
   winrt::Microsoft::ReactNative::ReactContext m_context;
 
   facebook::react::Tag m_pointerCapturingComponentTag{-1}; // Component that has captured input

vnext/Microsoft.ReactNative/Fabric/Composition/CompositionRootView.cpp

@@ -131,6 +131,13 @@ CompositionRootView::CompositionRootView(const winrt::Microsoft::UI::Composition
     : m_compositor(compositor) {}
 #endif
 
+CompositionRootView::~CompositionRootView() noexcept {
+  if (m_uiDispatcher) {
+    assert(m_uiDispatcher.HasThreadAccess());
+    UninitRootView();
+  }
+}
+
 ReactNative::IReactViewHost CompositionRootView::ReactViewHost() noexcept {
   return m_reactViewHost;
 }
@@ -240,50 +247,55 @@ void CompositionRootView::ScaleFactor(float value) noexcept {
   }
 }
 
+winrt::Microsoft::ReactNative::Composition::ICustomResourceLoader CompositionRootView::Resources() noexcept {
+  return m_resources;
+}
+
+void CompositionRootView::Resources(
+    const winrt::Microsoft::ReactNative::Composition::ICustomResourceLoader &resources) noexcept {
+  m_resources = resources;
+
+  if (m_context && m_theme) {
+    Theme(winrt::make<winrt::Microsoft::ReactNative::Composition::implementation::Theme>(m_context, m_resources));
+  }
+}
+
 winrt::Microsoft::ReactNative::Composition::Theme CompositionRootView::Theme() noexcept {
   if (!m_theme) {
-    Theme(winrt::Microsoft::ReactNative::Composition::Theme::GetDefaultTheme(m_context.Handle()));
-    m_themeChangedSubscription = m_context.Notifications().Subscribe(
-        winrt::Microsoft::ReactNative::ReactNotificationId<void>(
-            winrt::Microsoft::ReactNative::Composition::Theme::ThemeChangedEventName()),
-        m_context.UIDispatcher(),
-        [wkThis = get_weak()](
-            IInspectable const & /*sender*/,
-            winrt::Microsoft::ReactNative::ReactNotificationArgs<void> const & /*args*/) {
-          auto pThis = wkThis.get();
-          pThis->Theme(winrt::Microsoft::ReactNative::Composition::Theme::GetDefaultTheme(pThis->m_context.Handle()));
-        });
+    assert(m_context);
+    if (m_resources) {
+      Theme(winrt::make<winrt::Microsoft::ReactNative::Composition::implementation::Theme>(m_context, m_resources));
+    } else {
+      Theme(winrt::Microsoft::ReactNative::Composition::Theme::GetDefaultTheme(m_context.Handle()));
+    }
   }
   return m_theme;
 }
 
 void CompositionRootView::Theme(const winrt::Microsoft::ReactNative::Composition::Theme &value) noexcept {
-  if (m_themeChangedSubscription) {
-    m_themeChangedSubscription.Unsubscribe();
-    m_themeChangedSubscription = nullptr;
-  }
-
   if (value == m_theme)
     return;
 
   m_theme = value;
 
   m_themeChangedRevoker = m_theme.ThemeChanged(
       winrt::auto_revoke,
-      [this](
+      [wkThis = get_weak()](
           const winrt::Windows::Foundation::IInspectable & /*sender*/,
           const winrt::Windows::Foundation::IInspectable & /*args*/) {
-        if (auto rootView = GetComponentView()) {
-          Mso::Functor<bool(const winrt::Microsoft::ReactNative::ComponentView &)> fn =
-              [](const winrt::Microsoft::ReactNative::ComponentView &view) noexcept {
-                winrt::get_self<winrt::Microsoft::ReactNative::implementation::ComponentView>(view)->onThemeChanged();
-                return false;
-              };
-
-          winrt::Microsoft::ReactNative::ComponentView view{nullptr};
-          winrt::check_hresult(rootView->QueryInterface(
-              winrt::guid_of<winrt::Microsoft::ReactNative::ComponentView>(), winrt::put_abi(view)));
-          walkTree(view, true, fn);
+        if (auto strongThis = wkThis.get()) {
+          if (auto rootView = strongThis->GetComponentView()) {
+            Mso::Functor<bool(const winrt::Microsoft::ReactNative::ComponentView &)> fn =
+                [](const winrt::Microsoft::ReactNative::ComponentView &view) noexcept {
+                  winrt::get_self<winrt::Microsoft::ReactNative::implementation::ComponentView>(view)->onThemeChanged();
+                  return false;
+                };
+
+            winrt::Microsoft::ReactNative::ComponentView view{nullptr};
+            winrt::check_hresult(rootView->QueryInterface(
+                winrt::guid_of<winrt::Microsoft::ReactNative::ComponentView>(), winrt::put_abi(view)));
+            walkTree(view, true, fn);
+          }
         }
       });
 
@@ -382,13 +394,8 @@ void CompositionRootView::InitRootView(
   }
 
   m_context = winrt::Microsoft::ReactNative::ReactContext(std::move(context));
-
-  winrt::Microsoft::ReactNative::CompositionRootView compositionRootView;
-  get_strong().as(compositionRootView);
-
   m_reactViewOptions = std::move(viewOptions);
-  m_CompositionEventHandler =
-      std::make_shared<::Microsoft::ReactNative::CompositionEventHandler>(m_context, compositionRootView);
+  m_CompositionEventHandler = std::make_shared<::Microsoft::ReactNative::CompositionEventHandler>(m_context, *this);
 
   UpdateRootViewInternal();
 

vnext/Microsoft.ReactNative/Fabric/Composition/CompositionRootView.h

@@ -45,6 +45,7 @@ struct CompositionRootView
     : CompositionRootViewT<CompositionRootView, Composition::Experimental::IInternalCompositionRootView>,
       ::Microsoft::ReactNative::ICompositionRootView {
   CompositionRootView() noexcept;
+  ~CompositionRootView() noexcept;
 
 #ifdef USE_WINUI3
   CompositionRootView(const winrt::Microsoft::UI::Composition::Compositor &compositor) noexcept;
@@ -73,6 +74,9 @@ struct CompositionRootView
   void RemoveRenderedVisual(const winrt::Microsoft::ReactNative::Composition::Experimental::IVisual &visual) noexcept;
   bool TrySetFocus() noexcept;
 
+  winrt::Microsoft::ReactNative::Composition::ICustomResourceLoader Resources() noexcept;
+  void Resources(const winrt::Microsoft::ReactNative::Composition::ICustomResourceLoader &resources) noexcept;
+
   winrt::Microsoft::ReactNative::Composition::Theme Theme() noexcept;
   void Theme(const winrt::Microsoft::ReactNative::Composition::Theme &value) noexcept;
 
@@ -134,8 +138,8 @@ struct CompositionRootView
   winrt::Microsoft::ReactNative::Composition::Experimental::IVisual m_rootVisual{nullptr};
   winrt::Microsoft::ReactNative::Composition::Experimental::ISpriteVisual m_loadingVisual{nullptr};
   winrt::Microsoft::ReactNative::Composition::Experimental::IActivityVisual m_loadingActivityVisual{nullptr};
+  winrt::Microsoft::ReactNative::Composition::ICustomResourceLoader m_resources{nullptr};
   winrt::Microsoft::ReactNative::Composition::Theme m_theme{nullptr};
-  winrt::Microsoft::ReactNative::ReactNotificationSubscription m_themeChangedSubscription{nullptr};
   winrt::Microsoft::ReactNative::Composition::Theme::ThemeChanged_revoker m_themeChangedRevoker;
 
   void UpdateRootViewInternal() noexcept;

vnext/Microsoft.ReactNative/Fabric/Composition/CompositionRootView_emptyimpl.cpp

@@ -28,6 +28,7 @@ struct CompositionReactViewInstance
 //===========================================================================
 
 CompositionRootView::CompositionRootView() noexcept {}
+CompositionRootView::~CompositionRootView() noexcept {}
 
 CompositionRootView::CompositionRootView(const winrt::Microsoft::UI::Composition::Compositor &compositor) noexcept {}
 
@@ -69,6 +70,12 @@ winrt::Microsoft::ReactNative::Composition::Theme CompositionRootView::Theme() n
 }
 void CompositionRootView::Theme(const winrt::Microsoft::ReactNative::Composition::Theme &) noexcept {}
 
+winrt::Microsoft::ReactNative::Composition::ICustomResourceLoader CompositionRootView::Resources() noexcept {
+  return nullptr;
+}
+void CompositionRootView::Resources(
+    const winrt::Microsoft::ReactNative::Composition::ICustomResourceLoader &) noexcept {}
+
 winrt::IInspectable CompositionRootView::GetUiaProvider() noexcept {
   return nullptr;
 }

vnext/Microsoft.ReactNative/Fabric/Composition/Theme.cpp

@@ -526,6 +526,15 @@ static const winrt::Microsoft::ReactNative::ReactPropertyId<winrt::Microsoft::Re
   return prop;
 }
 
+static const winrt::Microsoft::ReactNative::ReactPropertyId<
+    winrt::Microsoft::ReactNative::Composition::ICustomResourceLoader>
+    &ThemeResourcesPropertyId() noexcept {
+  static const winrt::Microsoft::ReactNative::ReactPropertyId<
+      winrt::Microsoft::ReactNative::Composition::ICustomResourceLoader>
+      prop{L"ReactNative.Composition", L"ThemeResources"};
+  return prop;
+}
+
 winrt::Microsoft::ReactNative::Composition::Theme Theme::EmptyTheme() noexcept {
   static winrt::Microsoft::ReactNative::Composition::Theme s_emptyTheme{nullptr};
   if (!s_emptyTheme) {
@@ -537,24 +546,33 @@ winrt::Microsoft::ReactNative::Composition::Theme Theme::EmptyTheme() noexcept {
 /*static*/ winrt::Microsoft::ReactNative::Composition::Theme Theme::GetDefaultTheme(
     const winrt::Microsoft::ReactNative::IReactContext &context) noexcept {
   return winrt::Microsoft::ReactNative::ReactPropertyBag(context.Properties())
-      .GetOrCreate(ThemePropertyId(), [context]() { return winrt::make<Theme>(context, nullptr); });
+      .GetOrCreate(ThemePropertyId(), [context]() {
+        return winrt::make<Theme>(
+            context,
+            winrt::Microsoft::ReactNative::ReactPropertyBag(context.Properties()).Get(ThemeResourcesPropertyId()));
+      });
 }
 
-/*static*/ void Theme::SetDefaultTheme(
+/*static*/ void Theme::SetDefaultResources(
     const winrt::Microsoft::ReactNative::ReactInstanceSettings &settings,
-    const winrt::Microsoft::ReactNative::Composition::Theme &theme) noexcept {
-  winrt::Microsoft::ReactNative::ReactPropertyBag(settings.Properties()).Set(ThemePropertyId(), theme);
-  settings.Notifications().SendNotification(ThemeChangedEventName(), nullptr, nullptr);
+    const winrt::Microsoft::ReactNative::Composition::ICustomResourceLoader &resources) noexcept {
+  winrt::Microsoft::ReactNative::ReactPropertyBag properties(settings.Properties());
+  properties.Set(ThemeResourcesPropertyId(), resources);
+  // If a default theme has already been created - we need to update it with the new resources
+  if (auto theme = properties.Get(ThemePropertyId())) {
+    winrt::get_self<Theme>(theme)->UpdateCustomResources(resources);
+  }
+}
+
+void Theme::UpdateCustomResources(
+    const winrt::Microsoft::ReactNative::Composition::ICustomResourceLoader &resources) noexcept {
+  m_customResourceLoader = resources;
+  ClearCacheAndRaiseChangedEvent();
 }
 
 IReactPropertyNamespace ThemeNamespace() noexcept {
   static IReactPropertyNamespace value = ReactPropertyBagHelper::GetNamespace(L"ReactNative.Theme");
   return value;
 }
 
-/*static*/ IReactPropertyName Theme::ThemeChangedEventName() noexcept {
-  static IReactPropertyName propName = ReactPropertyBagHelper::GetName(ThemeNamespace(), L"Changed");
-  return propName;
-}
-
 } // namespace winrt::Microsoft::ReactNative::Composition::implementation

vnext/Microsoft.ReactNative/Fabric/Composition/Theme.h

@@ -50,12 +50,13 @@ struct Theme : ThemeT<Theme, Experimental::IInternalTheme> {
 
   static winrt::Microsoft::ReactNative::Composition::Theme GetDefaultTheme(
       const winrt::Microsoft::ReactNative::IReactContext &context) noexcept;
-  static void SetDefaultTheme(
+  static void SetDefaultResources(
       const winrt::Microsoft::ReactNative::ReactInstanceSettings &settings,
-      const winrt::Microsoft::ReactNative::Composition::Theme &theme) noexcept;
-  static winrt::Microsoft::ReactNative::IReactPropertyName ThemeChangedEventName() noexcept;
+      const winrt::Microsoft::ReactNative::Composition::ICustomResourceLoader &resources) noexcept;
 
  private:
+  void UpdateCustomResources(
+      const winrt::Microsoft::ReactNative::Composition::ICustomResourceLoader &resources) noexcept;
   bool TryGetPlatformColor(const std::string &platformColor, winrt::Windows::UI::Color &color) noexcept;
   void ClearCacheAndRaiseChangedEvent() noexcept;
 

vnext/Microsoft.ReactNative/Fabric/Composition/Theme_emptyimpl.cpp

@@ -74,12 +74,8 @@ winrt::Microsoft::ReactNative::Composition::Theme Theme::EmptyTheme() noexcept {
   return nullptr;
 }
 
-/*static*/ void Theme::SetDefaultTheme(
+/*static*/ void Theme::SetDefaultResources(
     const winrt::Microsoft::ReactNative::ReactInstanceSettings &,
-    const winrt::Microsoft::ReactNative::Composition::Theme &) noexcept {}
-
-/*static*/ IReactPropertyName Theme::ThemeChangedEventName() noexcept {
-  return nullptr;
-}
+    const winrt::Microsoft::ReactNative::Composition::ICustomResourceLoader &) noexcept {}
 
 } // namespace winrt::Microsoft::ReactNative::Composition::implementation

vnext/Microsoft.ReactNative/Theme.idl

@@ -61,8 +61,7 @@ namespace Microsoft.ReactNative.Composition
     event Windows.Foundation.EventHandler<Object> ThemeChanged;
 
     static Theme GetDefaultTheme(Microsoft.ReactNative.IReactContext context);
-    static void SetDefaultTheme(Microsoft.ReactNative.ReactInstanceSettings settings, Theme theme);
-    static Microsoft.ReactNative.IReactPropertyName ThemeChangedEventName { get; };
+    static void SetDefaultResources(Microsoft.ReactNative.ReactInstanceSettings settings, ICustomResourceLoader theme);
   };
 
 } // namespace Microsoft.ReactNative