diff --git a/src/Controls/src/Core/TitleBar/TitleBar.cs b/src/Controls/src/Core/TitleBar/TitleBar.cs index d915bfa13159..7465add2d58d 100644 --- a/src/Controls/src/Core/TitleBar/TitleBar.cs +++ b/src/Controls/src/Core/TitleBar/TitleBar.cs @@ -256,6 +256,16 @@ public TitleBar() } } + internal void Cleanup() + { + PropertyChanged -= TitleBar_PropertyChanged; + if (Window is not null) + { + Window.Activated -= Window_Activated; + Window.Deactivated -= Window_Deactivated; + } + } + private void TitleBar_PropertyChanged(object? sender, System.ComponentModel.PropertyChangedEventArgs e) { if (e.PropertyName == nameof(Window) && Window is not null) diff --git a/src/Controls/src/Core/Window/Window.cs b/src/Controls/src/Core/Window/Window.cs index 9e811170534f..d13d7cb4499a 100644 --- a/src/Controls/src/Core/Window/Window.cs +++ b/src/Controls/src/Core/Window/Window.cs @@ -392,6 +392,7 @@ static void TitleBarChanged(BindableObject bindable, object oldValue, object new { if (oldValue is TitleBar prevTitleBar) { + prevTitleBar.Cleanup(); self.RemoveLogicalChild(prevTitleBar); } diff --git a/src/Controls/tests/Core.UnitTests/TitleBarTests.cs b/src/Controls/tests/Core.UnitTests/TitleBarTests.cs new file mode 100644 index 000000000000..0ce87e10da12 --- /dev/null +++ b/src/Controls/tests/Core.UnitTests/TitleBarTests.cs @@ -0,0 +1,42 @@ +using System; +using System.Threading.Tasks; +using Xunit; + +namespace Microsoft.Maui.Controls.Core.UnitTests +{ + + public class TitleBarTests : BaseTestFixture + { + [Fact, Category(TestCategory.Memory)] + public async Task TitleBarDoesNotLeak() + { + var application = new Application(); + + WeakReference CreateReference() + { + var window = new Window { Page = new ContentPage() }; + var firstTitleBar = new TitleBar(); + var secondTitleBar = new TitleBar(); + var reference = new WeakReference(firstTitleBar); + + window.TitleBar = firstTitleBar; + + application.OpenWindow(window); + + window.TitleBar = secondTitleBar; + + ((IWindow)window).Destroying(); + return reference; + } + + var reference = CreateReference(); + + // GC + await TestHelpers.Collect(); + + Assert.False(reference.IsAlive, "TitleBar should not be alive!"); + + GC.KeepAlive(application); + } + } +} \ No newline at end of file diff --git a/src/Controls/tests/DeviceTests/Elements/Window/WindowTests.Windows.cs b/src/Controls/tests/DeviceTests/Elements/Window/WindowTests.Windows.cs index 39ea47319a41..530faf9ee862 100644 --- a/src/Controls/tests/DeviceTests/Elements/Window/WindowTests.Windows.cs +++ b/src/Controls/tests/DeviceTests/Elements/Window/WindowTests.Windows.cs @@ -12,6 +12,7 @@ using Xunit; using WPanel = Microsoft.UI.Xaml.Controls.Panel; using static Microsoft.Maui.DeviceTests.AssertHelpers; +using Windows.UI; namespace Microsoft.Maui.DeviceTests { @@ -239,6 +240,45 @@ await CreateHandlerAndAddToWindow(window, async (handler) => }); } + [Theory] + [ClassData(typeof(WindowPageSwapTestCases))] + public async Task TitlebarWorksWhenSwitchingPage(WindowPageSwapTestCase swapOrder) + { + SetupBuilder(); + + var firstRootPage = swapOrder.GetNextPageType(); + var window = new Window(firstRootPage) + { + TitleBar = new TitleBar() + { + Title = "Hello World", + BackgroundColor = Colors.CornflowerBlue + } + }; + + await CreateHandlerAndAddToWindow(window, async (handler) => + { + await OnLoadedAsync(swapOrder.Page); + while (!swapOrder.IsFinished()) + { + var nextRootPage = swapOrder.GetNextPageType(); + window.Page = nextRootPage; + + try + { + await OnLoadedAsync(swapOrder.Page); + + var navView = GetWindowRootView(handler); + Assert.NotNull(navView.TitleBar); + } + catch (Exception exc) + { + throw new Exception($"Failed to swap to {nextRootPage}", exc); + } + } + }); + } + [Collection(ControlsHandlerTestBase.RunInNewWindowCollection)] [Category(TestCategory.Lifecycle)] public class WindowTestsRunInNewWindowCollection : ControlsHandlerTestBase diff --git a/src/Controls/tests/DeviceTests/Elements/Window/WindowTests.cs b/src/Controls/tests/DeviceTests/Elements/Window/WindowTests.cs index 6391177f4d24..6afe70b147c1 100644 --- a/src/Controls/tests/DeviceTests/Elements/Window/WindowTests.cs +++ b/src/Controls/tests/DeviceTests/Elements/Window/WindowTests.cs @@ -53,6 +53,8 @@ void SetupBuilder() handlers.AddHandler(typeof(FlyoutPage), typeof(PhoneFlyoutPageRenderer)); #endif + handlers.AddHandler(); + handlers.AddHandler(); handlers.AddHandler(); handlers.AddHandler(); diff --git a/src/Core/src/Handlers/Window/WindowHandler.Windows.cs b/src/Core/src/Handlers/Window/WindowHandler.Windows.cs index 19a194cbd77e..8ec410aadb81 100644 --- a/src/Core/src/Handlers/Window/WindowHandler.Windows.cs +++ b/src/Core/src/Handlers/Window/WindowHandler.Windows.cs @@ -52,6 +52,7 @@ protected override void DisconnectHandler(UI.Xaml.Window platformView) if (windowRootContentManager is not null) { windowRootContentManager.OnApplyTemplateFinished -= WindowRootContentManagerOnApplyTemplateFinished; + windowRootContentManager.SetTitleBar(null, null); windowRootContentManager.Disconnect(); } @@ -78,18 +79,7 @@ public static void MapContent(IWindowHandler handler, IWindow window) _ = handler.MauiContext ?? throw new InvalidOperationException($"{nameof(MauiContext)} should have been set by base class."); var windowManager = handler.MauiContext.GetNavigationRootManager(); - var previousRootView = windowManager.RootView; - - windowManager.Disconnect(); - windowManager.Connect(handler.VirtualView.Content?.ToPlatform(handler.MauiContext)); - - if (handler.PlatformView.Content is WindowRootViewContainer container) - { - if (previousRootView != null && previousRootView != windowManager.RootView) - container.RemovePage(previousRootView); - - container.AddPage(windowManager.RootView); - } + windowManager.Connect(handler); window.VisualDiagnosticsOverlay?.Initialize(); } diff --git a/src/Core/src/Platform/Windows/NavigationRootManager.cs b/src/Core/src/Platform/Windows/NavigationRootManager.cs index bd1d4bfa7b65..9b175f5d31ab 100644 --- a/src/Core/src/Platform/Windows/NavigationRootManager.cs +++ b/src/Core/src/Platform/Windows/NavigationRootManager.cs @@ -82,6 +82,32 @@ void OnBackRequested(NavigationView sender, NavigationViewBackRequestedEventArgs internal MauiToolbar? Toolbar => _rootView.Toolbar; public FrameworkElement RootView => _rootView; + internal void Connect(IWindowHandler handler) + { + _ = handler.MauiContext ?? throw new InvalidOperationException($"{nameof(MauiContext)} should have been set by base class."); + + var platformWindow = _platformWindow.GetTargetOrDefault(); + if (platformWindow is null) + { + return; + } + + var previousRootView = RootView; + + Disconnect(); + Connect(handler.VirtualView.Content?.ToPlatform(handler.MauiContext)); + + if (platformWindow.Content is WindowRootViewContainer container) + { + if (previousRootView is not null && previousRootView != RootView) + { + container.RemovePage(previousRootView); + } + + container.AddPage(RootView); + } + } + public virtual void Connect(UIElement? platformView) { if (_rootView.Content != null) @@ -120,7 +146,6 @@ public virtual void Disconnect() } SetToolbar(null); - SetTitleBar(null, null); if (_rootView.Content is RootNavigationView navView) navView.Content = null; diff --git a/src/Core/src/Platform/Windows/WindowRootView.cs b/src/Core/src/Platform/Windows/WindowRootView.cs index 4b950f77e864..3b24bdcf7b42 100644 --- a/src/Core/src/Platform/Windows/WindowRootView.cs +++ b/src/Core/src/Platform/Windows/WindowRootView.cs @@ -80,6 +80,8 @@ internal MenuBar? MenuBar } } + internal ITitleBar? TitleBar => _titleBar; + public DataTemplate? AppTitleBarTemplate { get => (DataTemplate?)GetValue(AppTitleBarTemplateProperty);