Skip to content

Commit

Permalink
[windows] fix memory leak in MauiWinUIWindow (#23327)
Browse files Browse the repository at this point in the history
  • Loading branch information
jonathanpeppers authored Jul 4, 2024
1 parent 1779f57 commit 1628aea
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ internal void Unsubscribe(Window window)
{
IMauiContext? mauiContext = window?.Handler?.MauiContext;
var platformWindow = mauiContext?.GetPlatformWindow();
if (platformWindow == null)
return;

var toRemove = Subscriptions.Where(s => s.PlatformView == platformWindow).ToList();
var toRemove = platformWindow is null ?
Subscriptions.Where(s => s.VirtualView == window).ToList() :
Subscriptions.Where(s => s.PlatformView == platformWindow).ToList();

foreach (AlertRequestHelper alertRequestHelper in toRemove)
{
Expand Down
31 changes: 31 additions & 0 deletions src/Controls/tests/DeviceTests/Memory/MemoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Threading.Tasks;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Maui.Controls;
using Microsoft.Maui.Controls.Handlers;
using Microsoft.Maui.Controls.Handlers.Compatibility;
Expand Down Expand Up @@ -404,6 +405,36 @@ await CreateHandlerAndAddToWindow<WindowHandlerStub>(new Window(page), async _ =
await AssertionExtensions.WaitForGC(references[0], references[1], references[2]);
}

[Fact("Window Does Not Leak")]
public async Task WindowDoesNotLeak()
{
SetupBuilder();

var references = new List<WeakReference>();

{
var page = new ContentPage();
var window = new Window(page);
await CreateHandlerAndAddToWindow(window, async () =>
{
await OnLoadedAsync(page);
references.Add(new(window));
references.Add(new(window.Handler));
// NOTE: the PlatformView in this case remains alive in the test application:
// Activity on Android, Microsoft.UI.Xaml.Window on Windows, etc.
//references.Add(new(window.Handler.PlatformView));
if (MauiContext.Services.GetService<IApplication>() is ApplicationStub app)
{
app.SetWindow(null);
}
});
}

await AssertionExtensions.WaitForGC(references.ToArray());
}

#if IOS
[Fact]
public async Task ResignFirstResponderTouchGestureRecognizer()
Expand Down
2 changes: 1 addition & 1 deletion src/Core/src/Platform/Windows/MauiWinUIWindow.cs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ void OnWindowMessage(object? sender, WindowMessageEventArgs e)
var rootManager = Window?.Handler?.MauiContext?.GetNavigationRootManager();
if (rootManager != null)
{
rootManager?.SetTitleBarVisibility(hasTitleBar);
rootManager?.SetTitleBarVisibility(this, hasTitleBar);
}
}
}
Expand Down
27 changes: 15 additions & 12 deletions src/Core/src/Platform/Windows/NavigationRootManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,26 @@ namespace Microsoft.Maui.Platform
{
public partial class NavigationRootManager
{
Window _platformWindow;
readonly WeakReference<Window> _platformWindow;
WindowRootView _rootView;
bool _disconnected = true;
internal event EventHandler? OnApplyTemplateFinished;

public NavigationRootManager(Window platformWindow)
{
_platformWindow = platformWindow;
_platformWindow = new(platformWindow);
_rootView = new WindowRootView();
_rootView.BackRequested += OnBackRequested;
_rootView.OnApplyTemplateFinished += WindowRootViewOnApplyTemplateFinished;

var titleBar = _platformWindow.GetAppWindow()?.TitleBar;
var titleBar = platformWindow.GetAppWindow()?.TitleBar;
if (titleBar is not null)
{
SetTitleBarVisibility(titleBar.ExtendsContentIntoTitleBar);
SetTitleBarVisibility(platformWindow, titleBar.ExtendsContentIntoTitleBar);
}
}

internal void SetTitleBarVisibility(bool isVisible)
internal void SetTitleBarVisibility(Window platformWindow, bool isVisible)
{
// https://learn.microsoft.com/en-us/windows/apps/design/basics/titlebar-design
// Standard title bar height is 32px
Expand All @@ -35,10 +35,10 @@ internal void SetTitleBarVisibility(bool isVisible)
var appbarHeight = isVisible ? 32 : 0;
if (isVisible && UI.Windowing.AppWindowTitleBar.IsCustomizationSupported())
{
var titleBar = _platformWindow.GetAppWindow()?.TitleBar;
var titleBar = platformWindow.GetAppWindow()?.TitleBar;
if (titleBar is not null)
{
var density = _platformWindow.GetDisplayDensity();
var density = platformWindow.GetDisplayDensity();
appbarHeight = (int)(titleBar.Height / density);
}
}
Expand All @@ -55,7 +55,7 @@ void WindowRootViewOnWindowTitleBarContentSizeChanged(object? sender, EventArgs
if (_disconnected)
return;

_platformWindow?
_platformWindow.GetTargetOrDefault()?
.GetWindow()?
.Handler?
.UpdateValue(nameof(IWindow.TitleBarDragRectangles));
Expand All @@ -66,7 +66,7 @@ void WindowRootViewOnApplyTemplateFinished(object? sender, System.EventArgs e) =

void OnBackRequested(NavigationView sender, NavigationViewBackRequestedEventArgs args)
{
_platformWindow
_platformWindow.GetTargetOrDefault()?
.GetWindow()?
.BackButtonClicked();
}
Expand Down Expand Up @@ -94,9 +94,9 @@ public virtual void Connect(UIElement platformView)
Content = platformView
};

if (_disconnected)
if (_disconnected && _platformWindow.TryGetTarget(out var platformWindow))
{
_platformWindow.Activated += OnWindowActivated;
platformWindow.Activated += OnWindowActivated;
_disconnected = false;
}

Expand All @@ -106,7 +106,10 @@ public virtual void Connect(UIElement platformView)
public virtual void Disconnect()
{
_rootView.OnWindowTitleBarContentSizeChanged -= WindowRootViewOnWindowTitleBarContentSizeChanged;
_platformWindow.Activated -= OnWindowActivated;
if (_platformWindow.TryGetTarget(out var platformWindow))
{
platformWindow.Activated -= OnWindowActivated;
}
SetToolbar(null);

if (_rootView.Content is RootNavigationView navView)
Expand Down

0 comments on commit 1628aea

Please sign in to comment.