[ios/catalyst] fix more cycles in NavigationPage (#23164)

Fixes: #21453
Context: #22810

In #22810, a leak in `NavigationPage` was fixed for the case:

    Application.Current.MainPage = new NavigationPage(new Page1());
    Application.Current.MainPage = new Page2();

However, it does *not* work for the case:

    await Navigation.PushModalAsync(new NavigationPage(new Page1()));
    await Navigation.PopModalAsync();

I could reproduce this problem in `MemoryTests.cs`.

There were still a few cycles in `NavigationRenderer`:

* `NavigationRenderer` -> `VisualElement _element` ->
`NavigationRenderer`

* `NavigationRenderer` -> `Page Current` -> `NavigationRenderer`

* `NavigationRenderer` -> `ViewHandlerDelegator<NavigationPage>
_viewHandlerWrapper` -> `TElement _element` -> `NavigationRenderer`

After fixing these cycles, the test passes. The customer's sample also
seems to work:


![image](https:/dotnet/maui/assets/840039/51cac98c-fb33-4e88-9fd5-112d7a04817c)

`ViewHandlerDelegator` was slightly tricky, as I had to make a
`_tempElement` variable and *unset* it immediately after use.

src/Controls/src/Core/Compatibility/Handlers/NavigationPage/iOS/NavigationRenderer.cs

@@ -48,7 +48,8 @@ public class NavigationRenderer : UINavigationController, INavigationViewHandler
  public static CommandMapper<NavigationPage, NavigationRenderer> CommandMapper = new CommandMapper<NavigationPage, NavigationRenderer>(ViewHandler.ViewCommandMapper);
  ViewHandlerDelegator<NavigationPage> _viewHandlerWrapper;
  bool _navigating = false;
- VisualElement _element;
+ WeakReference<VisualElement> _element;
+ WeakReference<Page> _current;
  bool _uiRequestedPop; // User tapped the back button or swiped to navigate back
  MauiNavigationDelegate NavigationDelegate => Delegate as MauiNavigationDelegate;
 
@@ -60,14 +61,18 @@ public NavigationRenderer() : base(typeof(MauiControlsNavigationBar), null)
  Delegate = new MauiNavigationDelegate(this);
  }
 
- Page Current { get; set; }
+ Page Current
+ {
+ get => _current?.GetTargetOrDefault();
+ set => _current = value is null ? null : new(value);
+ }
 
  IPageController PageController => Element as IPageController;
 
  NavigationPage NavPage => Element as NavigationPage;
  INavigationPageController NavPageController => NavPage;
 
- public VisualElement Element { get => _viewHandlerWrapper.Element ?? _element; }
+ public VisualElement Element { get => _viewHandlerWrapper.Element ?? _element?.GetTargetOrDefault(); }
 
  public event EventHandler<VisualElementChangedEventArgs> ElementChanged;
 
@@ -85,7 +90,7 @@ public UIView NativeView
  public void SetElement(VisualElement element)
  {
  (this as IElementHandler).SetVirtualView(element);
- _element = element;
+ _element = element is null ? null : new(element);
  }
 
  public UIViewController ViewController
@@ -171,7 +176,8 @@ public override void ViewDidDisappear(bool animated)
  public override void ViewWillLayoutSubviews()
  {
  base.ViewWillLayoutSubviews();
- if (Current == null)
+
+ if (Current is not Page current)
  return;
 
  UpdateToolBarVisible();
@@ -180,7 +186,7 @@ public override void ViewWillLayoutSubviews()
  var toolbar = _secondaryToolbar;
 
  //save the state of the Current page we are calculating, this will fire before Current is updated
- _hasNavigationBar = NavigationPage.GetHasNavigationBar(Current);
+ _hasNavigationBar = NavigationPage.GetHasNavigationBar(current);
 
  // Use 0 if the NavBar is hidden or will be hidden
  var toolbarY = NavigationBarHidden || NavigationBar.Translucent || !_hasNavigationBar ? 0 : navBarFrameBottom;
@@ -475,8 +481,8 @@ void HandlePropertyChanged(object sender, PropertyChangedEventArgs e)
  }
  else if (e.PropertyName == NavigationPage.CurrentPageProperty.PropertyName)
  {
- Current = NavPage?.CurrentPage;
- ValidateNavbarExists(Current);
+ var current = Current = NavPage?.CurrentPage;
+ ValidateNavbarExists(current);
  }
  else if (e.PropertyName == IsNavigationBarTranslucentProperty.PropertyName)
  {
@@ -806,7 +812,7 @@ void UpdateBarTextColor()
  }
 
  // set Tint color (i. e. Back Button arrow and Text)
- var iconColor = Current != null ? NavigationPage.GetIconColor(Current) : null;
+ var iconColor = Current is Page current ? NavigationPage.GetIconColor(current) : null;
  if (iconColor == null)
  iconColor = barTextColor;
 
@@ -860,8 +866,8 @@ void UpdateToolBarVisible()
 
  if (currentHidden != _secondaryToolbar.Hidden)
  {
- if (Current?.Handler != null)
- Current.ToPlatform().InvalidateMeasure(Current);
+ if (Current is Page current && current.Handler is not null)
+ current.ToPlatform().InvalidateMeasure(current);
 
  if (VisibleViewController is ParentingViewController pvc)
  pvc.UpdateFrames();
@@ -1716,7 +1722,7 @@ void IElementHandler.SetMauiContext(IMauiContext mauiContext)
  void IElementHandler.SetVirtualView(Maui.IElement view)
  {
  _viewHandlerWrapper.SetVirtualView(view, ElementChanged, false);
- _element = view as VisualElement;
+ _element = view is VisualElement v ? new(v) : null;
 
  void ElementChanged(ElementChangedEventArgs<NavigationPage> e)
  {

src/Controls/src/Core/Compatibility/Handlers/ViewHandlerDelegator.cs

@@ -20,8 +20,14 @@ class ViewHandlerDelegator<TElement>
  internal IPropertyMapper _mapper;
  internal readonly CommandMapper _commandMapper;
  IPlatformViewHandler _viewHandler;
+#if IOS || MACCATALYST
+ TElement? _tempElement;
+ WeakReference<TElement>? _element;
+ public TElement? Element => _tempElement ?? _element?.GetTargetOrDefault();
+#else
  TElement? _element;
  public TElement? Element => _element;
+#endif
  bool _disposed;
 
  public ViewHandlerDelegator(
@@ -48,11 +54,11 @@ public void Invoke(string command, object? args)
 
  public void DisconnectHandler()
  {
- if (_element == null)
+ if (Element is not TElement element)
  return;
 
- if (_element.Handler == _viewHandler)
- _element.Handler = null;
+ if (element.Handler == _viewHandler)
+ element.Handler = null;
 
  _element = null;
 
@@ -80,6 +86,12 @@ public void SetVirtualView(
  {
 #if WINDOWS
  VisualElementRenderer<TElement, TNativeElement>.SetVirtualView(view, _viewHandler, onElementChanged, ref _element, ref _mapper, _defaultMapper, autoPackage);
+#elif IOS || MACCATALYST
+ // _tempElement is used here, because the Element property is accessed before SetVirtualView() returns
+ VisualElementRenderer<TElement>.SetVirtualView(view, _viewHandler, onElementChanged, ref _tempElement, ref _mapper, _defaultMapper, autoPackage);
+ // We use _element as a WeakReference, and clear _tempElement
+ _element = _tempElement is null ? null : new(_tempElement);
+ _tempElement = null;
 #else
  VisualElementRenderer<TElement>.SetVirtualView(view, _viewHandler, onElementChanged, ref _element, ref _mapper, _defaultMapper, autoPackage);
 #endif

src/Controls/tests/DeviceTests/Elements/NavigationPage/NavigationPageTests.cs

@@ -311,24 +311,25 @@ await CreateHandlerAndAddToWindow<WindowHandlerStub>(new Window(navPage), async
  public async Task DoesNotLeak()
  {
  SetupBuilder();
- WeakReference pageReference = null;
- WeakReference handlerReference = null;
+ var references = new List<WeakReference>();
 
  {
  var navPage = new NavigationPage(new ContentPage());
  var window = new Window(navPage);
 
  await CreateHandlerAndAddToWindow<WindowHandlerStub>(window, (handler) =>
  {
- pageReference = new WeakReference(navPage);
- handlerReference = new WeakReference(handler);
+ references.Add(new(navPage));
+ references.Add(new(navPage.Handler));
+ references.Add(new(navPage.Handler.PlatformView));
+ references.Add(new(handler));
 
  // Just replace the page with a new one
  window.Page = new ContentPage();
  });
  }
 
- await AssertionExtensions.WaitForGC(pageReference, handlerReference);
+ await AssertionExtensions.WaitForGC(references.ToArray());
  }
 
  [Fact(DisplayName = "Child Pages Do Not Leak")]

src/Controls/tests/DeviceTests/Memory/MemoryTests.cs

@@ -68,8 +68,10 @@ void SetupBuilder()
  });
  }
 
- [Fact("Page Does Not Leak")]
- public async Task PageDoesNotLeak()
+ [Theory("Pages Do Not Leak")]
+ [InlineData(typeof(ContentPage))]
+ [InlineData(typeof(NavigationPage))]
+ public async Task PagesDoNotLeak(Type type)
  {
  SetupBuilder();
 
@@ -81,7 +83,15 @@ public async Task PageDoesNotLeak()
 
  await CreateHandlerAndAddToWindow(new Window(navPage), async () =>
  {
- var page = new ContentPage { Content = new Label() };
+ var page = (Page)Activator.CreateInstance(type);
+ if (page is ContentPage contentPage)
+ {
+ contentPage.Content = new Label();
+ }
+ else if (page is NavigationPage navigationPage)
+ {
+ await navigationPage.PushAsync(new ContentPage { Content = new Label() });
+ }
  
  await navPage.Navigation.PushModalAsync(page);