[Mica] Fix crash accessing BaseWindowHandler::m_capabilities from Cap…

…abilities::Changed handler (#6288) * use weak refs to avoid crashes accessing captured data * remove unnecessary weak refs * clean up commented code Co-authored-by: Dmitriy Komin <dkomin@windows.microsoft.com>
microsoft · Nov 15, 2021 · f904131 · f904131
1 parent 76f5ebf
commit f904131
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 31 deletions.
diff --git a/dev/Materials/Backdrop/MicaController.cpp b/dev/Materials/Backdrop/MicaController.cpp
@@ -51,7 +51,7 @@ bool MicaController::SetTarget(winrt::Windows::UI::Xaml::Window const& xamlWindo
         return false;
     }
 
-    m_windowHandler = std::make_unique<SystemBackdropComponentInternal::XamlWindowHandler>(this, xamlWindow);
+    m_windowHandler = winrt::make_self<SystemBackdropComponentInternal::XamlWindowHandler>(this, xamlWindow);
 
     // Ensure we are in the correct policy state only *after* creating the appropriate WindowHandler.
     // If we start in high contrast mode, the controller will query the window handler for the correct system fallback color.

diff --git a/dev/Materials/Backdrop/MicaController.h b/dev/Materials/Backdrop/MicaController.h
@@ -51,7 +51,7 @@ struct MicaController : winrt::implementation::MicaControllerT<MicaController>,
     winrt::CompositionBrush m_currentBrush{ nullptr };
     winrt::Windows::UI::Composition::Compositor m_compositor{ nullptr };
     winrt::weak_ref<winrt::Microsoft::UI::Private::Controls::ICompositionSupportsSystemBackdrop> m_target{ nullptr };
-    std::unique_ptr<SystemBackdropComponentInternal::BaseWindowHandler> m_windowHandler{ nullptr };
+    com_ptr<SystemBackdropComponentInternal::BaseWindowHandler> m_windowHandler{ nullptr };
 
     bool m_isActive{ false };
     bool m_isExplicitFallbackColorSet{ false };

diff --git a/dev/Materials/Backdrop/SystemBackdropWindowHandler.cpp b/dev/Materials/Backdrop/SystemBackdropWindowHandler.cpp
@@ -31,65 +31,70 @@ namespace SystemBackdropComponentInternal
 
         m_policy = std::make_unique<SystemBackdropPolicyStateMachine>(SystemBackdropPolicyState::Active);
 
-        m_powerManagerEventRevoker = winrt::Windows::System::Power::PowerManager::EnergySaverStatusChanged(winrt::auto_revoke, [&](auto&&, auto&&)
+        m_powerManagerEventRevoker = winrt::Windows::System::Power::PowerManager::EnergySaverStatusChanged(winrt::auto_revoke,
+            [weakThis = get_weak(), dispatcherQueue = m_dispatcherQueue](auto&&, auto&&)
             {
-                m_dispatcherQueue.TryEnqueue([&]()
+                dispatcherQueue.TryEnqueue([weakThis]()
                     {
-                        if (winrt::Windows::System::Power::PowerManager::EnergySaverStatus() == winrt::Windows::System::Power::EnergySaverStatus::On)
-                        {
-                            m_policy->SetPowerSavingMode(true);
-                        }
-                        else
+                        auto strongThis = weakThis.get();
+                        if (strongThis)
                         {
-                            m_policy->SetPowerSavingMode(false);
-                        }
+                            if (winrt::Windows::System::Power::PowerManager::EnergySaverStatus() == winrt::Windows::System::Power::EnergySaverStatus::On)
+                            {
+                                strongThis->m_policy->SetPowerSavingMode(true);
+                            }
+                            else
+                            {
+                                strongThis->m_policy->SetPowerSavingMode(false);
+                            }
 
-                        ActivateOrDeactivateController();
+                            strongThis->ActivateOrDeactivateController();
+                        }
                     });
             });
 
         m_capabilities = winrt::Windows::UI::Composition::CompositionCapabilities::GetForCurrentView();
-        m_capabilitiesEventRevoker = { m_capabilities, m_capabilities.Changed([&](auto&&, auto&&)
+        m_capabilitiesEventRevoker = { m_capabilities, m_capabilities.Changed(
+            [weakThis = get_weak(), dispatcherQueue = m_dispatcherQueue](auto&&, auto&&)
             {
-                auto capabilitiesWeakRef = winrt::make_weak(m_capabilities);
-
-                m_dispatcherQueue.TryEnqueue([capabilitiesWeakRef, this]()
+                dispatcherQueue.TryEnqueue([weakThis]()
                     {
-                        if (auto capabilities = capabilitiesWeakRef.get())
+                        auto strongThis= weakThis.get();
+                        if (strongThis)
                         {
-                            if (capabilities.AreEffectsFast())
+                            if (strongThis->m_capabilities.AreEffectsFast())
                             {
-                                m_policy->SetIncompatibleGraphicsDevice(false);
+                                strongThis->m_policy->SetIncompatibleGraphicsDevice(false);
                             }
                             else
                             {
-                                m_policy->SetIncompatibleGraphicsDevice(true);
+                                strongThis->m_policy->SetIncompatibleGraphicsDevice(true);
                             }
 
-                            ActivateOrDeactivateController();
+                            strongThis->ActivateOrDeactivateController();
                         }
                     });
             }) };
 
         m_uiSettings = winrt::Windows::UI::ViewManagement::UISettings();
-        m_uiSettingsEventRevoker = m_uiSettings.AdvancedEffectsEnabledChanged(winrt::auto_revoke, [&](auto&&, auto&&)
+        m_uiSettingsEventRevoker = m_uiSettings.AdvancedEffectsEnabledChanged(winrt::auto_revoke,
+            [weakThis = get_weak(), dispatcherQueue = m_dispatcherQueue](auto&&, auto&&)
             {
-                auto uiSettingsWeakRef = winrt::make_weak(m_uiSettings);
-
-                m_dispatcherQueue.TryEnqueue([uiSettingsWeakRef, this]()
+                dispatcherQueue.TryEnqueue([weakThis]()
                     {
-                        if (auto uiSettings = uiSettingsWeakRef.get())
+                        auto strongThis = weakThis.get();
+                        if (strongThis)
                         {
-                            if (uiSettings.AdvancedEffectsEnabled())
+                            if (strongThis->m_uiSettings.AdvancedEffectsEnabled())
                             {
-                                m_policy->SetTransparencyDisabled(false);
+                                strongThis->m_policy->SetTransparencyDisabled(false);
                             }
                             else
                             {
-                                m_policy->SetTransparencyDisabled(true);
+                                strongThis->m_policy->SetTransparencyDisabled(true);
                             }
 
-                            ActivateOrDeactivateController();
+                            strongThis->ActivateOrDeactivateController();
                         }
                     });
             });

diff --git a/dev/Materials/Backdrop/SystemBackdropWindowHandler.h b/dev/Materials/Backdrop/SystemBackdropWindowHandler.h
@@ -61,7 +61,7 @@ namespace SystemBackdropComponentInternal
         winrt::event_token m_token{};
     };
 
-    struct BaseWindowHandler
+    struct BaseWindowHandler : public winrt::implements<BaseWindowHandler, winrt::IInspectable>
     {
         virtual ~BaseWindowHandler()
         {