Skip to content
48 changes: 48 additions & 0 deletions src/Controls/tests/DeviceTests/Memory/MemoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,11 @@ void SetupBuilder()
#if IOS || MACCATALYST
handlers.AddHandler<NavigationPage, NavigationRenderer>();
handlers.AddHandler<TabbedPage, TabbedRenderer>();
handlers.AddHandler<FlyoutPage, PhoneFlyoutPageRenderer>();
#else
handlers.AddHandler<NavigationPage, NavigationViewHandler>();
handlers.AddHandler<TabbedPage, TabbedViewHandler>();
handlers.AddHandler<FlyoutPage, FlyoutViewHandler>();
#endif
});
});
Expand Down Expand Up @@ -148,6 +150,52 @@ await CreateHandlerAndAddToWindow(new Window(navPage), async () =>
await AssertionExtensions.WaitForGC(references.ToArray());
}

#if ANDROID
[Fact("FlyoutPage Detail Navigation Does Not Leak")]
public async Task FlyoutPageDetailNavigationDoesNotLeak()
{
SetupBuilder();

var references = new List<WeakReference>();

var initialDetail = new NavigationPage(new ContentPage { Title = "Initial Detail" });

var flyoutPage = new FlyoutPage
{
Flyout = new ContentPage { Title = "Flyout" },
Detail = initialDetail
};
Comment thread
pictos marked this conversation as resolved.

await CreateHandlerAndAddToWindow(new Window(flyoutPage), async () =>
{
for (int i = 0; i < 4; i++)
{
var detailPage = new ContentPage
{
Title = $"Detail {i}",
Content = new Label { Text = $"Content {i}" }
};
var navPage = new NavigationPage(detailPage);

flyoutPage.Detail = navPage;
flyoutPage.IsPresented = false;

await OnLoadedAsync(detailPage);

references.Add(new(detailPage));
references.Add(new(navPage));
}
});


// The last page will be alive and attached to the FlyoutPage
references.RemoveAt(references.Count - 1);
references.RemoveAt(references.Count - 1);

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

[Theory("Handler Does Not Leak")]
[InlineData(typeof(ActivityIndicator))]
[InlineData(typeof(Border))]
Expand Down
7 changes: 5 additions & 2 deletions src/Core/src/Handlers/FlyoutView/FlyoutViewHandler.Android.cs
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,11 @@ void UpdateDetailsFragmentView()
if (context is null)
return;

if (VirtualView.Detail?.Handler is IPlatformViewHandler pvh)
pvh.DisconnectHandler();
if (_detailViewFragment?.DetailView is IView previousDetail &&
previousDetail != VirtualView.Detail)
{
previousDetail.Handler?.DisconnectHandler();
}

var fragmentManager = MauiContext.GetFragmentManager();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,14 @@ public override void OnResume()

public override void OnDestroy()
{
if (_currentView is not null)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we can get rid of this block and only need the code that sets the nav manager to null

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.

After removing this block, the leak gets worse. You can see the stats in the table

Object Type Before Current Change Status
Page2 1 21 +2000% 🚨 MAJOR REGRESSION
PageHandler 4 24 +500% 🚨 MAJOR REGRESSION
StackNavigationManager 1 1 No change Still good
Callbacks 1 1 No change Still good
NavigationViewFragment 102 102 No change ⚠️ Still present

{
_fragmentContainerView?.RemoveView(_currentView);
_currentView.RemoveFromParent();
}
_currentView = null;
_fragmentContainerView = null;
_navigationManager = null;

base.OnDestroy();
}
Expand Down
Loading