Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/Controls/src/Core/NavigationPage/NavigationPage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -838,7 +838,6 @@ await Owner.SendHandlerUpdateAsync(animated,
Owner.FireDisappearing(currentPage);
Owner.RemoveFromInnerChildren(currentPage);
Owner.CurrentPage = newCurrentPage;
Owner.RemoveFromInnerChildren(currentPage);
if (currentPage.TitleView != null)
{
currentPage.RemoveLogicalChild(currentPage.TitleView);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
using System;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using System.Text;
using System.Threading.Tasks;
using Google.Android.Material.AppBar;
using Microsoft.Maui;
using Microsoft.Maui.Controls;
using Microsoft.Maui.Controls.Handlers;
using Microsoft.Maui.DeviceTests.Stubs;
using Microsoft.Maui.Handlers;
using Microsoft.Maui.Platform;
using Xunit;
using static Microsoft.Maui.DeviceTests.AssertHelpers;

namespace Microsoft.Maui.DeviceTests
{
Expand Down Expand Up @@ -47,5 +50,65 @@ await CreateHandlerAndAddToWindow<WindowHandlerStub>(new Window(navPage), async
Assert.False(failed);
});
}

[Fact(DisplayName = "StackNavigationManager Clears References On Disconnect (Issue 33918)")]
public async Task StackNavigationManagerClearsReferencesOnDisconnect()
{
SetupBuilder();

var mainPage = new ContentPage { Title = "Main Page" };
var baseNavPage = new NavigationPage(mainPage);

await CreateHandlerAndAddToWindow<WindowHandlerStub>(new Window(baseNavPage), async (handler) =>
{
var tab1Content = new ContentPage { Title = "Tab 1", Content = new Label { Text = "Tab 1 Content" } };
var tab2Content = new ContentPage { Title = "Tab 2", Content = new Label { Text = "Tab 2 Content" } };

var tab1Nav = new NavigationPage(tab1Content) { Title = "Tab 1" };
var tab2Nav = new NavigationPage(tab2Content) { Title = "Tab 2" };

var tabbedPage = new TabbedPage
{
Title = "Tabbed Modal",
Children = { tab1Nav, tab2Nav }
};

await baseNavPage.Navigation.PushModalAsync(tabbedPage, animated: false);
await OnLoadedAsync(tab1Content);

var tab1Handler = tab1Nav.Handler as NavigationViewHandler;
var tab2Handler = tab2Nav.Handler as NavigationViewHandler;
Assert.NotNull(tab1Handler);
Assert.NotNull(tab2Handler);

var tab1SnManager = tab1Handler.StackNavigationManager;
var tab2SnManager = tab2Handler.StackNavigationManager;
Assert.NotNull(tab1SnManager);
Assert.NotNull(tab2SnManager);

tabbedPage.CurrentPage = tab2Nav;
await OnLoadedAsync(tab2Content);

await baseNavPage.Navigation.PopModalAsync(animated: false);

var flags = BindingFlags.NonPublic | BindingFlags.Instance;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of using reflection, why not write a test on MemoryTests and validate that GC will collect the NavigationPage?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That didn't work. Using reflection is a pattern used by other tests as well.

var currentPageField = typeof(StackNavigationManager).GetField("_currentPage", flags);
var fragmentContainerViewField = typeof(StackNavigationManager).GetField("_fragmentContainerView", flags);
var fragmentManagerField = typeof(StackNavigationManager).GetField("_fragmentManager", flags);

Assert.NotNull(currentPageField);
Assert.NotNull(fragmentContainerViewField);
Assert.NotNull(fragmentManagerField);

await AssertEventually(() =>
currentPageField.GetValue(tab1SnManager) == null &&
fragmentContainerViewField.GetValue(tab1SnManager) == null &&
fragmentManagerField.GetValue(tab1SnManager) == null &&
currentPageField.GetValue(tab2SnManager) == null &&
fragmentContainerViewField.GetValue(tab2SnManager) == null &&
fragmentManagerField.GetValue(tab2SnManager) == null,
message: "StackNavigationManager fields were not cleared after Disconnect()");
});
}
}
}
4 changes: 1 addition & 3 deletions src/Controls/tests/DeviceTests/Memory/MemoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,8 @@ void SetupBuilder()

[Theory("Pages Do Not Leak")]
[InlineData(typeof(ContentPage))]
#if !ANDROID
[InlineData(typeof(NavigationPage))]
//https://github.com/dotnet/maui/issues/27411
#endif
// Issue #27411 (partially) and #33918 have been fixed - NavigationPage no longer leaks on Android
[InlineData(typeof(TabbedPage))]
public async Task PagesDoNotLeak([DynamicallyAccessedMembers(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] Type type)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,10 @@ public virtual void Disconnect()
NavigationView = null;
SetNavHost(null);
_fragmentNavigator = null;
_currentPage = null;
NavigationStack = [];
_fragmentContainerView = null;
_fragmentManager = null;
}

public virtual void Connect(IView navigationView)
Expand Down
Loading