Ensure disconnected ItemsViewHandler doesn't hold onto the items sour…

…ce (#24699) * Ensure disconnected ItemsViewHandler doesn't hold onto the items source * Update CarouselViewTests.cs
dotnet · Sep 11, 2024 · 9055699 · 9055699
1 parent 0548772
commit 9055699
Show file tree

Hide file tree

Showing 5 changed files with 75 additions and 6 deletions.
diff --git a/src/Controls/src/Core/Handlers/Items/CarouselViewHandler.Windows.cs b/src/Controls/src/Core/Handlers/Items/CarouselViewHandler.Windows.cs
@@ -38,7 +38,7 @@ public partial class CarouselViewHandler : ItemsViewHandler<CarouselView>
 		protected override void ConnectHandler(ListViewBase platformView)
 		{
 			ItemsView.Scrolled += CarouselScrolled;
-			ListViewBase.SizeChanged += OnListViewSizeChanged;
+			platformView.SizeChanged += OnListViewSizeChanged;
 
 			UpdateScrollBarVisibilityForLoop();
 
@@ -50,9 +50,9 @@ protected override void DisconnectHandler(ListViewBase platformView)
 			if (ItemsView != null)
 				ItemsView.Scrolled -= CarouselScrolled;
 
-			if (ListViewBase != null)
+			if (platformView != null)
 			{
-				ListViewBase.SizeChanged -= OnListViewSizeChanged;
+				platformView.SizeChanged -= OnListViewSizeChanged;
 				_proxy.Unsubscribe();
 			}
 

diff --git a/src/Controls/src/Core/Handlers/Items/ItemsViewHandler.Windows.cs b/src/Controls/src/Core/Handlers/Items/ItemsViewHandler.Windows.cs
@@ -57,6 +57,7 @@ protected override void ConnectHandler(ListViewBase platformView)
 		protected override void DisconnectHandler(ListViewBase platformView)
 		{
 			VirtualView.ScrollToRequested -= ScrollToRequested;
+			CleanUpCollectionViewSource(platformView);
 			base.DisconnectHandler(platformView);
 		}
 
@@ -154,6 +155,11 @@ void OnItemsVectorChanged(global::Windows.Foundation.Collections.IObservableVect
 		protected abstract ListViewBase SelectListViewBase();
 
 		protected virtual void CleanUpCollectionViewSource()
+		{
+			CleanUpCollectionViewSource(ListViewBase);
+		}
+
+		private void CleanUpCollectionViewSource(ListViewBase platformView)
 		{
 			if (CollectionViewSource is not null)
 			{
@@ -174,7 +180,7 @@ protected virtual void CleanUpCollectionViewSource()
 			// Remove all children inside the ItemsSource
 			if (VirtualView is not null)
 			{
-				foreach (var item in ListViewBase.GetChildren<ItemContentControl>())
+				foreach (var item in platformView.GetChildren<ItemContentControl>())
 				{
 					var element = item.GetVisualElement();
 					VirtualView.RemoveLogicalChild(element);
@@ -183,7 +189,7 @@ protected virtual void CleanUpCollectionViewSource()
 
 			if (VirtualView?.ItemsSource is null)
 			{
-				ListViewBase.ItemsSource = null;
+				platformView.ItemsSource = null;
 				return;
 			}
 		}

diff --git a/src/Controls/src/Core/Handlers/Items/ItemsViewHandler.iOS.cs b/src/Controls/src/Core/Handlers/Items/ItemsViewHandler.iOS.cs
@@ -17,6 +17,7 @@ public abstract partial class ItemsViewHandler<TItemsView> : ViewHandler<TItemsV
 		protected override void DisconnectHandler(UIView platformView)
 		{
 			ItemsView.ScrollToRequested -= ScrollToRequested;
+			Controller?.DisposeItemsSource();
 			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
@@ -329,6 +329,15 @@ public virtual void UpdateItemsSource()
 			(ItemsView as IView)?.InvalidateMeasure();
 		}
 
+		internal void DisposeItemsSource()
+		{
+			_measurementCells?.Clear();
+			ItemsViewLayout?.ClearCellSizeCache();
+			ItemsSource?.Dispose();
+			ItemsSource = new EmptySource();
+			CollectionView.ReloadData();
+		}
+
 		public virtual void UpdateFlowDirection()
 		{
 			CollectionView.UpdateFlowDirection(ItemsView);

diff --git a/src/Controls/tests/DeviceTests/Elements/CarouselView/CarouselViewTests.cs b/src/Controls/tests/DeviceTests/Elements/CarouselView/CarouselViewTests.cs
@@ -1,4 +1,5 @@
 using System.Collections.ObjectModel;
+using System.Collections.Specialized;
 using System.Threading.Tasks;
 using Microsoft.Maui.Controls;
 using Microsoft.Maui.Controls.Handlers.Items;
@@ -112,6 +113,45 @@ await CreateHandlerAndAddToWindow<LayoutHandler>(layout, async (handler) =>
 				Assert.NotNull(handler.PlatformView);
 			});
 		}
+
+		#if !ANDROID //https://github.com/dotnet/maui/pull/24610
+		[Fact]
+		public async void DisconnectedCarouselViewDoesNotHookCollectionViewChanged()
+		{
+			SetupBuilder();
+
+			CollectionChangedObservableCollection<int> data = new CollectionChangedObservableCollection<int>()
+			{
+				1,
+				2,
+			};
+
+			var template = new DataTemplate(() =>
+			{
+				return new Grid()
+				{
+					new Label()
+				};
+			});
+
+			var carouselView = new CarouselView()
+			{
+				ItemTemplate = template,
+				ItemsSource = data
+			};
+
+			await CreateHandlerAndAddToWindow<CarouselViewHandler>(carouselView, async (handler) =>
+			{
+				await Task.Delay(100);
+				Assert.NotNull(handler.PlatformView);
+				Assert.False(data.IsCollectionChangedEventEmpty);
+			});
+
+			carouselView.Handler?.DisconnectHandler();
+
+			Assert.True(data.IsCollectionChangedEventEmpty);
+		}
+		#endif
 	}
 
 	internal class CustomDataTemplateSelectorSelector : DataTemplateSelector
@@ -129,4 +169,17 @@ protected override DataTemplate OnSelectTemplate(object item, BindableObject con
 				return Template2;
 		}
 	}
-}
+
+	internal class CollectionChangedObservableCollection<T> : ObservableCollection<T>, INotifyCollectionChanged
+	{
+		NotifyCollectionChangedEventHandler collectionChanged;
+
+		event NotifyCollectionChangedEventHandler INotifyCollectionChanged.CollectionChanged
+		{
+			add { collectionChanged += value; base.CollectionChanged += value; }
+			remove { collectionChanged -= value; base.CollectionChanged -= value; }
+		}
+
+		public bool IsCollectionChangedEventEmpty => collectionChanged is null;
+	}
+}