Prevent iOS CollectionView size shifts from clearing the cell size cache (#18464)

* Prevent iOS CollectionView size shifts from clearing the cell size cache
Fixes #17890

* Try to exclude tests from macOS

src/Controls/src/Core/Handlers/Items/iOS/ItemsViewController.cs

@@ -7,7 +7,6 @@ using CoreGraphics;
 using Foundation;
 using Microsoft.Maui.Controls.Internals;
 using Microsoft.Maui.Graphics;
-using ObjCRuntime;
 using UIKit;
 
 namespace Microsoft.Maui.Controls.Handlers.Items
@@ -178,12 +177,12 @@ namespace Microsoft.Maui.Controls.Handlers.Items
 		public override void ViewWillAppear(bool animated)
 		{
 			base.ViewWillAppear(animated);
-			ConstrainToItemsView();
+			ConstrainItemsToBounds();
 		}
 
 		public override void ViewWillLayoutSubviews()
 		{
-			ConstrainToItemsView();
+			ConstrainItemsToBounds();
 			base.ViewWillLayoutSubviews();
 			InvalidateMeasureIfContentSizeChanged();
 			LayoutEmptyView();
@@ -243,7 +242,7 @@ namespace Microsoft.Maui.Controls.Handlers.Items
 
 		internal Size? GetSize()
 		{
-			if (_emptyViewDisplayed)
+			if (_emptyViewDisplayed) 
 			{
 				return _emptyUIView.Frame.Size.ToSize();
 			}
@@ -251,18 +250,11 @@ namespace Microsoft.Maui.Controls.Handlers.Items
 			return CollectionView.CollectionViewLayout.CollectionViewContentSize.ToSize();
 		}
 
-		void ConstrainToItemsView()
+		void ConstrainItemsToBounds()
 		{
-			var itemsViewWidth = ItemsView.Width;
+			var contentBounds = CollectionView.AdjustedContentInset.InsetRect(CollectionView.Bounds);
-			var itemsViewHeight = ItemsView.Height;
+			var constrainedSize = contentBounds.Size;
-
+			ItemsViewLayout.UpdateConstraints(constrainedSize);
-			if (itemsViewHeight < 0 || itemsViewWidth < 0)
-			{
-				ItemsViewLayout.UpdateConstraints(CollectionView.Bounds.Size);
-				return;
-			}
-
-			ItemsViewLayout.UpdateConstraints(new CGSize(itemsViewWidth, itemsViewHeight));
 		}
 
 		void EnsureLayoutInitialized()
@@ -321,7 +313,6 @@ namespace Microsoft.Maui.Controls.Handlers.Items
 			Layout.InvalidateLayout();
 		}
 
-
 		public override nint NumberOfSections(UICollectionView collectionView)
 		{
 			CheckForEmptySource();

src/Controls/src/Core/Handlers/Items/iOS/ItemsViewLayout.cs

@@ -5,8 +5,6 @@ using System.ComponentModel;
 using CoreGraphics;
 using Foundation;
 using Microsoft.Extensions.Logging;
-using Microsoft.Maui.Controls.Internals;
-using ObjCRuntime;
 using UIKit;
 
 namespace Microsoft.Maui.Controls.Handlers.Items
@@ -21,9 +19,7 @@ namespace Microsoft.Maui.Controls.Handlers.Items
 		CGSize _currentSize;
 		WeakReference<Func<UICollectionViewCell>> _getPrototype;
 
-		const double ConstraintSizeTolerance = 0.00001;
+		readonly Dictionary<object, CGSize> _cellSizeCache = new();
-
-		Dictionary<object, CGSize> _cellSizeCache = new Dictionary<object, CGSize>();
 
 		public ItemsUpdatingScrollMode ItemsUpdatingScrollMode { get; set; }
 
@@ -102,7 +98,7 @@ namespace Microsoft.Maui.Controls.Handlers.Items
 
 		internal virtual void UpdateConstraints(CGSize size)
 		{
-			if (!RequiresConstraintUpdate(size, _currentSize))
+			if (size.IsCloseTo(_currentSize))
 			{
 				return;
 			}
@@ -568,23 +564,12 @@ namespace Microsoft.Maui.Controls.Handlers.Items
 
 		public override bool ShouldInvalidateLayoutForBoundsChange(CGRect newBounds)
 		{
-			if (newBounds.Size == _currentSize)
+			if(newBounds.Size == _currentSize)
 			{
 				return base.ShouldInvalidateLayoutForBoundsChange(newBounds);
 			}
 
-			if (OperatingSystem.IsIOSVersionAtLeast(11) || OperatingSystem.IsMacCatalystVersionAtLeast(11)
+			UpdateConstraints(CollectionView.AdjustedContentInset.InsetRect(newBounds).Size);
-#if TVOS
-				|| OperatingSystem.IsTvOSVersionAtLeast(11)
-#endif
-			)
-			{
-				UpdateConstraints(CollectionView.AdjustedContentInset.InsetRect(newBounds).Size);
-			}
-			else
-			{
-				UpdateConstraints(CollectionView.Bounds.Size);
-			}
 
 			return true;
 		}
@@ -610,20 +595,5 @@ namespace Microsoft.Maui.Controls.Handlers.Items
 		{
 			_cellSizeCache.Clear();
 		}
-
-		bool RequiresConstraintUpdate(CGSize newSize, CGSize current)
-		{
-			if (Math.Abs(newSize.Width - current.Width) > ConstraintSizeTolerance)
-			{
-				return true;
-			}
-
-			if (Math.Abs(newSize.Height - current.Height) > ConstraintSizeTolerance)
-			{
-				return true;
-			}
-
-			return false;
-		}
 	}
 }

src/Controls/src/Core/Handlers/Items/iOS/SizeExtensions.cs

@@ -0,0 +1,42 @@
+#nullable disable
+using System;
+using CoreGraphics;
+using Microsoft.Maui.Graphics;
+
+namespace Microsoft.Maui.Controls.Handlers.Items
+{
+	internal static class SizeExtensions 
+	{
+		const double Tolerance = 0.001;
+
+		public static bool IsCloseTo(this CGSize sizeA, CGSize sizeB) 
+		{
+			if (Math.Abs(sizeA.Height - sizeB.Height) > Tolerance)
+			{
+				return false;
+			}
+
+			if (Math.Abs(sizeA.Width - sizeB.Width) > Tolerance)
+			{
+				return false;
+			}
+
+			return true;
+		}
+
+		public static bool IsCloseTo(this CGSize sizeA, Size sizeB)
+		{
+			if (Math.Abs(sizeA.Height - sizeB.Height) > Tolerance)
+			{
+				return false;
+			}
+
+			if (Math.Abs(sizeA.Width - sizeB.Width) > Tolerance)
+			{
+				return false;
+			}
+
+			return true;
+		}
+	}
+}

src/Controls/src/Core/Handlers/Items/iOS/TemplatedCell.cs

@@ -5,7 +5,6 @@ using CoreGraphics;
 using Foundation;
 using Microsoft.Maui.Controls.Internals;
 using Microsoft.Maui.Graphics;
-using ObjCRuntime;
 using UIKit;
 
 namespace Microsoft.Maui.Controls.Handlers.Items
@@ -66,7 +65,7 @@ namespace Microsoft.Maui.Controls.Handlers.Items
 
 			var preferredSize = preferredAttributes.Frame.Size;
 
-			if (SizesAreSame(preferredSize, _size)
+			if (preferredSize.IsCloseTo(_size)
 				&& AttributesConsistentWithConstrainedDimension(preferredAttributes))
 			{
 				return preferredAttributes;
@@ -79,8 +78,6 @@ namespace Microsoft.Maui.Controls.Handlers.Items
 
 			OnLayoutAttributesChanged(preferredAttributes);
 
-			//_isMeasured = true;
-
 			return preferredAttributes;
 		}
 
@@ -290,23 +287,6 @@ namespace Microsoft.Maui.Controls.Handlers.Items
 
 		protected abstract bool AttributesConsistentWithConstrainedDimension(UICollectionViewLayoutAttributes attributes);
 
-		bool SizesAreSame(CGSize preferredSize, Size elementSize)
-		{
-			const double tolerance = 0.000001;
-
-			if (Math.Abs(preferredSize.Height - elementSize.Height) > tolerance)
-			{
-				return false;
-			}
-
-			if (Math.Abs(preferredSize.Width - elementSize.Width) > tolerance)
-			{
-				return false;
-			}
-
-			return true;
-		}
-
 		void UpdateVisualStates()
 		{
 			if (PlatformHandler?.VirtualView is VisualElement element)

src/Controls/tests/DeviceTests/Elements/CollectionView/CollectionViewCacheTests.ios.cs

@@ -0,0 +1,152 @@
+using System.Collections.Generic;
+using System.Threading.Tasks;
+using CoreGraphics;
+using Foundation;
+using Microsoft.Maui.Controls;
+using Microsoft.Maui.Controls.Handlers.Items;
+using UIKit;
+using Xunit;
+
+namespace Microsoft.Maui.DeviceTests
+{
+#if !MACCATALYST
+	public partial class CollectionViewTests
+	{
+		public class CacheTestCollectionView : CollectionView { }
+
+		class CacheTestCollectionViewHandler : ReorderableItemsViewHandler<CacheTestCollectionView>
+		{
+			protected override ItemsViewController<CacheTestCollectionView> CreateController(CacheTestCollectionView itemsView, ItemsViewLayout layout)
+			{
+				return new CacheTestItemsViewController(itemsView, layout);
+			}
+		}
+
+		class CacheTestItemsViewController : ItemsViewController<CacheTestCollectionView>
+		{
+			protected override bool IsHorizontal { get; }
+
+			public UICollectionViewDelegateFlowLayout DelegateFlowLayout { get; private set; }
+
+			public CacheTestItemsViewController(CacheTestCollectionView reorderableItemsView, ItemsViewLayout layout) : base(reorderableItemsView, layout)
+			{
+			}
+
+			protected override UICollectionViewDelegateFlowLayout CreateDelegator()
+			{
+				DelegateFlowLayout = new CacheMissCountingDelegate(ItemsViewLayout, this);
+				return DelegateFlowLayout;
+			}
+		}
+
+		internal class CacheMissCountingDelegate : ItemsViewDelegator<CacheTestCollectionView, ItemsViewController<CacheTestCollectionView>>
+		{
+			public int CacheMissCount { get; set; }
+
+			public CacheMissCountingDelegate(ItemsViewLayout itemsViewLayout, ItemsViewController<CacheTestCollectionView> itemsViewController) : base(itemsViewLayout, itemsViewController)
+			{
+			}
+
+			public override CGSize GetSizeForItem(UICollectionView collectionView, UICollectionViewLayout layout, NSIndexPath indexPath)
+			{
+				var itemSize = base.GetSizeForItem(collectionView, layout, indexPath);
+
+				if (ViewController.Layout is UICollectionViewFlowLayout flowLayout)
+				{
+					// This is totally a cheat from a unit-testing perspective; we know how the item size cache
+					// functions internally (that it will return the estimated size if the item size is not found in the cache),
+					// and we're relying on that for this test. Normally we wouldn't do this, but we need to ensure that further
+					// code changes don't break the cache again, and the only observable outside consequence is that the scrolling
+					// for the CollectionView becomes "janky". We don't want to rely entirely on human perception of "jankiness" to 
+					// catch this problem. 
+					
+					// If the size comes back as EstimatedItemSize, we'll know that the actual value was not found in the cache.
+
+					if (itemSize == flowLayout.EstimatedItemSize)
+					{
+						CacheMissCount += 1;
+					}
+				}
+
+				return itemSize;
+			}
+		}
+
+		internal class ItemModel
+		{
+			public int Index { get; set; }
+			public double HeightRequest => 40 * (Index + 1);
+		}
+
+		[Fact]
+		public async Task EnsureCellSizesAreCached()
+		{
+			SetupBuilder();
+
+			var collectionView = new CacheTestCollectionView()
+			{
+				// Deliberately choosing a height which will cause rounding issues which can caused pathological 
+				// sizing/layout loops if handled wrong 
+				HeightRequest = 654.66666
+			};
+
+			var template = new DataTemplate(() => {
+				var content = new Label() { Text = "Howdy" };
+				content.SetBinding(VisualElement.HeightRequestProperty, new Binding(nameof(VisualElement.HeightRequest)));
+				return content;
+			});
+
+			// Build up a view model that's got enough items to ensure scrolling and template reuse
+			int itemCount = 15;
+			var source = new List<ItemModel>();
+			for (int n = 0; n < itemCount; n++)
+			{
+				source.Add(new ItemModel() { Index = n });
+			}
+
+			collectionView.ItemTemplate = template;
+			collectionView.ItemsSource = source;
+
+			var frame = collectionView.Frame;
+
+			await CreateHandlerAndAddToWindow<CacheTestCollectionViewHandler>(collectionView, async handler =>
+			{
+				// Wait until the CollectionView is actually rendering
+				await WaitForUIUpdate(frame, collectionView);
+
+				// Tell it to scroll to the bottom (and give it time to do so)
+				collectionView.ScrollTo(itemCount - 1);
+				await Task.Delay(1000);
+
+				// Now back to the top				
+				collectionView.ScrollTo(0);
+				await Task.Delay(1000);
+
+				if (handler.Controller is CacheTestItemsViewController controller 
+					&& controller.DelegateFlowLayout is CacheMissCountingDelegate cacheMissCounter)
+				{
+					// Different screen sizes and timings mean this isn't 100% predictable. But we can work out some conditions
+					// which will tell us that the cache is completely broken and test for those.
+
+					// With 15 items in the list, we can assume a minimum of 15 size cache misses until the cache is populated.
+					// Plus at least one for the initial proxy item measurement. 
+
+					// The bugs we are trying to avoid clear the cache prematurely and cause _every_ GetSizeForItem call
+					// to miss the cache, so scrolling top to bottom and back with 15 items we would see at _least_ 30 
+					// cache misses, likely more (since item sizes will be retrieved more than once as items scroll). 
+
+					// If we have fewer than 20 cache misses, we at least know that the cache isn't being wiped as we scroll.
+					int missCountThreshold = 20;
+					int actualMissCount = cacheMissCounter.CacheMissCount;
+					Assert.True(actualMissCount < missCountThreshold, $"Cache miss count {actualMissCount} was higher than the threshold value of {missCountThreshold}");
+				}
+				else
+				{
+					// Something went wrong with this test
+					Assert.Fail("Wrong controller type in the test; is the handler registration broken?");
+				}
+			});
+		}
+	}
+#endif
+}

src/Controls/tests/DeviceTests/Elements/CollectionView/CollectionViewTests.cs

@@ -34,6 +34,10 @@ namespace Microsoft.Maui.DeviceTests
 					handlers.AddHandler<VerticalStackLayout, LayoutHandler>();
 					handlers.AddHandler<Grid, LayoutHandler>();
 					handlers.AddHandler<Label, LabelHandler>();
+
+#if IOS && !MACCATALYST
+					handlers.AddHandler<CacheTestCollectionView, CacheTestCollectionViewHandler>();
+#endif
 				});
 			});
 		}

src/Controls/tests/DeviceTests/Elements/CollectionView/CollectionViewTests.iOS.cs

@@ -1,9 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
-using System.Reflection;
 using System.Threading.Tasks;
-using System.Windows.Input;
 using CoreGraphics;
 using Microsoft.Maui.Controls;
 using Microsoft.Maui.Controls.Handlers.Items;