diff --git a/src/Controls/src/Core/Compatibility/Handlers/NavigationPage/iOS/NavigationRenderer.cs b/src/Controls/src/Core/Compatibility/Handlers/NavigationPage/iOS/NavigationRenderer.cs index 4d44d032c580..7c6a684c1492 100644 --- a/src/Controls/src/Core/Compatibility/Handlers/NavigationPage/iOS/NavigationRenderer.cs +++ b/src/Controls/src/Core/Compatibility/Handlers/NavigationPage/iOS/NavigationRenderer.cs @@ -674,11 +674,11 @@ void RemovePage(Page page) void RemoveViewControllers(bool animated) { var controller = TopViewController as ParentingViewController; - if (controller == null || controller.Child == null || controller.Child.Handler == null) + if (controller?.Child is not Page child || child.Handler == null) return; // Gesture in progress, lets not be proactive and just wait for it to finish - var task = GetAppearedOrDisappearedTask(controller.Child); + var task = GetAppearedOrDisappearedTask(child); task.ContinueWith(t => { @@ -1113,7 +1113,7 @@ class ParentingViewController : UIViewController { readonly WeakReference _navigation; - Page _child; + WeakReference _child; bool _disposed; ToolbarTracker _tracker = new ToolbarTracker(); @@ -1128,19 +1128,25 @@ public ParentingViewController(NavigationRenderer navigation) public Page Child { - get { return _child; } + get => _child?.GetTargetOrDefault(); set { - if (_child == value) + var child = Child; + if (child == value) return; - if (_child != null) - _child.PropertyChanged -= HandleChildPropertyChanged; - - _child = value; + if (child is not null) + child.PropertyChanged -= HandleChildPropertyChanged; - if (_child != null) - _child.PropertyChanged += HandleChildPropertyChanged; + if (value is not null) + { + _child = new(value); + value.PropertyChanged += HandleChildPropertyChanged; + } + else + { + _child = null; + } UpdateHasBackButton(); UpdateLargeTitles(); @@ -1228,7 +1234,6 @@ public override void ViewDidLoad() _tracker.Target = Child; _tracker.AdditionalTargets = Child.GetParentPages(); - _tracker.CollectionChanged += TrackerOnCollectionChanged; UpdateToolbarItems(); } @@ -1247,6 +1252,20 @@ public override void ViewWillAppear(bool animated) base.ViewWillAppear(animated); } + public override void WillMoveToParentViewController(UIViewController parent) + { + base.WillMoveToParentViewController(parent); + + if (parent is null) + { + _tracker.CollectionChanged -= TrackerOnCollectionChanged; + } + else + { + _tracker.CollectionChanged += TrackerOnCollectionChanged; + } + } + protected override void Dispose(bool disposing) { if (_disposed) @@ -1258,10 +1277,10 @@ protected override void Dispose(bool disposing) if (disposing) { - if (Child != null) + if (Child is Page child) { - Child.SendDisappearing(); - Child.PropertyChanged -= HandleChildPropertyChanged; + child.SendDisappearing(); + child.PropertyChanged -= HandleChildPropertyChanged; Child = null; } @@ -1517,17 +1536,17 @@ void TrackerOnCollectionChanged(object sender, EventArgs eventArgs) void UpdateHasBackButton() { - if (Child == null || NavigationItem.HidesBackButton == !NavigationPage.GetHasBackButton(Child)) + if (Child is not Page child || NavigationItem.HidesBackButton == !NavigationPage.GetHasBackButton(child)) return; - NavigationItem.HidesBackButton = !NavigationPage.GetHasBackButton(Child); + NavigationItem.HidesBackButton = !NavigationPage.GetHasBackButton(child); NavigationRenderer n; if (!_navigation.TryGetTarget(out n)) return; if (!(OperatingSystem.IsIOSVersionAtLeast(11) || OperatingSystem.IsMacCatalystVersionAtLeast(11)) || n._parentFlyoutPage != null) - UpdateTitleArea(Child); + UpdateTitleArea(child); } void UpdateNavigationBarVisibility(bool animated) diff --git a/src/Controls/src/Core/Menu/MenuItemTracker.cs b/src/Controls/src/Core/Menu/MenuItemTracker.cs index a6c614d086f3..e25510688dab 100644 --- a/src/Controls/src/Core/Menu/MenuItemTracker.cs +++ b/src/Controls/src/Core/Menu/MenuItemTracker.cs @@ -12,7 +12,8 @@ internal abstract class MenuItemTracker where TMenuItem : BaseMenuItem { int _flyoutDetails; - Page _target; + WeakReference _target; + List> _additionalTargets = new(); public MenuItemTracker() { } @@ -21,7 +22,28 @@ public MenuItemTracker() protected abstract IComparer CreateComparer(); - public IEnumerable AdditionalTargets { get; set; } + public IEnumerable AdditionalTargets + { + get + { + foreach (var target in _additionalTargets) + { + if (target.TryGetTarget(out var page)) + yield return page; + } + } + set + { + _additionalTargets.Clear(); + if (value is not null) + { + foreach (var page in value) + { + _additionalTargets.Add(new(page)); + } + } + } + } public bool HaveFlyoutPage { @@ -32,17 +54,18 @@ public bool HaveFlyoutPage public Page Target { - get { return _target; } + get => _target is not null && _target.TryGetTarget(out var target) ? target : null; set { - if (_target == value) + var target = Target; + if (target == value) return; - UntrackTarget(_target); - _target = value; + UntrackTarget(target); + _target = value is null ? null : new(value); - if (_target != null) - TrackTarget(_target); + if (value != null) + TrackTarget(value); EmitCollectionChanged(); } } diff --git a/src/Controls/tests/DeviceTests/Elements/NavigationPage/NavigationPageTests.cs b/src/Controls/tests/DeviceTests/Elements/NavigationPage/NavigationPageTests.cs index ab675465a492..55fbdc59f7db 100644 --- a/src/Controls/tests/DeviceTests/Elements/NavigationPage/NavigationPageTests.cs +++ b/src/Controls/tests/DeviceTests/Elements/NavigationPage/NavigationPageTests.cs @@ -303,9 +303,33 @@ await CreateHandlerAndAddToWindow(new Window(navPage), async }); } - [Fact(DisplayName = "NavigationPage Does Not Leak")] + [Fact(DisplayName = "Does Not Leak")] public async Task DoesNotLeak() { + SetupBuilder(); + WeakReference pageReference = null; + WeakReference handlerReference = null; + + { + var navPage = new NavigationPage(new ContentPage()); + var window = new Window(navPage); + + await CreateHandlerAndAddToWindow(window, (handler) => + { + pageReference = new WeakReference(navPage); + handlerReference = new WeakReference(handler); + + // Just replace the page with a new one + window.Page = new ContentPage(); + }); + } + + await AssertionExtensions.WaitForGC(pageReference, handlerReference); + } + + [Fact(DisplayName = "Child Pages Do Not Leak")] + public async Task ChildPagesDoNotLeak() + { #if ANDROID if (!OperatingSystem.IsAndroidVersionAtLeast(30))