Make sure to unsubscribe from LayoutChanged on CV (#26138)

* Make sure to unsubscribe from LayoutChanged on CV * - cleanup
dotnet · Nov 27, 2024 · d91bf0a · d91bf0a
1 parent 1ce909f
commit d91bf0a
Show file tree

Hide file tree

Showing 4 changed files with 103 additions and 25 deletions.
diff --git a/src/Controls/src/Core/Handlers/Items/ItemsViewHandler.iOS.cs b/src/Controls/src/Core/Handlers/Items/ItemsViewHandler.iOS.cs
@@ -17,7 +17,7 @@ public abstract partial class ItemsViewHandler<TItemsView> : ViewHandler<TItemsV
 		protected override void DisconnectHandler(UIView platformView)
 		{
 			ItemsView.ScrollToRequested -= ScrollToRequested;
-			Controller?.DisposeItemsSource();
+			Controller?.Disconnect();
 			base.DisconnectHandler(platformView);
 		}
 

diff --git a/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewController.cs b/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewController.cs
@@ -73,6 +73,11 @@ public void UpdateLayout(ItemsViewLayout newLayout)
 			}
 		}
 
+		internal virtual void Disconnect()
+		{
+			DisposeItemsSource();
+		}
+
 		protected override void Dispose(bool disposing)
 		{
 			if (_disposed)

diff --git a/src/Controls/src/Core/Handlers/Items/iOS/StructuredItemsViewController.cs b/src/Controls/src/Core/Handlers/Items/iOS/StructuredItemsViewController.cs
@@ -25,6 +25,37 @@ public StructuredItemsViewController(TItemsView structuredItemsView, ItemsViewLa
 		{
 		}
 
+
+		internal override void Disconnect()
+		{
+			base.Disconnect();
+
+			if (_headerViewFormsElement is not null)
+			{
+				_headerViewFormsElement.MeasureInvalidated -= OnFormsElementMeasureInvalidated;
+			}
+
+			if (_footerViewFormsElement is not null)
+			{
+				_footerViewFormsElement.MeasureInvalidated -= OnFormsElementMeasureInvalidated;
+			}
+
+			if (_headerUIView is MauiView hv)
+			{
+				hv.LayoutChanged -= HeaderViewLayoutChanged;
+			}
+
+			if (_footerUIView is MauiView fv)
+			{
+				fv.LayoutChanged -= FooterViewLayoutChanged;
+			}
+
+			_headerUIView = null;
+			_headerViewFormsElement = null;
+			_footerUIView = null;
+			_footerViewFormsElement = null;
+		}
+
 		protected override void Dispose(bool disposing)
 		{
 			if (_disposed)
@@ -36,30 +67,7 @@ protected override void Dispose(bool disposing)
 
 			if (disposing)
 			{
-				if (_headerViewFormsElement != null)
-				{
-					_headerViewFormsElement.MeasureInvalidated -= OnFormsElementMeasureInvalidated;
-				}
-
-				if (_footerViewFormsElement != null)
-				{
-					_footerViewFormsElement.MeasureInvalidated -= OnFormsElementMeasureInvalidated;
-				}
-
-				if (_headerUIView is MauiView hv)
-				{
-					hv.LayoutChanged -= HeaderViewLayoutChanged;
-				}
-
-				if (_footerUIView is MauiView fv)
-				{
-					fv.LayoutChanged -= FooterViewLayoutChanged;
-				}
-
-				_headerUIView = null;
-				_headerViewFormsElement = null;
-				_footerUIView = null;
-				_footerViewFormsElement = null;
+				Disconnect();
 			}
 
 			base.Dispose(disposing);
@@ -112,6 +120,11 @@ public override void ViewWillLayoutSubviews()
 
 		internal void UpdateFooterView()
 		{
+			if (_footerUIView is MauiView mvPrevious)
+			{
+				mvPrevious.LayoutChanged -= FooterViewLayoutChanged;
+			}
+
 			UpdateSubview(ItemsView?.Footer, ItemsView?.FooterTemplate, FooterTag,
 				ref _footerUIView, ref _footerViewFormsElement);
 			UpdateHeaderFooterPosition();
@@ -124,6 +137,11 @@ internal void UpdateFooterView()
 
 		internal void UpdateHeaderView()
 		{
+			if (_headerUIView is MauiView mvPrevious)
+			{
+				mvPrevious.LayoutChanged -= HeaderViewLayoutChanged;
+			}
+
 			UpdateSubview(ItemsView?.Header, ItemsView?.HeaderTemplate, HeaderTag,
 				ref _headerUIView, ref _headerViewFormsElement);
 			UpdateHeaderFooterPosition();

diff --git a/src/Controls/tests/DeviceTests/Memory/MemoryTests.cs b/src/Controls/tests/DeviceTests/Memory/MemoryTests.cs
@@ -259,6 +259,61 @@ await InvokeOnMainThreadAsync(async () =>
 		await AssertionExtensions.WaitForGC(viewReference, handlerReference, platformViewReference);
 	}
 
+	[Fact("CollectionView Header/Footer Doesn't Leak")]
+	public async Task CollectionViewHeaderFooterDoesntLeak()
+	{
+		SetupBuilder();
+
+		WeakReference viewReference = null;
+		WeakReference handlerReference = null;
+		WeakReference controllerReference = null;
+
+		var observable = new ObservableCollection<int> { 1 };
+		var navPage = new NavigationPage(new ContentPage { Title = "Page 1" });
+
+		await CreateHandlerAndAddToWindow(new Window(navPage), async () =>
+		{
+			var cv = new CollectionView
+			{
+				Footer = new VerticalStackLayout(),
+				Header = new VerticalStackLayout(),
+				ItemTemplate = new DataTemplate(() =>
+				{
+					var view = new Label
+					{
+					};
+					view.SetBinding(Label.TextProperty, ".");
+					return view;
+				}),
+				ItemsSource = observable
+			};
+
+			viewReference = new WeakReference(cv);
+			handlerReference = new WeakReference(cv.Handler);
+
+
+			await navPage.Navigation.PushAsync(new ContentPage
+			{
+				Content = cv
+			});
+
+
+			#if IOS
+			var controller = (cv.Handler as CollectionViewHandler).Controller;
+			controllerReference = new WeakReference(controller);
+			controller = null;
+			#else
+			controllerReference = new WeakReference(new object());
+			#endif
+
+			cv = null;
+
+			await navPage.Navigation.PopAsync();
+		});
+
+		await AssertionExtensions.WaitForGC(viewReference, handlerReference, controllerReference);
+	}
+
 	[Theory("Gesture Does Not Leak")]
 	[InlineData(typeof(DragGestureRecognizer))]
 	[InlineData(typeof(DropGestureRecognizer))]