From bbea0597cd0d90e4f0a84445f4ae35445f553bb4 Mon Sep 17 00:00:00 2001
From: Jonathan Peppers <jonathan.peppers@microsoft.com>
Date: Thu, 6 Jul 2023 09:29:54 -0500
Subject: [PATCH] WIP test shows leak in CollectionView

I added some logging for the sample:

* https://github.com/dotnet/maui/issues/14664
* https://github.com/nacompllo/MemoryLeakEverywhere/tree/bugfix/memoryLeakItemsSource

I found the "logical children" grow indefinitely on iOS/Catalyst, after
adding some logging it quickly got up to 71 items:

    2023-07-06 09:04:27.745 MemoryLeakEverywhere[93127:7440192] Microsoft.Maui.Controls.CollectionView added child -> Microsoft.Maui.Controls.Image, count: 71

I was able to reproduce in a test, after I added some code to allow the
`CollectionView` to create items (using `WidthRequest`/`HeightRequest`).

When you replace an `ItemsSource` on iOS, in this way:

    var newCollection = new ObservableCollection<string>();
    collectionView.ItemsSource = newCollection;
    foreach (var item in data)
    {
        newCollection.Add(item);
    }

It appears that the old items are not removed, the test fails with:

    Expected: 3
    Actual: 6

But passes on Windows & Android.

Somewhere, we need to call one of:

* `ItemsView.ClearLogicalChildren()`
* `ItemsView.RemoveLogicalChild()`

But I've not been successful yet.
---
 .../CollectionView/CollectionViewTests.cs     | 53 ++++++++++++-------
 1 file changed, 34 insertions(+), 19 deletions(-)

diff --git a/src/Controls/tests/DeviceTests/Elements/CollectionView/CollectionViewTests.cs b/src/Controls/tests/DeviceTests/Elements/CollectionView/CollectionViewTests.cs
index cb60179cdd00..4bf85bd03829 100644
--- a/src/Controls/tests/DeviceTests/Elements/CollectionView/CollectionViewTests.cs
+++ b/src/Controls/tests/DeviceTests/Elements/CollectionView/CollectionViewTests.cs
@@ -41,36 +41,51 @@ void SetupBuilder()
 		[Fact]
 		public async Task ItemsSourceDoesNotLeak()
 		{
+			const int expectedSize = 3;
 			SetupBuilder();
 
 			IList logicalChildren = null;
 			WeakReference weakReference = null;
+
 			var collectionView = new CollectionView
 			{
-				ItemTemplate = new DataTemplate(() => new Label())
+				ItemTemplate = new DataTemplate(() => new Label { HeightRequest = 100 }),
 			};
+			var layout = new Grid { IgnoreSafeArea = true, HeightRequest = 500, WidthRequest = 500 };
+			layout.Add(collectionView);
 
-			await CreateHandlerAndAddToWindow<CollectionViewHandler>(collectionView, async handler =>
 			{
-				var data = new ObservableCollection<string>()
-				{
-					"Item 1",
-					"Item 2",
-					"Item 3"
-				};
-				weakReference = new WeakReference(data);
+				var data = new ObservableCollection<string>();
+				GenerateItems(expectedSize, data);
 				collectionView.ItemsSource = data;
-				await Task.Delay(100);
 
-				// Get ItemsView._logicalChildren
-				var flags = BindingFlags.NonPublic | BindingFlags.Instance;
-				logicalChildren = typeof(Element).GetField("_internalChildren", flags).GetValue(collectionView) as IList;
-				Assert.NotNull(logicalChildren);
+				var frame = collectionView.Frame;
+				await CreateHandlerAndAddToWindow<LayoutHandler>(layout, async handler =>
+				{
+					weakReference = new WeakReference(data);
 
-				// Replace with cloned collection
-				collectionView.ItemsSource = new ObservableCollection<string>(data);
-				await Task.Delay(100);
-			});
+					await WaitForUIUpdate(frame, collectionView);
+					frame = collectionView.Frame;
+
+					// Get ItemsView._logicalChildren
+					var flags = BindingFlags.NonPublic | BindingFlags.Instance;
+					logicalChildren = typeof(Element).GetField("_internalChildren", flags).GetValue(collectionView) as IList;
+					Assert.NotNull(logicalChildren);
+					Assert.Equal(data.Count, logicalChildren.Count);
+
+					// Replace with ObservableCollection, calling newCollection.Add()
+					var newCollection = new ObservableCollection<string>();
+					collectionView.ItemsSource = newCollection;
+					foreach (var item in data)
+					{
+						newCollection.Add(item);
+					}
+					await Task.Delay(100);
+					Assert.Equal(data.Count, logicalChildren.Count);
+				});
+
+				// data is out of scope here
+			}
 
 			await Task.Yield();
 			GC.Collect();
@@ -79,7 +94,7 @@ await CreateHandlerAndAddToWindow<CollectionViewHandler>(collectionView, async h
 			Assert.NotNull(weakReference);
 			Assert.False(weakReference.IsAlive, "ObservableCollection should not be alive!");
 			Assert.NotNull(logicalChildren);
-			Assert.True(logicalChildren.Count <= 3, "_logicalChildren should not grow in size!");
+			Assert.Equal(expectedSize, logicalChildren.Count);
 		}
 
 		[Theory]