From fe9c697bb6926854edb7b3eaf1fd406fc360ae37 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 30 Mar 2023 16:33:56 -0500 Subject: [PATCH 1/2] [controls] fix memory leak in `CollectionView` Fixes: https://github.com/dotnet/maui/issues/10578 Context: https://github.com/Vroomer/MAUI-navigation-memory-leak.git After testing the above sample, I found that adding a `CollectionView` to a `Page`, makes it and the entire page live forever. Android & Windows: * `MauiRecyclerView` and `StructuredItemsViewHandler` respectively subscribed to `ItemsLayout.PropertyChanged`. This kept the `CollectionView` alive -> all the way up to the `Page`. * Switched to using `WeakNotifyPropertyChangedProxy` solved the issue for these two platforms. iOS: * Generally had a small "nest" of circular references. Initially, I saw `CollectionView`, `UICollectionView`, and various helper classes that would live forever. * `ItemsViewController : UICollectionViewController` -> * `ObservableItemsSource` -> * `UICollectionViewController` and `UICollectionView` * `UICollectionView` -> * `ItemsViewLayout : UICollectionViewFlowLayout` -> * `Func` -> * `ItemsViewController : UICollectionViewController` -> * `UICollectionView` I switched to using `WeakReference` to break the circular references. This required several null checks, where `null` references were not possible before. After these changes my tests pass, yay! --- .../Items/Android/MauiRecyclerView.cs | 17 ++++--- .../StructuredItemsViewHandler.Windows.cs | 21 +++++++-- .../Items/iOS/CarouselViewDelegator.cs | 8 ++-- .../Items/iOS/GroupableItemsViewDelegator.cs | 6 +-- .../Handlers/Items/iOS/ItemsViewController.cs | 4 +- .../Handlers/Items/iOS/ItemsViewDelegator.cs | 24 +++++++--- .../Handlers/Items/iOS/ItemsViewLayout.cs | 7 ++- .../Items/iOS/LoopObservableItemsSource.cs | 7 ++- .../Items/iOS/ObservableItemsSource.cs | 44 ++++++++++++------- .../iOS/ReorderableItemsViewDelegator.cs | 2 +- .../net-android/PublicAPI.Unshipped.txt | 1 + .../net-windows/PublicAPI.Unshipped.txt | 1 + .../NavigationPage/NavigationPageTests.cs | 9 +++- .../DeviceTests/Elements/Shell/ShellTests.cs | 9 +++- 14 files changed, 114 insertions(+), 46 deletions(-) diff --git a/src/Controls/src/Core/Handlers/Items/Android/MauiRecyclerView.cs b/src/Controls/src/Core/Handlers/Items/Android/MauiRecyclerView.cs index 95d40474a355..a481c4bf12bb 100644 --- a/src/Controls/src/Core/Handlers/Items/Android/MauiRecyclerView.cs +++ b/src/Controls/src/Core/Handlers/Items/Android/MauiRecyclerView.cs @@ -42,6 +42,10 @@ public class MauiRecyclerView : Recycler ItemTouchHelper _itemTouchHelper; SimpleItemTouchHelperCallback _itemTouchHelperCallback; + WeakNotifyPropertyChangedProxy _layoutPropertyChangedProxy; + PropertyChangedEventHandler _layoutPropertyChanged; + + ~MauiRecyclerView() => _layoutPropertyChangedProxy?.Unsubscribe(); public MauiRecyclerView(Context context, Func getItemsLayout, Func getAdapter) : base(context) { @@ -58,9 +62,10 @@ public MauiRecyclerView(Context context, Func getItemsLayout, Func public virtual void TearDownOldElement(TItemsView oldElement) { // Stop listening for layout property changes - if (ItemsLayout != null) + if (_layoutPropertyChangedProxy is not null) { - ItemsLayout.PropertyChanged -= LayoutPropertyChanged; + _layoutPropertyChangedProxy.Unsubscribe(); + _layoutPropertyChanged = null; } // Stop listening for ScrollTo requests @@ -283,14 +288,16 @@ public virtual void UpdateCanReorderItems() public virtual void UpdateLayoutManager() { - if (ItemsLayout != null) - ItemsLayout.PropertyChanged -= LayoutPropertyChanged; + _layoutPropertyChangedProxy?.Unsubscribe(); ItemsLayout = _getItemsLayout(); // Keep track of the ItemsLayout's property changes if (ItemsLayout != null) - ItemsLayout.PropertyChanged += LayoutPropertyChanged; + { + _layoutPropertyChanged ??= LayoutPropertyChanged; + _layoutPropertyChangedProxy = new WeakNotifyPropertyChangedProxy(ItemsLayout, _layoutPropertyChanged); + } SetLayoutManager(SelectLayoutManager(ItemsLayout)); diff --git a/src/Controls/src/Core/Handlers/Items/StructuredItemsViewHandler.Windows.cs b/src/Controls/src/Core/Handlers/Items/StructuredItemsViewHandler.Windows.cs index 581bda96db9c..2e8cc1c4683d 100644 --- a/src/Controls/src/Core/Handlers/Items/StructuredItemsViewHandler.Windows.cs +++ b/src/Controls/src/Core/Handlers/Items/StructuredItemsViewHandler.Windows.cs @@ -16,6 +16,10 @@ public partial class StructuredItemsViewHandler : ItemsViewHandler _layoutPropertyChangedProxy?.Unsubscribe(); protected override IItemsLayout Layout { get => ItemsView?.ItemsLayout; } @@ -24,15 +28,26 @@ protected override void ConnectHandler(ListViewBase platformView) base.ConnectHandler(platformView); if (Layout is not null) - Layout.PropertyChanged += LayoutPropertyChanged; + { + _layoutPropertyChanged ??= LayoutPropertyChanged; + _layoutPropertyChangedProxy = new WeakNotifyPropertyChangedProxy(Layout, _layoutPropertyChanged); + } + else if (_layoutPropertyChangedProxy is not null) + { + _layoutPropertyChangedProxy.Unsubscribe(); + _layoutPropertyChangedProxy = null; + } } protected override void DisconnectHandler(ListViewBase platformView) { base.DisconnectHandler(platformView); - if (Layout is not null) - Layout.PropertyChanged -= LayoutPropertyChanged; + if (_layoutPropertyChangedProxy is not null) + { + _layoutPropertyChangedProxy.Unsubscribe(); + _layoutPropertyChangedProxy = null; + } } void LayoutPropertyChanged(object sender, PropertyChangedEventArgs e) diff --git a/src/Controls/src/Core/Handlers/Items/iOS/CarouselViewDelegator.cs b/src/Controls/src/Core/Handlers/Items/iOS/CarouselViewDelegator.cs index eb3ec58dedf0..6675a0624737 100644 --- a/src/Controls/src/Core/Handlers/Items/iOS/CarouselViewDelegator.cs +++ b/src/Controls/src/Core/Handlers/Items/iOS/CarouselViewDelegator.cs @@ -49,11 +49,11 @@ protected override (bool VisibleItems, int First, int Center, int Last) GetVisib { var (VisibleItems, First, Center, Last) = GetVisibleItemsIndexPath(); int firstVisibleItemIndex = -1, centerItemIndex = -1, lastVisibleItemIndex = -1; - if (VisibleItems) + if (VisibleItems && ViewController is CarouselViewController vc) { - firstVisibleItemIndex = ViewController.GetIndexFromIndexPath(First); - centerItemIndex = ViewController.GetIndexFromIndexPath(Center); - lastVisibleItemIndex = ViewController.GetIndexFromIndexPath(Last); + firstVisibleItemIndex = vc.GetIndexFromIndexPath(First); + centerItemIndex = vc.GetIndexFromIndexPath(Center); + lastVisibleItemIndex = vc.GetIndexFromIndexPath(Last); } return (VisibleItems, firstVisibleItemIndex, centerItemIndex, lastVisibleItemIndex); } diff --git a/src/Controls/src/Core/Handlers/Items/iOS/GroupableItemsViewDelegator.cs b/src/Controls/src/Core/Handlers/Items/iOS/GroupableItemsViewDelegator.cs index 66a1fe417c19..c8855267892b 100644 --- a/src/Controls/src/Core/Handlers/Items/iOS/GroupableItemsViewDelegator.cs +++ b/src/Controls/src/Core/Handlers/Items/iOS/GroupableItemsViewDelegator.cs @@ -17,12 +17,12 @@ public GroupableItemsViewDelegator(ItemsViewLayout itemsViewLayout, TViewControl public override CGSize GetReferenceSizeForHeader(UICollectionView collectionView, UICollectionViewLayout layout, nint section) { - return ViewController.GetReferenceSizeForHeader(collectionView, layout, section); + return ViewController?.GetReferenceSizeForHeader(collectionView, layout, section) ?? CGSize.Empty; } public override CGSize GetReferenceSizeForFooter(UICollectionView collectionView, UICollectionViewLayout layout, nint section) { - return ViewController.GetReferenceSizeForFooter(collectionView, layout, section); + return ViewController?.GetReferenceSizeForFooter(collectionView, layout, section) ?? CGSize.Empty; } public override void ScrollAnimationEnded(UIScrollView scrollView) @@ -37,7 +37,7 @@ public override UIEdgeInsets GetInsetForSection(UICollectionView collectionView, return default; } - return ViewController.GetInsetForSection(ItemsViewLayout, collectionView, section); + return ViewController?.GetInsetForSection(ItemsViewLayout, collectionView, section) ?? UIEdgeInsets.Zero; } } } \ No newline at end of file diff --git a/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewController.cs b/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewController.cs index b901675386b4..73d34a37c26d 100644 --- a/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewController.cs +++ b/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewController.cs @@ -30,6 +30,7 @@ public abstract class ItemsViewController : UICollectionViewControll bool _emptyViewDisplayed; bool _disposed; + Func _getPrototype; UIView _emptyUIView; VisualElement _emptyViewFormsElement; Dictionary _measurementCells = new Dictionary(); @@ -200,7 +201,8 @@ void EnsureLayoutInitialized() _initialized = true; - ItemsViewLayout.GetPrototype = GetPrototype; + _getPrototype ??= GetPrototype; + ItemsViewLayout.GetPrototype = _getPrototype; Delegator = CreateDelegator(); CollectionView.Delegate = Delegator; diff --git a/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewDelegator.cs b/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewDelegator.cs index 9e4a6aa95445..a4888f30e420 100644 --- a/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewDelegator.cs +++ b/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewDelegator.cs @@ -12,15 +12,17 @@ public class ItemsViewDelegator : UICollectionViewD where TItemsView : ItemsView where TViewController : ItemsViewController { + readonly WeakReference _viewController; + public ItemsViewLayout ItemsViewLayout { get; } - public TViewController ViewController { get; } + public TViewController ViewController => _viewController.TryGetTarget(out var vc) ? vc : null; protected float PreviousHorizontalOffset, PreviousVerticalOffset; public ItemsViewDelegator(ItemsViewLayout itemsViewLayout, TViewController itemsViewController) { ItemsViewLayout = itemsViewLayout; - ViewController = itemsViewController; + _viewController = new(itemsViewController); } public override void Scrolled(UIScrollView scrollView) @@ -45,8 +47,12 @@ public override void Scrolled(UIScrollView scrollView) LastVisibleItemIndex = lastVisibleItemIndex }; - var itemsView = ViewController.ItemsView; - var source = ViewController.ItemsSource; + var viewController = ViewController; + if (viewController is null) + return; + + var itemsView = viewController.ItemsView; + var source = viewController.ItemsSource; itemsView.SendScrolled(itemsViewScrolledEventArgs); PreviousHorizontalOffset = (float)contentOffsetX; @@ -119,7 +125,11 @@ public override void CellDisplayingEnded(UICollectionView collectionView, UIColl protected virtual (bool VisibleItems, NSIndexPath First, NSIndexPath Center, NSIndexPath Last) GetVisibleItemsIndexPath() { - var indexPathsForVisibleItems = ViewController.CollectionView.IndexPathsForVisibleItems.OrderBy(x => x.Row).ToList(); + var collectionView = ViewController?.CollectionView; + if (collectionView is null) + return default; + + var indexPathsForVisibleItems = collectionView.IndexPathsForVisibleItems.OrderBy(x => x.Row).ToList(); var visibleItems = indexPathsForVisibleItems.Count > 0; NSIndexPath firstVisibleItemIndex = null, centerItemIndex = null, lastVisibleItemIndex = null; @@ -127,7 +137,7 @@ protected virtual (bool VisibleItems, NSIndexPath First, NSIndexPath Center, NSI if (visibleItems) { firstVisibleItemIndex = indexPathsForVisibleItems.First(); - centerItemIndex = GetCenteredIndexPath(ViewController.CollectionView); + centerItemIndex = GetCenteredIndexPath(collectionView); lastVisibleItemIndex = indexPathsForVisibleItems.Last(); } @@ -166,7 +176,7 @@ static NSIndexPath GetCenteredIndexPath(UICollectionView collectionView) public override CGSize GetSizeForItem(UICollectionView collectionView, UICollectionViewLayout layout, NSIndexPath indexPath) { - return ViewController.GetSizeForItem(indexPath); + return ViewController?.GetSizeForItem(indexPath) ?? CGSize.Empty; } } } \ No newline at end of file diff --git a/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewLayout.cs b/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewLayout.cs index 6bd0d0267155..59428bed0d62 100644 --- a/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewLayout.cs +++ b/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewLayout.cs @@ -19,6 +19,7 @@ public abstract class ItemsViewLayout : UICollectionViewFlowLayout CGSize _adjustmentSize0; CGSize _adjustmentSize1; CGSize _currentSize; + WeakReference> _getPrototype; const double ConstraintSizeTolerance = 0.00001; @@ -28,7 +29,11 @@ public abstract class ItemsViewLayout : UICollectionViewFlowLayout public nfloat ConstrainedDimension { get; set; } - public Func GetPrototype { get; set; } + public Func GetPrototype + { + get => _getPrototype is not null && _getPrototype.TryGetTarget(out var func) ? func : null; + set => _getPrototype = new(value); + } internal ItemSizingStrategy ItemSizingStrategy { get; private set; } diff --git a/src/Controls/src/Core/Handlers/Items/iOS/LoopObservableItemsSource.cs b/src/Controls/src/Core/Handlers/Items/iOS/LoopObservableItemsSource.cs index 73771374cb8e..705e663c4a17 100644 --- a/src/Controls/src/Core/Handlers/Items/iOS/LoopObservableItemsSource.cs +++ b/src/Controls/src/Core/Handlers/Items/iOS/LoopObservableItemsSource.cs @@ -1,4 +1,5 @@ #nullable disable +using System; using System.Collections; using Foundation; using ObjCRuntime; @@ -26,8 +27,12 @@ protected override NSIndexPath[] CreateIndexesFrom(int startIndex, int count) return base.CreateIndexesFrom(startIndex, count); } + var collectionView = CollectionView; + if (collectionView == null) + return Array.Empty(); + return IndexPathHelpers.GenerateLoopedIndexPathRange(Section, - (int)CollectionView.NumberOfItemsInSection(Section), LoopBy, startIndex, count); + (int)collectionView.NumberOfItemsInSection(Section), LoopBy, startIndex, count); } } } diff --git a/src/Controls/src/Core/Handlers/Items/iOS/ObservableItemsSource.cs b/src/Controls/src/Core/Handlers/Items/iOS/ObservableItemsSource.cs index c158753830e8..fa25e3708c61 100644 --- a/src/Controls/src/Core/Handlers/Items/iOS/ObservableItemsSource.cs +++ b/src/Controls/src/Core/Handlers/Items/iOS/ObservableItemsSource.cs @@ -10,8 +10,7 @@ namespace Microsoft.Maui.Controls.Handlers.Items { internal class ObservableItemsSource : IObservableItemsViewSource { - readonly UICollectionViewController _collectionViewController; - protected readonly UICollectionView CollectionView; + readonly WeakReference _collectionViewController; readonly bool _grouped; readonly int _section; readonly IEnumerable _itemsSource; @@ -19,8 +18,7 @@ internal class ObservableItemsSource : IObservableItemsViewSource public ObservableItemsSource(IEnumerable itemSource, UICollectionViewController collectionViewController, int group = -1) { - _collectionViewController = collectionViewController; - CollectionView = _collectionViewController.CollectionView; + _collectionViewController = new(collectionViewController); _section = group < 0 ? 0 : group; _grouped = group >= 0; @@ -35,6 +33,8 @@ public ObservableItemsSource(IEnumerable itemSource, UICollectionViewController internal event NotifyCollectionChangedEventHandler CollectionViewUpdating; internal event NotifyCollectionChangedEventHandler CollectionViewUpdated; + internal UICollectionView CollectionView => _collectionViewController.TryGetTarget(out var controller) ? controller.CollectionView : null; + public int Count { get; private set; } public int Section => _section; @@ -125,9 +125,13 @@ void CollectionChanged(object sender, NotifyCollectionChangedEventArgs args) void CollectionChanged(NotifyCollectionChangedEventArgs args) { + if (!_collectionViewController.TryGetTarget(out var controller)) + return; + // Force UICollectionView to get the internal accounting straight - if (!CollectionView.Hidden) - CollectionView.NumberOfItemsInSection(_section); + var collectionView = controller.CollectionView; + if (!collectionView.Hidden) + collectionView.NumberOfItemsInSection(_section); switch (args.Action) { @@ -153,14 +157,18 @@ void CollectionChanged(NotifyCollectionChangedEventArgs args) void Reload() { + if (!_collectionViewController.TryGetTarget(out var controller)) + return; + var args = new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset); Count = ItemsCount(); OnCollectionViewUpdating(args); - CollectionView.ReloadData(); - CollectionView.CollectionViewLayout.InvalidateLayout(); + var collectionView = controller.CollectionView; + collectionView.ReloadData(); + collectionView.CollectionViewLayout.InvalidateLayout(); OnCollectionViewUpdated(args); } @@ -177,7 +185,7 @@ void Add(NotifyCollectionChangedEventArgs args) var startIndex = args.NewStartingIndex > -1 ? args.NewStartingIndex : IndexOf(args.NewItems[0]); // Queue up the updates to the UICollectionView - Update(() => CollectionView.InsertItems(CreateIndexesFrom(startIndex, count)), args); + Update(c => c.InsertItems(CreateIndexesFrom(startIndex, count)), args); } void Remove(NotifyCollectionChangedEventArgs args) @@ -196,7 +204,7 @@ void Remove(NotifyCollectionChangedEventArgs args) var count = args.OldItems.Count; Count -= count; - Update(() => CollectionView.DeleteItems(CreateIndexesFrom(startIndex, count)), args); + Update(c => c.DeleteItems(CreateIndexesFrom(startIndex, count)), args); } void Replace(NotifyCollectionChangedEventArgs args) @@ -209,7 +217,7 @@ void Replace(NotifyCollectionChangedEventArgs args) // We are replacing one set of items with a set of equal size; we can do a simple item range update - Update(() => CollectionView.ReloadItems(CreateIndexesFrom(startIndex, newCount)), args); + Update(c => c.ReloadItems(CreateIndexesFrom(startIndex, newCount)), args); return; } @@ -228,14 +236,14 @@ void Move(NotifyCollectionChangedEventArgs args) var oldPath = NSIndexPath.Create(_section, args.OldStartingIndex); var newPath = NSIndexPath.Create(_section, args.NewStartingIndex); - Update(() => CollectionView.MoveItem(oldPath, newPath), args); + Update(c => c.MoveItem(oldPath, newPath), args); return; } var start = Math.Min(args.OldStartingIndex, args.NewStartingIndex); var end = Math.Max(args.OldStartingIndex, args.NewStartingIndex) + count; - Update(() => CollectionView.ReloadItems(CreateIndexesFrom(start, end)), args); + Update(c => c.ReloadItems(CreateIndexesFrom(start, end)), args); } internal int ItemsCount() @@ -281,15 +289,19 @@ internal int IndexOf(object item) return -1; } - void Update(Action update, NotifyCollectionChangedEventArgs args) + void Update(Action update, NotifyCollectionChangedEventArgs args) { - if (CollectionView.Hidden) + if (!_collectionViewController.TryGetTarget(out var controller)) + return; + + var collectionView = controller.CollectionView; + if (collectionView.Hidden) { return; } OnCollectionViewUpdating(args); - update(); + update(collectionView); OnCollectionViewUpdated(args); } diff --git a/src/Controls/src/Core/Handlers/Items/iOS/ReorderableItemsViewDelegator.cs b/src/Controls/src/Core/Handlers/Items/iOS/ReorderableItemsViewDelegator.cs index e2c19bc4f011..2d680eea97b5 100644 --- a/src/Controls/src/Core/Handlers/Items/iOS/ReorderableItemsViewDelegator.cs +++ b/src/Controls/src/Core/Handlers/Items/iOS/ReorderableItemsViewDelegator.cs @@ -19,7 +19,7 @@ public override NSIndexPath GetTargetIndexPathForMove(UICollectionView collectio { NSIndexPath targetIndexPath; - var itemsView = ViewController.ItemsView; + var itemsView = ViewController?.ItemsView; if (itemsView?.IsGrouped == true) { if (originalIndexPath.Section == proposedIndexPath.Section || itemsView.CanMixGroups) diff --git a/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt index 39f6866a6601..a1fc5d0652d1 100644 --- a/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt @@ -3,6 +3,7 @@ Microsoft.Maui.Controls.Border.~Border() -> void Microsoft.Maui.Controls.Handlers.Compatibility.FrameRenderer.FrameRenderer(Android.Content.Context! context, Microsoft.Maui.IPropertyMapper! mapper) -> void Microsoft.Maui.Controls.Handlers.Compatibility.FrameRenderer.FrameRenderer(Android.Content.Context! context, Microsoft.Maui.IPropertyMapper! mapper, Microsoft.Maui.CommandMapper! commandMapper) -> void +Microsoft.Maui.Controls.Handlers.Items.MauiRecyclerView.~MauiRecyclerView() -> void Microsoft.Maui.Controls.LayoutOptions.Equals(Microsoft.Maui.Controls.LayoutOptions other) -> bool Microsoft.Maui.Controls.Region.Equals(Microsoft.Maui.Controls.Region other) -> bool Microsoft.Maui.Controls.Shapes.Matrix.Equals(Microsoft.Maui.Controls.Shapes.Matrix other) -> bool diff --git a/src/Controls/src/Core/PublicAPI/net-windows/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net-windows/PublicAPI.Unshipped.txt index df99564a9206..52e2575dedb7 100644 --- a/src/Controls/src/Core/PublicAPI/net-windows/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net-windows/PublicAPI.Unshipped.txt @@ -1,6 +1,7 @@ #nullable enable *REMOVED*override Microsoft.Maui.Controls.RefreshView.MeasureOverride(double widthConstraint, double heightConstraint) -> Microsoft.Maui.Graphics.Size Microsoft.Maui.Controls.Border.~Border() -> void +Microsoft.Maui.Controls.Handlers.Items.StructuredItemsViewHandler.~StructuredItemsViewHandler() -> void override Microsoft.Maui.Controls.Handlers.BoxViewHandler.NeedsContainer.get -> bool Microsoft.Maui.Controls.Handlers.BoxViewHandler Microsoft.Maui.Controls.Handlers.BoxViewHandler.BoxViewHandler() -> void diff --git a/src/Controls/tests/DeviceTests/Elements/NavigationPage/NavigationPageTests.cs b/src/Controls/tests/DeviceTests/Elements/NavigationPage/NavigationPageTests.cs index 9f6ec7ba276d..14630441f4dd 100644 --- a/src/Controls/tests/DeviceTests/Elements/NavigationPage/NavigationPageTests.cs +++ b/src/Controls/tests/DeviceTests/Elements/NavigationPage/NavigationPageTests.cs @@ -5,6 +5,7 @@ using System.Threading.Tasks; using Microsoft.Maui.Controls; using Microsoft.Maui.Controls.Handlers.Compatibility; +using Microsoft.Maui.Controls.Handlers.Items; using Microsoft.Maui.DeviceTests.Stubs; using Microsoft.Maui.Handlers; using Microsoft.Maui.Hosting; @@ -35,6 +36,7 @@ void SetupBuilder() handlers.AddHandler(); handlers.AddHandler(); handlers.AddHandler(); + handlers.AddHandler(); }); }); } @@ -306,6 +308,7 @@ await CreateHandlerAndAddToWindow(new Window(navPage), async { new Label(), new Button(), + new CollectionView(), } }; pageReference = new WeakReference(page); @@ -313,9 +316,11 @@ await CreateHandlerAndAddToWindow(new Window(navPage), async await navPage.Navigation.PopAsync(); }); - // 3 GCs were required in Android API 23, 2 worked otherwise - for (int i = 0; i < 3; i++) + // As we add more controls to this test, more GCs will be required + for (int i = 0; i < 16; i++) { + if (!pageReference.IsAlive) + break; await Task.Yield(); GC.Collect(); GC.WaitForPendingFinalizers(); diff --git a/src/Controls/tests/DeviceTests/Elements/Shell/ShellTests.cs b/src/Controls/tests/DeviceTests/Elements/Shell/ShellTests.cs index c69fb54ad79f..76326cc2bb38 100644 --- a/src/Controls/tests/DeviceTests/Elements/Shell/ShellTests.cs +++ b/src/Controls/tests/DeviceTests/Elements/Shell/ShellTests.cs @@ -8,6 +8,7 @@ using Microsoft.Maui.Controls; using Microsoft.Maui.Controls.Handlers; using Microsoft.Maui.Controls.Handlers.Compatibility; +using Microsoft.Maui.Controls.Handlers.Items; using Microsoft.Maui.Devices; using Microsoft.Maui.DeviceTests.Stubs; using Microsoft.Maui.Handlers; @@ -34,6 +35,7 @@ void SetupBuilder() SetupShellHandlers(handlers); handlers.AddHandler(typeof(NavigationPage), typeof(NavigationViewHandler)); handlers.AddHandler(typeof(Button), typeof(ButtonHandler)); + handlers.AddHandler(typeof(CollectionView), typeof(CollectionViewHandler)); }); }); } @@ -975,6 +977,7 @@ await CreateHandlerAndAddToWindow(shell, async (handler) => { new Label(), new Button(), + new CollectionView(), } }; pageReference = new WeakReference(page); @@ -983,9 +986,11 @@ await CreateHandlerAndAddToWindow(shell, async (handler) => await shell.Navigation.PopAsync(); }); - // 3 GCs were required in Android API 23, 2 worked otherwise - for (int i = 0; i < 3; i++) + // As we add more controls to this test, more GCs will be required + for (int i = 0; i < 16; i++) { + if (!pageReference.IsAlive) + break; await Task.Yield(); GC.Collect(); GC.WaitForPendingFinalizers(); From 1cbfe2a955932a7fc6474cdafac9f4d89a17dbe5 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Mon, 10 Apr 2023 22:48:05 -0500 Subject: [PATCH 2/2] `is null` --- .../src/Core/Handlers/Items/iOS/LoopObservableItemsSource.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Controls/src/Core/Handlers/Items/iOS/LoopObservableItemsSource.cs b/src/Controls/src/Core/Handlers/Items/iOS/LoopObservableItemsSource.cs index 705e663c4a17..a060c8f47cd5 100644 --- a/src/Controls/src/Core/Handlers/Items/iOS/LoopObservableItemsSource.cs +++ b/src/Controls/src/Core/Handlers/Items/iOS/LoopObservableItemsSource.cs @@ -28,7 +28,7 @@ protected override NSIndexPath[] CreateIndexesFrom(int startIndex, int count) } var collectionView = CollectionView; - if (collectionView == null) + if (collectionView is null) return Array.Empty(); return IndexPathHelpers.GenerateLoopedIndexPathRange(Section,