diff --git a/src/Controls/src/Core/Handlers/Items/iOS/ObservableGroupedSource.cs b/src/Controls/src/Core/Handlers/Items/iOS/ObservableGroupedSource.cs index 93d36e034e6f..5a1a3125b18e 100644 --- a/src/Controls/src/Core/Handlers/Items/iOS/ObservableGroupedSource.cs +++ b/src/Controls/src/Core/Handlers/Items/iOS/ObservableGroupedSource.cs @@ -164,9 +164,19 @@ void CollectionChanged(NotifyCollectionChangedEventArgs args) if (!_collectionViewController.TryGetTarget(out var controller)) return; - // Force UICollectionView to get the internal accounting straight var collectionView = controller.CollectionView; - UpdateSection(collectionView); + + // Force UICollectionView to get the internal accounting straight. + // Skip for Remove: standard INotifyCollectionChanged semantics require the item to be + // already removed from the backing collection before CollectionChanged fires. This means + // _groupSource has N-1 items when we receive Remove, but UIKit still has N sections. + // Calling UpdateSection here triggers GetGroupCount with UIKit's stale section index + // (N-1), which is out of range on the already-mutated _groupSource → ArgumentOutOfRangeException. + // The post-processing UpdateSection call below handles reconciliation after DeleteSections. + if (args.Action != NotifyCollectionChangedAction.Remove) + { + UpdateSection(collectionView); + } switch (args.Action) { @@ -360,6 +370,12 @@ void Move(NotifyCollectionChangedEventArgs args) int GetGroupCount(int groupIndex) { + // Defensive bounds check: UIKit can call back synchronously during DeleteSections, + // and other re-entrancy scenarios may present a stale groupIndex. Return 0 instead + // of throwing ArgumentOutOfRangeException. + if ((uint)groupIndex >= (uint)_groupSource.Count) + return 0; + switch (_groupSource[groupIndex]) { case IList list: diff --git a/src/Controls/tests/DeviceTests/Elements/CollectionView/CollectionViewTests.iOS.cs b/src/Controls/tests/DeviceTests/Elements/CollectionView/CollectionViewTests.iOS.cs index 1a9dfd28b7ce..cfb18040853f 100644 --- a/src/Controls/tests/DeviceTests/Elements/CollectionView/CollectionViewTests.iOS.cs +++ b/src/Controls/tests/DeviceTests/Elements/CollectionView/CollectionViewTests.iOS.cs @@ -49,6 +49,125 @@ await CreateHandlerAndAddToWindow(collectionView, async h }); } + // Regression tests for https://github.com/dotnet/maui/issues/34691 + // ObservableGroupedSource.CollectionChanged() called UpdateSection() before Remove, + // causing GetGroupCount to access a stale section index → ArgumentOutOfRangeException. + // Each test forces a synchronous UIKit layout pass after removal to exercise the fix. + + [Fact(DisplayName = "Removing last section from grouped CollectionView does not crash (Handler2)")] + public async Task ItemsSourceGroupedRemoveLastSectionDoesNotCrash() + { + SetupBuilder(); + + var data = new List { "item 1", "item 2", "item 3" }; + var groupData = new ObservableCollection + { + new("Header 1", data), + new("Header 2", data), + new("Header 3", data), + }; + + var collectionView = new CollectionView + { + IsGrouped = true, + ItemsSource = groupData, + ItemTemplate = new DataTemplate(() => new Label()), + }; + + var initialFrame = collectionView.Frame; + + await CreateHandlerAndAddToWindow(collectionView, async handler => + { + await WaitForUIUpdate(initialFrame, collectionView); + var uiCollectionView = handler.Controller.CollectionView; + + groupData.RemoveAt(groupData.Count - 1); + + uiCollectionView.SetNeedsLayout(); + uiCollectionView.LayoutIfNeeded(); + + Assert.Equal(groupData.Count, (int)uiCollectionView.NumberOfSections()); + }); + } + + [Fact(DisplayName = "Removing first section from grouped CollectionView does not crash (Handler2)")] + public async Task ItemsSourceGroupedRemoveFirstSectionDoesNotCrash() + { + SetupBuilder(); + + var data = new List { "item 1", "item 2" }; + var groupData = new ObservableCollection + { + new("Header 1", data), + new("Header 2", data), + new("Header 3", data), + }; + + var collectionView = new CollectionView + { + IsGrouped = true, + ItemsSource = groupData, + ItemTemplate = new DataTemplate(() => new Label()), + }; + + var initialFrame = collectionView.Frame; + + await CreateHandlerAndAddToWindow(collectionView, async handler => + { + await WaitForUIUpdate(initialFrame, collectionView); + var uiCollectionView = handler.Controller.CollectionView; + + groupData.RemoveAt(0); + uiCollectionView.SetNeedsLayout(); + uiCollectionView.LayoutIfNeeded(); + Assert.Equal(groupData.Count, (int)uiCollectionView.NumberOfSections()); + + groupData.RemoveAt(0); + uiCollectionView.SetNeedsLayout(); + uiCollectionView.LayoutIfNeeded(); + Assert.Equal(groupData.Count, (int)uiCollectionView.NumberOfSections()); + }); + } + + [Fact(DisplayName = "Removing all sections one by one from grouped CollectionView does not crash (Handler2)")] + public async Task ItemsSourceGroupedRemoveAllSectionsOneByOneDoesNotCrash() + { + SetupBuilder(); + + var data = new List { "item 1", "item 2" }; + var groupData = new ObservableCollection + { + new("Header 1", data), + new("Header 2", data), + new("Header 3", data), + }; + + var collectionView = new CollectionView + { + IsGrouped = true, + ItemsSource = groupData, + ItemTemplate = new DataTemplate(() => new Label()), + }; + + var initialFrame = collectionView.Frame; + + await CreateHandlerAndAddToWindow(collectionView, async handler => + { + await WaitForUIUpdate(initialFrame, collectionView); + var uiCollectionView = handler.Controller.CollectionView; + + while (groupData.Count > 0) + { + groupData.RemoveAt(groupData.Count - 1); + uiCollectionView.SetNeedsLayout(); + uiCollectionView.LayoutIfNeeded(); + Assert.Equal(groupData.Count, (int)uiCollectionView.NumberOfSections()); + } + + Assert.Empty(groupData); + }); + } + class CollectionViewStringGroup : List { public string GroupHeader { get; private set; }