From 056e30f8e0be078c22bd8126b1cc60d5e87adebc Mon Sep 17 00:00:00 2001 From: Thomas Mijieux Date: Mon, 12 Jul 2021 11:04:08 +0200 Subject: [PATCH] fix memory leak (and some crashes) in ItemsViewController (iOS) (#14111) * add unit test * fix memory leak (and some crashes) in ItemsViewController (iOS) * Update ItemsViewController.cs Co-authored-by: Rui Marinho --- .../ItemsViewControllerLeakTest.cs | 140 ++++++++++++++++++ ...amarin.Forms.Platform.iOS.UnitTests.csproj | 1 + .../CollectionView/ItemsViewController.cs | 4 +- 3 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 Xamarin.Forms.Platform.iOS.UnitTests/ItemsViewControllerLeakTest.cs diff --git a/Xamarin.Forms.Platform.iOS.UnitTests/ItemsViewControllerLeakTest.cs b/Xamarin.Forms.Platform.iOS.UnitTests/ItemsViewControllerLeakTest.cs new file mode 100644 index 00000000000..50efcbe4fba --- /dev/null +++ b/Xamarin.Forms.Platform.iOS.UnitTests/ItemsViewControllerLeakTest.cs @@ -0,0 +1,140 @@ +using System.Collections.Generic; +using System.Collections.Specialized; +using System.Runtime.CompilerServices; +using System.Threading.Tasks; +using NUnit.Framework; + +namespace Xamarin.Forms.Platform.iOS.UnitTests +{ + + class MyTestAdapter : List, INotifyCollectionChanged + { + public event NotifyCollectionChangedEventHandler CollectionChanged; + + public int NumberOfListener { get => CollectionChanged?.GetInvocationList().Length ?? 0; } + public bool ExceptionHappened { get; private set; } = false; + + public new void Add(int item) + { + base.Add(item); + OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item)); + } + + public new void Clear() + { + base.Clear(); + OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); + } + + private void OnCollectionChanged(NotifyCollectionChangedEventArgs args) + { + if (CollectionChanged != null) + { + try + { + CollectionChanged.Invoke(this, args); + } + catch (System.Exception) + { + ExceptionHappened = true; + } + } + } + } + + class MyTestViewModel : System.ComponentModel.INotifyPropertyChanged + { + private MyTestAdapter _myTestAdapter; + public MyTestAdapter MyTestAdapter + { + get => _myTestAdapter; + set => RaiseAndSetIfChanged(ref _myTestAdapter, value); + } + + public event System.ComponentModel.PropertyChangedEventHandler PropertyChanged; + + protected void OnPropertyChanged([CallerMemberName] string propertyName = null) + { + PropertyChanged?.Invoke(this, new System.ComponentModel.PropertyChangedEventArgs(propertyName)); + } + + protected void RaiseAndSetIfChanged(ref T backingField, T value, [CallerMemberName] string propertyName = null) + { + if (!EqualityComparer.Default.Equals(backingField, value)) + { + backingField = value; + OnPropertyChanged(propertyName); + } + } + } + + + [TestFixture] + public class ItemsViewControllerLeakTest : PlatformTestFixture + { + [Test, Category("ItemsView")] + public async Task ItemsViewControllerDoesNotLeakAsync() + { + var myAdapter1 = new MyTestAdapter(); + var myAdapter2 = new MyTestAdapter(); + var vm = new MyTestViewModel { MyTestAdapter = myAdapter1 }; + + var view = new CollectionView + { + ItemTemplate = new DataTemplate(() => new ContentView()), + BindingContext = vm + }; + view.SetBinding(ItemsView.ItemsSourceProperty, nameof(MyTestViewModel.MyTestAdapter)); + await GetRendererProperty(view, ver => ver.NativeView, requiresLayout: true); + + await Device.InvokeOnMainThreadAsync(() => + { + vm.MyTestAdapter = myAdapter2; + vm.MyTestAdapter = myAdapter1; + vm.MyTestAdapter = myAdapter2; + vm.MyTestAdapter = myAdapter1; + + myAdapter1.Add(1); + }); + Assert.That(myAdapter1.NumberOfListener, Is.EqualTo(1)); + Assert.That(myAdapter2.NumberOfListener, Is.EqualTo(0)); + } + + [Test, Category("ItemsView")] + public async Task ItemsViewControllerDoesNotCrashAsync() + { + var myAdapter = new MyTestAdapter(); + var view1 = new CollectionView + { + BindingContext = new MyTestViewModel { MyTestAdapter = myAdapter } + }; + view1.SetBinding(ItemsView.ItemsSourceProperty, nameof(MyTestViewModel.MyTestAdapter)); + await GetRendererProperty(view1, ver => ver.NativeView, requiresLayout: true); + + await Device.InvokeOnMainThreadAsync(() => + { + myAdapter.Add(1); + myAdapter.Clear(); + view1.BindingContext = null; + }); + + var view2 = new CollectionView + { + BindingContext = new MyTestViewModel { MyTestAdapter = myAdapter } + }; + + view2.SetBinding(ItemsView.ItemsSourceProperty, nameof(MyTestViewModel.MyTestAdapter)); + await GetRendererProperty(view2, (ver) => ver.NativeView, requiresLayout: true); + + + await Device.InvokeOnMainThreadAsync(() => + { + myAdapter.Add(2); + myAdapter.Clear(); + view2.BindingContext = null; + }); + + Assert.That(myAdapter.ExceptionHappened, Is.EqualTo(false)); + } + } +} \ No newline at end of file diff --git a/Xamarin.Forms.Platform.iOS.UnitTests/Xamarin.Forms.Platform.iOS.UnitTests.csproj b/Xamarin.Forms.Platform.iOS.UnitTests/Xamarin.Forms.Platform.iOS.UnitTests.csproj index 52c2a58446e..00fea7f00e4 100644 --- a/Xamarin.Forms.Platform.iOS.UnitTests/Xamarin.Forms.Platform.iOS.UnitTests.csproj +++ b/Xamarin.Forms.Platform.iOS.UnitTests/Xamarin.Forms.Platform.iOS.UnitTests.csproj @@ -48,6 +48,7 @@ + diff --git a/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewController.cs b/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewController.cs index 076b29176b7..89f2eab2401 100644 --- a/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewController.cs +++ b/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewController.cs @@ -13,6 +13,7 @@ public abstract class ItemsViewController : UICollectionViewControll public const int EmptyTag = 333; public IItemsViewSource ItemsSource { get; protected set; } + public TItemsView ItemsView { get; } protected ItemsViewLayout ItemsViewLayout { get; set; } bool _initialized; @@ -209,6 +210,7 @@ public virtual void UpdateItemsSource() { _measurementCells.Clear(); ItemsViewLayout?.ClearCellSizeCache(); + ItemsSource?.Dispose(); ItemsSource = CreateItemsViewSource(); CollectionView.ReloadData(); CollectionView.CollectionViewLayout.InvalidateLayout(); @@ -403,7 +405,7 @@ protected void OnFormsElementMeasureInvalidated(object sender, EventArgs e) protected virtual void HandleFormsElementMeasureInvalidated(VisualElement formsElement) { RemeasureLayout(formsElement); - } + } internal void UpdateView(object view, DataTemplate viewTemplate, ref UIView uiView, ref VisualElement formsElement) {