Skip to content

Commit e4d66e2

Browse files
bhavanesh2001PureWeen
authored andcommitted
[iOS]Fix: FlyoutPage memory leak (#28769)
* fix * enable Bugzilla31255Test * add device test * make _element a weak reference directly * fix device test * Use AssertMemoryTest in UITest
1 parent 19705bd commit e4d66e2

File tree

4 files changed

+50
-19
lines changed

4 files changed

+50
-19
lines changed

src/Controls/src/Core/Compatibility/Handlers/FlyoutPage/iOS/PhoneFlyoutPageRenderer.cs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ public class PhoneFlyoutPageRenderer : UIViewController, IPlatformViewHandler
1717
{
1818
UIView _clickOffView;
1919
UIViewController _detailController;
20-
VisualElement _element;
20+
WeakReference<VisualElement> _element;
2121
bool _disposed;
2222

2323
UIViewController _flyoutController;
@@ -75,7 +75,7 @@ bool Presented
7575
get { return _presented; }
7676
}
7777

78-
public VisualElement Element => _viewHandlerWrapper.Element ?? _element;
78+
public VisualElement Element => _viewHandlerWrapper.Element ?? _element?.GetTargetOrDefault();
7979

8080
public event EventHandler<VisualElementChangedEventArgs> ElementChanged;
8181

@@ -100,7 +100,7 @@ public void SetElement(VisualElement element)
100100
_clickOffView = new UIView();
101101
_clickOffView.BackgroundColor = new Color(0, 0, 0, 0).ToPlatform();
102102
_viewHandlerWrapper.SetVirtualView(element, OnElementChanged, false);
103-
_element = element;
103+
_element = new(element);
104104

105105
if (_intialLayoutFinished)
106106
{
@@ -269,9 +269,7 @@ protected override void Dispose(bool disposing)
269269
}
270270

271271
EmptyContainers();
272-
273-
Page.SendDisappearing();
274-
272+
_element = null;
275273
_disposed = true;
276274
}
277275

src/Controls/tests/DeviceTests/Elements/FlyoutPage/FlyoutPageTests.cs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,39 @@ await CreateHandlerAndAddToWindow<FlyoutViewHandler>(flyoutPage, async (handler)
252252
});
253253
}
254254

255+
[Fact(DisplayName = "FlyoutPage as Modal Does Not Leak")]
256+
public async Task DoesNotLeakAsModal()
257+
{
258+
SetupBuilder();
259+
260+
var references = new List<WeakReference>();
261+
var launcherPage = new ContentPage();
262+
var window = new Window(launcherPage);
263+
264+
await CreateHandlerAndAddToWindow<WindowHandlerStub>(window, async handler =>
265+
{
266+
var flyoutPage = new FlyoutPage
267+
{
268+
Flyout = new ContentPage
269+
{
270+
Title = "Flyout",
271+
IconImageSource = "icon.png"
272+
},
273+
Detail = new ContentPage { Title = "Detail" }
274+
};
275+
276+
await launcherPage.Navigation.PushModalAsync(flyoutPage, true);
277+
278+
references.Add(new WeakReference(flyoutPage));
279+
references.Add(new WeakReference(flyoutPage.Flyout));
280+
references.Add(new WeakReference(flyoutPage.Detail));
281+
282+
await launcherPage.Navigation.PopModalAsync();
283+
});
284+
285+
await AssertionExtensions.WaitForGC(references.ToArray());
286+
}
287+
255288
bool CanDeviceDoSplitMode(FlyoutPage page)
256289
{
257290
return ((IFlyoutPageController)page).ShouldShowSplitMode;

src/Controls/tests/TestCases.HostApp/Issues/Bugzilla/Bugzilla31255.cs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ public MainPage()
1818
{
1919
VerticalOptions = LayoutOptions.Center,
2020
HorizontalTextAlignment = TextAlignment.Center,
21-
Text = "Page 1"
21+
Text = "Page 1",
22+
AutomationId = "MauiLabel"
2223
});
2324

2425
Content = stack;
@@ -48,8 +49,7 @@ async void StartTrackPage2()
4849
{
4950
while (true)
5051
{
51-
((Label)((StackLayout)Content).Children[0]).Text =
52-
string.Format("Page1. But Page2 IsAlive = {0}", _page2Tracker.IsAlive);
52+
((Label)((StackLayout)Content).Children[0]).Text = _page2Tracker.IsAlive ? "Failed" : "Success";
5353
await Task.Delay(1000);
5454
GarbageCollectionHelper.Collect();
5555
}
@@ -60,12 +60,12 @@ public class Page2 : FlyoutPage
6060
{
6161
public Page2()
6262
{
63-
Flyout = new Page()
63+
Flyout = new ContentPage
6464
{
6565
Title = "Flyout",
6666
IconImageSource = "Icon.png"
6767
};
68-
Detail = new Page() { Title = "Detail" };
68+
Detail = new ContentPage() { Title = "Detail" };
6969
}
7070

7171
protected override async void OnAppearing()
@@ -75,6 +75,10 @@ protected override async void OnAppearing()
7575
await Task.Delay(1000);
7676
await Navigation.PopModalAsync();
7777
}
78+
protected override void OnDisappearing()
79+
{
80+
base.OnDisappearing();
81+
}
7882
}
7983
}
8084
}
Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
#if IOS
2-
using NUnit.Framework;
1+
using NUnit.Framework;
32
using UITest.Appium;
43
using UITest.Core;
54

@@ -14,15 +13,12 @@ public Bugzilla31255(TestDevice testDevice) : base(testDevice)
1413
public override string Issue => "Flyout's page Icon cause memory leak after FlyoutPage is popped out by holding on page";
1514

1615
[Test]
17-
[Ignore("The sample is crashing. More information: https://github.com/dotnet/maui/issues/21206")]
1816
[Category(UITestCategories.Navigation)]
1917
[Category(UITestCategories.Compatibility)]
20-
public async Task Bugzilla31255Test()
18+
public void Bugzilla31255Test()
2119
{
2220
App.Screenshot("I am at Bugzilla 31255");
23-
await Task.Delay(5000);
24-
App.WaitForNoElement("Page1. But Page2 IsAlive = False");
21+
App.AssertMemoryTest();
2522
}
2623
}
27-
}
28-
#endif
24+
}

0 commit comments

Comments
 (0)