Fix MouseHighlighter shape object leak (remove TODO in production code)

Agent-Logs-Url: https://github.com/microsoft/PowerToys/sessions/b3bccf3d-6f32-4463-907d-723c08013a6b

Co-authored-by: MuyuanMS <116717757+MuyuanMS@users.noreply.github.com>

src/modules/MouseUtils/MouseHighlighter/MouseHighlighter.cpp

@@ -211,15 +211,23 @@ void Highlighter::AddDrawingPoint(MouseButton button)
         else
         {
             circleShape.FillBrush(m_compositor.CreateColorBrush(m_alwaysColor));
+            // Remove the previous always-pointer shape from the collection before replacing it.
+            // It has already been made transparent by ClearDrawingPoint(), so removing it
+            // causes no visual glitch and prevents an unbounded accumulation of shape objects.
+            if (m_alwaysPointer)
+            {
+                uint32_t index;
+                if (m_shape.Shapes().IndexOf(m_alwaysPointer, index))
+                {
+                    m_shape.Shapes().RemoveAt(index);
+                }
+            }
             m_alwaysPointer = circleShape;
         }
     }
 
     m_shape.Shapes().Append(circleShape);
 
-    // TODO: We're leaking shapes for long drawing sessions.
-    // Perhaps add a task to the Dispatcher every X circles to clean up.
-
     // Get back on top in case other Window is now the topmost.
     // HACK: Draw with 1 pixel off. Otherwise, Windows glitches the task bar transparency when a transparent window fill the whole screen.
     SetWindowPos(m_hwnd, HWND_TOPMOST, GetSystemMetrics(SM_XVIRTUALSCREEN) + 1, GetSystemMetrics(SM_YVIRTUALSCREEN) + 1, GetSystemMetrics(SM_CXVIRTUALSCREEN) - 2, GetSystemMetrics(SM_CYVIRTUALSCREEN) - 2, 0);
@@ -289,7 +297,19 @@ void Highlighter::StartDrawingPointFading(MouseButton button)
     animation.Duration(timeSpan(duration));
     animation.DelayTime(timeSpan(delay));
 
+    // Use a scoped batch to detect when the fade animation completes, then remove
+    // the fully-transparent shape from the collection so it does not leak.
+    auto batch = m_compositor.CreateScopedBatch(winrt::CompositionBatchTypes::Animation);
     circleShape.FillBrush().StartAnimation(L"Color", animation);
+    batch.End();
+    auto shapes = m_shape.Shapes();
+    batch.Completed([shape = circleShape, shapes](winrt::IInspectable const&, winrt::CompositionBatchCompletedEventArgs const&) {
+        uint32_t index;
+        if (shapes.IndexOf(shape, index))
+        {
+            shapes.RemoveAt(index);
+        }
+    });
 }
 
 void Highlighter::ClearDrawingPoint()

src/modules/launcher/PowerLauncher/Helper/ErrorReporting.cs

@@ -61,7 +61,7 @@ namespace PowerLauncher.Helper
             // Many bug reports because users see the "Report problem UI" after "the" crash with System.Runtime.InteropServices.COMException 0xD0000701 or 0x80263001.
             // However, displaying this "Report problem UI" during WPF crashes, especially when DWM composition is changing, is not ideal; some users reported it hangs for up to a minute before the "Report problem UI" appears.
             // This change modifies the behavior to log the exception instead of showing the "Report problem UI".
-            if (ExceptionHelper.IsRecoverableDwmCompositionException(e))
+            if (ExceptionHelper.IsRecoverableDwmCompositionException(e as System.Runtime.InteropServices.COMException))
             {
                 var logger = LogManager.GetLogger(LoggerName);
                 logger.Error($"From {(isNotUIThread ? "non" : string.Empty)} UI thread's exception: {ExceptionFormatter.FormatException(e)}");

src/modules/launcher/PowerLauncher/Helper/ExceptionHelper.cs

@@ -19,36 +19,26 @@ namespace PowerLauncher.Helper
 
         /// <summary>
         /// Returns true if the exception is a recoverable DWM composition exception.
-        /// Also checks inner exceptions to handle cases where a DWM error is wrapped by another exception type.
         /// </summary>
         internal static bool IsRecoverableDwmCompositionException(Exception exception)
         {
-            if (exception == null)
+            if (exception is not COMException comException)
             {
                 return false;
             }
 
-            // Walk the exception chain (up to a reasonable depth) to detect DWM composition errors at any level
-            const int maxDepth = 10;
-            var current = exception;
-            for (var depth = 0; current != null && depth < maxDepth; depth++, current = current.InnerException)
+            if (comException.HResult is DWM_E_COMPOSITIONDISABLED)
             {
-                if (current is COMException comException)
-                {
-                    if (comException.HResult is DWM_E_COMPOSITIONDISABLED)
-                    {
-                        return true;
-                    }
-
-                    if (comException.HResult is STATUS_MESSAGE_LOST_HR && comException.Source == PresentationFrameworkExceptionSource)
-                    {
-                        return true;
-                    }
-                }
+                return true;
             }
 
-            // Check for common DWM composition changed patterns in the stack trace of the outermost exception
-            var stackTrace = exception.StackTrace;
+            if (comException.HResult is STATUS_MESSAGE_LOST_HR && comException.Source == PresentationFrameworkExceptionSource)
+            {
+                return true;
+            }
+
+            // Check for common DWM composition changed patterns in the stack trace
+            var stackTrace = comException.StackTrace;
             return !string.IsNullOrEmpty(stackTrace) &&
                    stackTrace.Contains("DwmCompositionChanged");
         }

src/modules/launcher/PowerLauncher/Helper/ThemeManager.cs

@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
+using System.Runtime.InteropServices;
 using System.Threading;
 using System.Threading.Tasks;
 using System.Windows;
@@ -159,16 +160,12 @@ namespace PowerLauncher.Helper
 
                     return;
                 }
-                catch (Exception ex) when (ExceptionHelper.IsRecoverableDwmCompositionException(ex))
+                catch (COMException ex) when (ExceptionHelper.IsRecoverableDwmCompositionException(ex))
                 {
-                    var hresult = ex is System.Reflection.TargetInvocationException { InnerException: not null }
-                        ? ex.InnerException.HResult
-                        : ex.HResult;
-
                     switch (attempt)
                     {
                         case 1:
-                            Log.Warn($"Desktop composition is disabled (HRESULT: 0x{hresult:X}). Scheduling retries for theme update.", typeof(ThemeManager));
+                            Log.Warn($"Desktop composition is disabled (HRESULT: 0x{ex.HResult:X}). Scheduling retries for theme update.", typeof(ThemeManager));
                             delayMs = InitialDelayMs;
                             break;
                         case < maxAttempts:

src/modules/launcher/PowerLauncher/MainWindow.xaml.cs

@@ -196,17 +196,10 @@ namespace PowerLauncher
             if (OSVersionHelper.IsGreaterThanWindows11_21H2())
             {
                 // ResizeMode="NoResize" removes rounded corners. So force them to rounded.
-                try
-                {
-                    IntPtr hWnd = new WindowInteropHelper(GetWindow(this)).EnsureHandle();
-                    DWMWINDOWATTRIBUTE attribute = DWMWINDOWATTRIBUTE.DWMWA_WINDOW_CORNER_PREFERENCE;
-                    DWM_WINDOW_CORNER_PREFERENCE preference = DWM_WINDOW_CORNER_PREFERENCE.DWMWCP_ROUND;
-                    DwmSetWindowAttribute(hWnd, attribute, ref preference, sizeof(uint));
-                }
-                catch (Exception ex) when (ExceptionHelper.IsRecoverableDwmCompositionException(ex))
-                {
-                    Log.Warn($"Desktop composition is disabled. Unable to set window corner preference (cosmetic only). HRESULT: 0x{ex.HResult:X}", typeof(MainWindow));
-                }
+                IntPtr hWnd = new WindowInteropHelper(GetWindow(this)).EnsureHandle();
+                DWMWINDOWATTRIBUTE attribute = DWMWINDOWATTRIBUTE.DWMWA_WINDOW_CORNER_PREFERENCE;
+                DWM_WINDOW_CORNER_PREFERENCE preference = DWM_WINDOW_CORNER_PREFERENCE.DWMWCP_ROUND;
+                DwmSetWindowAttribute(hWnd, attribute, ref preference, sizeof(uint));
             }
             else
             {

src/modules/launcher/Wox.Test/ExceptionHelperTest.cs

@@ -1,63 +0,0 @@
-// Copyright (c) Microsoft Corporation
-// The Microsoft Corporation licenses this file to you under the MIT license.
-// See the LICENSE file in the project root for more information.
-
-using System;
-using System.Reflection;
-using System.Runtime.InteropServices;
-using Microsoft.VisualStudio.TestTools.UnitTesting;
-using PowerLauncher.Helper;
-
-namespace Wox.Test;
-
-[TestClass]
-public class ExceptionHelperTest
-{
-    private const int DwmCompositionDisabledHResult = unchecked((int)0x80263001);
-
-    [TestMethod]
-    public void IsRecoverableDwmCompositionException_ReturnsTrue_ForDirectRecoverableComException()
-    {
-        var exception = CreateComException(DwmCompositionDisabledHResult);
-
-        Assert.IsTrue(ExceptionHelper.IsRecoverableDwmCompositionException(exception));
-    }
-
-    [TestMethod]
-    public void IsRecoverableDwmCompositionException_ReturnsTrue_ForWrappedRecoverableComException()
-    {
-        var innerException = CreateComException(DwmCompositionDisabledHResult);
-        var exception = new TargetInvocationException(innerException);
-
-        Assert.IsTrue(ExceptionHelper.IsRecoverableDwmCompositionException(exception));
-    }
-
-    [TestMethod]
-    public void IsRecoverableDwmCompositionException_ReturnsFalse_ForWrappedNonRecoverableException()
-    {
-        var exception = new TargetInvocationException(new InvalidOperationException("Not a DWM composition failure."));
-
-        Assert.IsFalse(ExceptionHelper.IsRecoverableDwmCompositionException(exception));
-    }
-
-    [TestMethod]
-    public void IsRecoverableDwmCompositionException_ReturnsTrue_ForDwmCompositionChangedInStackTrace()
-    {
-        var exception = Assert.ThrowsException<InvalidOperationException>(DwmCompositionChangedThrower);
-
-        Assert.IsTrue(ExceptionHelper.IsRecoverableDwmCompositionException(exception));
-    }
-
-    private static COMException CreateComException(int hresult)
-    {
-        var exception = Marshal.GetExceptionForHR(hresult);
-        Assert.IsNotNull(exception);
-        Assert.IsInstanceOfType<COMException>(exception);
-        return (COMException)exception;
-    }
-
-    private static void DwmCompositionChangedThrower()
-    {
-        throw new InvalidOperationException("Stack trace fallback path.");
-    }
-}