[iOS] Prevent crash when adding item to source of CollectionView in tab (#7940)

* Prevent crash when adding item to source of CollectionView that's never been displayed;
fixes #7700

* Fix renaming error in UI test

* Add test and fix for adding groups offscreen

* Fix rebase error

* Remove flags

* Add instructions

* Better instructions

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue7700.cs

@@ -0,0 +1,165 @@
+using System;
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+using System.Text;
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+
+#if UITEST
+using Xamarin.Forms.Core.UITests;
+using Xamarin.UITest;
+using NUnit.Framework;
+#endif
+
+namespace Xamarin.Forms.Controls.Issues
+{
+#if UITEST
+	[Category(UITestCategories.CollectionView)]
+#endif
+	[Preserve(AllMembers = true)]
+	[Issue(IssueTracker.Github, 7700, "[Bug][iOS] If CollectionView in other Tab gets changed before it's displayed, it stays invisible",
+		PlatformAffected.iOS)]
+	public class Issue7700 : TestTabbedPage
+	{
+		readonly ObservableCollection<string> _source = new ObservableCollection<string>(){ "one", "two", "three" };
+		readonly ObservableCollection<Group> _groupedSource = new ObservableCollection<Group>();
+
+		[Preserve(AllMembers = true)]
+		class Group : List<string>
+		{
+			public string Text { get; set; }
+
+			public Group()
+			{
+				Add("Uno");
+				Add("Dos");
+				Add("Tres");
+			}
+		}
+
+		const string Add1 = "Add1";
+		const string Add2 = "Add2";
+		const string Success = "Success";
+		const string Tab2 = "Tab2";
+		const string Tab3 = "Tab3";
+		const string Add1Label = "Add to List";
+		const string Add2Label = "Add to Grouped List";
+
+		protected override void Init()
+		{
+#if APP
+			Children.Add(FirstPage());
+			Children.Add(CollectionViewPage());
+			Children.Add(GroupedCollectionViewPage());
+#endif
+		}
+
+		ContentPage FirstPage()
+		{
+			var page = new ContentPage() { Title = "7700 First Page", Padding = 40 };
+
+			var instructions = new Label { Text = $"Tap the button marked '{Add1Label}'. Then tap the button marked '{Add2Label}'. If the application does not crash, the test has passed." };
+
+			var button1 = new Button() { Text = Add1Label, AutomationId = Add1 };
+			button1.Clicked += Button1Clicked;
+
+			var button2 = new Button() { Text = Add2Label, AutomationId = Add2 };
+			button2.Clicked += Button2Clicked;
+
+			var layout = new StackLayout { Children = { instructions, button1, button2 } };
+
+			page.Content = layout;
+
+			return page;
+		}
+
+		void Button1Clicked(object sender, EventArgs e)
+		{
+			_source.Insert(0, Success);
+		}
+
+		void Button2Clicked(object sender, EventArgs e)
+		{
+			_groupedSource.Insert(0, new Group() { Text = Success });
+		}
+
+		ContentPage CollectionViewPage()
+		{
+			var cv = new CollectionView
+			{
+				ItemTemplate = new DataTemplate(() =>
+				{
+					var label = new Label();
+					label.SetBinding(Label.TextProperty, new Binding("."));
+					return label;
+				}),
+
+				ItemsSource = _source
+			};
+
+			var page = new ContentPage() { Title = Tab2, Padding = 40 };
+
+			page.Content = cv;
+
+			return page;
+		}
+
+		ContentPage GroupedCollectionViewPage()
+		{
+			var cv = new CollectionView
+			{
+				ItemTemplate = new DataTemplate(() =>
+				{
+					var label = new Label();
+					label.SetBinding(Label.TextProperty, new Binding("."));
+					return label;
+				}),
+
+				GroupHeaderTemplate = new DataTemplate(() =>
+				{
+					var label = new Label();
+					label.SetBinding(Label.TextProperty, new Binding("Text"));
+					return label;
+				}),
+
+				GroupFooterTemplate = new DataTemplate(() =>
+				{
+					var label = new Label();
+					label.SetBinding(Label.TextProperty, new Binding("Text"));
+					return label;
+				}),
+
+				ItemsSource = _groupedSource,
+				IsGrouped = true
+			};
+
+			var page = new ContentPage() { Title = Tab3, Padding = 40 };
+
+			page.Content = cv;
+
+			return page;
+		}
+
+#if UITEST
+		[Test]
+		public void AddingItemToUnviewedCollectionViewShouldNotCrash()
+		{
+			RunningApp.WaitForElement(Add1);
+			RunningApp.Tap(Add1);	
+			RunningApp.Tap(Tab2);		
+
+			RunningApp.WaitForElement(Success);
+		}
+
+		[Test]
+		public void AddingGroupToUnviewedGroupedCollectionViewShouldNotCrash()
+		{
+			RunningApp.WaitForElement(Add2);
+			RunningApp.Tap(Add2);
+			RunningApp.Tap(Tab3);
+
+			RunningApp.WaitForElement(Success);
+		}
+#endif
+	}
+}

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue7993.xaml.cs

@@ -28,8 +28,6 @@ namespace Xamarin.Forms.Controls.Issues
 #if APP
 		public Issue7993()
 		{
-			Device.SetFlags(new List<string> { CollectionView.CollectionViewExperimental });
-
 			InitializeComponent();
 
 			BindingContext = new ViewModel7993();

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems

@@ -59,6 +59,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Issue7519Xaml.xaml.cs">
       <SubType>Code</SubType>
     </Compile>
+    <Compile Include="$(MSBuildThisFileDirectory)Issue7700.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue7758.xaml.cs">
       <SubType>Code</SubType>
     </Compile>

Xamarin.Forms.Platform.iOS/CollectionView/GroupableItemsViewController.cs

@@ -30,7 +30,7 @@ namespace Xamarin.Forms.Platform.iOS
 			// Use the BindableProperty here (instead of _isGroupingEnabled) because the cached value might not be set yet
 			if (ItemsView.IsGrouped) 
 			{
-				return ItemsSourceFactory.CreateGrouped(ItemsView.ItemsSource, CollectionView);
+				return ItemsSourceFactory.CreateGrouped(ItemsView.ItemsSource, this);
 			}
 
 			return base.CreateItemsViewSource();

Xamarin.Forms.Platform.iOS/CollectionView/ItemsSourceFactory.cs

@@ -7,7 +7,7 @@ namespace Xamarin.Forms.Platform.iOS
 {
 	internal static class ItemsSourceFactory
 	{
-		public static IItemsViewSource Create(IEnumerable itemsSource, UICollectionView collectionView)
+		public static IItemsViewSource Create(IEnumerable itemsSource, UICollectionViewController collectionViewController)
 		{
 			if (itemsSource == null)
 			{
@@ -17,21 +17,21 @@ namespace Xamarin.Forms.Platform.iOS
 			switch (itemsSource)
 			{
 				case INotifyCollectionChanged _:
-					return new ObservableItemsSource(itemsSource as IList, collectionView);
+					return new ObservableItemsSource(itemsSource as IList, collectionViewController);
 				case IEnumerable _:
 				default:
 					return new ListSource(itemsSource);
 			}
 		}
 
-		public static IItemsViewSource CreateGrouped(IEnumerable itemsSource, UICollectionView collectionView)
+		public static IItemsViewSource CreateGrouped(IEnumerable itemsSource, UICollectionViewController collectionViewController)
 		{
 			if (itemsSource == null)
 			{
 				return new EmptySource();
 			}
 
-			return new ObservableGroupedSource(itemsSource, collectionView);
+			return new ObservableGroupedSource(itemsSource, collectionViewController);
 		}
 	}
 }

Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewController.cs

@@ -166,7 +166,7 @@ namespace Xamarin.Forms.Platform.iOS
 
 		protected virtual IItemsViewSource CreateItemsViewSource()
 		{
-			return ItemsSourceFactory.Create(ItemsView.ItemsSource, CollectionView);
+			return ItemsSourceFactory.Create(ItemsView.ItemsSource, this);
 		}
 
 		public virtual void UpdateItemsSource()

Xamarin.Forms.Platform.iOS/CollectionView/ListSource.cs

@@ -42,7 +42,7 @@ namespace Xamarin.Forms.Platform.iOS
 			}
 		}
 
-		public int GroupCount => Count == 0 ? 0 : 1;
+		public int GroupCount => 1;
 
 		public int ItemCount => Count;
 

Xamarin.Forms.Platform.iOS/CollectionView/ObservableGroupedSource.cs

@@ -4,19 +4,22 @@ using System.Collections.Generic;
 using System.Collections.Specialized;
 using Foundation;
 using UIKit;
+using Xamarin.Forms.Internals;
 
 namespace Xamarin.Forms.Platform.iOS
 {
 	internal class ObservableGroupedSource : IItemsViewSource
 	{
 		readonly UICollectionView _collectionView;
+		UICollectionViewController _collectionViewController;
 		readonly IList _groupSource;
 		bool _disposed;
 		List<ObservableItemsSource> _groups = new List<ObservableItemsSource>();
 
-		public ObservableGroupedSource(IEnumerable groupSource, UICollectionView collectionView)
+		public ObservableGroupedSource(IEnumerable groupSource, UICollectionViewController collectionViewController)
 		{
-			_collectionView = collectionView;
+			_collectionViewController = collectionViewController;
+			_collectionView = _collectionViewController.CollectionView;
 			_groupSource = groupSource as IList ?? new ListSource(groupSource);
 
 			if (_groupSource is INotifyCollectionChanged incc)
@@ -120,7 +123,7 @@ namespace Xamarin.Forms.Platform.iOS
 			{
 				if (_groupSource[n] is INotifyCollectionChanged && _groupSource[n] is IList list)
 				{
-					_groups.Add(new ObservableItemsSource(list, _collectionView, n));
+					_groups.Add(new ObservableItemsSource(list, _collectionViewController, n));
 				}
 			}
 		}
@@ -161,6 +164,13 @@ namespace Xamarin.Forms.Platform.iOS
 			return NSIndexSet.FromNSRange(new NSRange(startIndex, count));
 		}
 
+		bool NotLoadedYet()
+		{
+			// If the UICollectionView hasn't actually been loaded, then calling InsertSections or DeleteSections is 
+			// going to crash or get in an unusable state; instead, ReloadData should be used
+			return !_collectionViewController.IsViewLoaded || _collectionViewController.View.Window == null;
+		}
+
 		void Add(NotifyCollectionChangedEventArgs args)
 		{
 			var startIndex = args.NewStartingIndex > -1 ? args.NewStartingIndex : _groupSource.IndexOf(args.NewItems[0]);
@@ -170,6 +180,12 @@ namespace Xamarin.Forms.Platform.iOS
 			// is to reset all the group tracking to get it up-to-date
 			ResetGroupTracking();
 
+			if (NotLoadedYet())
+			{
+				_collectionView.ReloadData();
+				return;
+			}
+
 			_collectionView.InsertSections(CreateIndexSetFrom(startIndex, count));
 		}
 
@@ -192,7 +208,14 @@ namespace Xamarin.Forms.Platform.iOS
 			// is to reset all the group tracking to get it up-to-date
 			ResetGroupTracking();
 
-			_collectionView.DeleteSections(CreateIndexSetFrom(startIndex, count));
+			if (NotLoadedYet())
+			{
+				_collectionView.ReloadData();
+			}
+			else
+			{
+				_collectionView.DeleteSections(CreateIndexSetFrom(startIndex, count));
+			}
 		}
 
 		void Replace(NotifyCollectionChangedEventArgs args)

Xamarin.Forms.Platform.iOS/CollectionView/ObservableItemsSource.cs

@@ -8,21 +8,23 @@ namespace Xamarin.Forms.Platform.iOS
 {
 	internal class ObservableItemsSource : IItemsViewSource
 	{
+		readonly UICollectionViewController _collectionViewController;
 		readonly UICollectionView _collectionView;
 		readonly bool _grouped;
 		readonly int _section;
 		readonly IList _itemsSource;
 		bool _disposed;
 
-		public ObservableItemsSource(IList itemSource, UICollectionView collectionView, int group = -1)
+		public ObservableItemsSource(IList itemSource, UICollectionViewController collectionViewController, int group = -1)
 		{
-			_collectionView = collectionView;
+			_collectionViewController = collectionViewController;
+			_collectionView = _collectionViewController.CollectionView;
 		
 			_section = group < 0 ? 0 : group;
 			_grouped = group >= 0;
 
 			_itemsSource = itemSource;
-
+			
 			((INotifyCollectionChanged)itemSource).CollectionChanged += CollectionChanged;
 		}
 
@@ -71,7 +73,7 @@ namespace Xamarin.Forms.Platform.iOS
 			return NSIndexPath.Create(-1, -1);
 		}
 
-		public int GroupCount => _itemsSource.Count == 0 ? 0 : 1;
+		public int GroupCount => 1;
 
 		public int ItemCount => _itemsSource.Count;
 
@@ -130,32 +132,49 @@ namespace Xamarin.Forms.Platform.iOS
 			return result;
 		}
 
+		bool NotLoadedYet()
+		{
+			// If the UICollectionView hasn't actually been loaded, then calling InsertItems or DeleteItems is 
+			// going to crash or get in an unusable state; instead, ReloadData should be used
+			return !_collectionViewController.IsViewLoaded || _collectionViewController.View.Window == null;
+		}
+
 		void Add(NotifyCollectionChangedEventArgs args)
 		{
 			var startIndex = args.NewStartingIndex > -1 ? args.NewStartingIndex : _itemsSource.IndexOf(args.NewItems[0]);
 			var count = args.NewItems.Count;
 
-			if (!_grouped && _collectionView.NumberOfSections() != GroupCount && count > 0)
+			if (NotLoadedYet())
 			{
-				// Okay, we're going from completely empty to more than 0 items; this means we don't even
-				// have a section 0 yet. Inserting a section 0 manually results in an unexplained crash, so instead
-				// we'll just reload the data so the UICollectionView can get its internal state sorted out.
 				_collectionView.ReloadData();
+				return;
 			}
-			else
+
+			if (!_grouped && _collectionView.NumberOfItemsInSection(_section) == 0)
 			{
-				_collectionView.PerformBatchUpdates(() =>
+				// Okay, we're going from completely empty to more than 0 items; there's an iOS bug which apparently
+				// will just crash if we call InsertItems here, so we have to do ReloadData.
+				_collectionView.ReloadData();
+				return;
+			}
+
+			_collectionView.PerformBatchUpdates(() =>
 				{
 					var indexes = CreateIndexesFrom(startIndex, count);
 					_collectionView.InsertItems(indexes);
 				}, null);
-			}
 		}
 
 		void Remove(NotifyCollectionChangedEventArgs args)
 		{
 			var startIndex = args.OldStartingIndex;
 
+			if (NotLoadedYet())
+			{
+				_collectionView.ReloadData();
+				return;
+			}
+
 			if (startIndex < 0)
 			{
 				// INCC implementation isn't giving us enough information to know where the removed items were in the
@@ -163,20 +182,13 @@ namespace Xamarin.Forms.Platform.iOS
 				Reload();
 				return;
 			}
-	
+
 			// If we have a start index, we can be more clever about removing the item(s) (and get the nifty animations)
 			var count = args.OldItems.Count;
 
 			_collectionView.PerformBatchUpdates(() =>
 			{
 				_collectionView.DeleteItems(CreateIndexesFrom(startIndex, count));
-
-				if (!_grouped && _collectionView.NumberOfSections() != GroupCount)
-				{
-					// We had a non-grouped list with items, and we're removing the last one;
-					// we also need to remove the group it was in
-					_collectionView.DeleteSections(new NSIndexSet(0));
-				}
 			}, null);
 		}