[iOS] Failed to marshal HeaderWrapperView (xamarin#10759) fixes xamar…

…in#9711

* Add UI test for xamarin#9711 Failed to marshal HeaderWrapperView

* Fix xamarin#9711 Failed to marshal HeaderWrapperView

Crash appears to be occurring due to the premature disposal of a `HeaderWrapperView` instance.

Crash occurs in `VisualElementRenderer::UpdateParentPageAccessibilityElements()`. As it walks up the superview chain from the `UILabel` that was tapped on in the test, it arrives at a superview whose `NSObject` cannot be retrieved. That superview happens to be a `HeaderWrapperView` instance. In `Runtime.cs:1249`, the call to `TryGetNSObject()` to retrieve this `HeaderWrapperView` returns `null`.  In the debugger, I manually called `TryGetNSObject()` with `true` for the `evenInFinalizerQueue` parameter, which returned the expected instance -- confirming that the instance has been unexpectedly garbage collected.

The resulting exception claims that it was not "possible to create a new managed instance (because the type 'Xamarin.Forms.Platform.iOS.HeaderWrapperView' does not have a constructor that takes one IntPtr argument)".  In my first attempted workaround, I added the missing constructors.  This got me past the initial problem, only to reveal another -- a call to `ShouldReceiveTouch()` with a `HeaderWrapperView` instance passed as the `UITouch` parameter.  That looked to me like a dangling pointer problem, so I decided to look elsewhere.

I'm very new to Xamarin/.NET, so someone with more context might be able to discover why this `HeaderWrapperView` instance was garbage collected. But I do know iOS, and when I looked into where and how `HeaderWrapperView` is used I saw some suspect code.  Typically, when implementing section headers with custom views in a `UITableView`, you want to use the same kind of reuse that you do for table view cells.  That is, you should use `UITableView.dequeueReusableHeaderFooterView()` to retrieve the section header view.  However, the current implementation creates a new `HeaderWrapperView` on each call to `ListViewDataSource.GetViewForHeader()`. In the test case, this happens each time the state changes in response to the tap on a section header.

To work around the problem, I used `UITableView.dequeueReusableHeaderFooterView()` to attempt reuse of `HeaderFooterView` so a new instance isn't created each time.  This appears to be sufficient to avoid the original issue.

**Additional considerations**

This header reuse only applies to `HeaderWrapperView`, and not to its subviews.  It would be better for scrolling performance if all subviews of `HeaderWrapperView` could be reused. I attempted to implement that (See commented out line at `ListViewRenderer.cs:1122`), but was unable to get the subviews to refresh properly. Again someone with more Xamarin experience and context might be able to make this work.

* Remove commented and debugging code

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue9711.xaml

@@ -0,0 +1,37 @@
+<?xml version="1.0" encoding="utf-8"?>
+
+<controls:TestContentPage
+    xmlns="http://xamarin.com/schemas/2014/forms"
+             xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
+             xmlns:controls="clr-namespace:Xamarin.Forms.Controls"
+             x:Class="Xamarin.Forms.Controls.Issues.Issue9711"
+             >
+    <ListView x:Name="TestListView"
+              IsGroupingEnabled="True"
+              CachingStrategy="RecycleElement"
+              SelectionMode="None"
+              SeparatorVisibility="None"
+              HasUnevenRows="True">
+        <ListView.ItemTemplate>
+            <DataTemplate>
+                <ViewCell>
+                    <Label Text="{Binding ., Mode=OneTime}" />
+                </ViewCell>
+            </DataTemplate>
+        </ListView.ItemTemplate>
+        <ListView.GroupHeaderTemplate>
+            <DataTemplate>
+                <ViewCell BindingContextChanged="ViewCell_OnBindingContextChanged">
+                    <ContentView BackgroundColor="Red">
+                        <ContentView.GestureRecognizers>
+                            <TapGestureRecognizer Tapped="TapGestureRecognizer_OnTapped" />
+                        </ContentView.GestureRecognizers>
+                        <Label Text="{Binding Title, Mode=OneTime}"
+                               IsVisible="{Binding IsExpanded}"
+                               AutomationId="{Binding AutomationId}" />
+                    </ContentView>
+                </ViewCell>
+            </DataTemplate>
+        </ListView.GroupHeaderTemplate>
+    </ListView>
+</controls:TestContentPage>

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

@@ -0,0 +1,128 @@
+using System;
+using System.ComponentModel;
+using System.Runtime.CompilerServices;
+using System.Collections.Generic;
+using System.Collections.Specialized;
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+using System.Threading.Tasks;
+using CategoryAttribute = NUnit.Framework.CategoryAttribute;
+
+// Thanks to GitHub user [@Matmork](https://github.com/Matmork) for this reproducible test case.
+// https://github.com/xamarin/Xamarin.Forms/issues/9711#issuecomment-602520024
+
+#if UITEST
+using Xamarin.Forms.Core.UITests;
+using NUnit.Framework;
+#endif
+
+namespace Xamarin.Forms.Controls.Issues
+{
+	[Preserve(AllMembers = true)]
+	[Issue(IssueTracker.Github, 9711, "[Bug] iOS Failed to marshal the Objective-C object HeaderWrapperView", PlatformAffected.iOS)]
+	public partial class Issue9711 : TestContentPage
+	{
+		protected override void Init()
+		{
+#if APP
+			InitializeComponent();
+
+			List<ListGroup<string>> groups = new List<ListGroup<string>>();
+			for (int i = 0; i < 105; i++)
+			{
+				var group = new ListGroup<string> { Title = $"Group{i}" };
+				for (int j = 0; j < 5; j++)
+				{
+					group.Add($"Group {i} Item {j}");
+				}
+
+				groups.Add(group);
+			}
+
+			TestListView.AutomationId = "9711TestListView";
+			TestListView.ItemsSource = groups;
+#endif
+		}
+
+		private void ViewCell_OnBindingContextChanged(object sender, EventArgs e)
+		{
+			if (sender is ViewCell cell && cell.BindingContext is ListGroup<string> list)
+			{
+				list.Cell = cell;
+			}
+		}
+
+		private async void TapGestureRecognizer_OnTapped(object sender, EventArgs e)
+		{
+			if (sender is ContentView cnt && cnt.BindingContext is ListGroup<string> list)
+			{
+				for (int i = 0; i <= 50; i++)
+				{
+					await Task.Delay(25);
+					list.IsExpanded = !list.IsExpanded;
+				}
+			}
+		}
+
+
+#if UITEST		
+		[Category(UITestCategories.ListView)]
+		[Test]
+		public void TestTappingHeaderDoesNotCrash()
+		{
+			// Usually, tapping one header is sufficient to produce the exception.
+			// However, sometimes it takes two taps, and rarely, three.  If the app
+			// crashes, one of the RunningApp queries will throw, failing the test.
+			Assert.DoesNotThrowAsync(async () =>
+			{
+				RunningApp.Tap(x => x.Marked("Group2"));
+				await Task.Delay(3000);
+				RunningApp.Tap(x => x.Marked("Group1"));
+				await Task.Delay(3000);
+				RunningApp.Tap(x => x.Marked("Group0"));
+				await Task.Delay(3000);
+				RunningApp.Query(x => x.Marked("9711TestListView"));
+			});
+		}
+#endif
+	}
+
+	[Preserve(AllMembers = true)]
+	public sealed class ListGroup<T> : List<T>, INotifyPropertyChanged, INotifyCollectionChanged
+	{
+		public string Title { get; set; }
+		public string AutomationId => Title;
+		private bool _isExpanded = true;
+
+		public bool IsExpanded
+		{
+			get => _isExpanded;
+			set
+			{
+				if (_isExpanded == value)
+					return;
+
+				if (Cell != null)
+					Cell.Height = value ? 75 : 40;
+
+				_isExpanded = value;
+				OnPropertyChanged();
+				OnCollectionChanged();
+			}
+		}
+
+		public ViewCell Cell { get; set; }
+		public event NotifyCollectionChangedEventHandler CollectionChanged;
+		public event PropertyChangedEventHandler PropertyChanged;
+
+		private void OnCollectionChanged()
+		{
+			CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
+		}
+
+		private void OnPropertyChanged([CallerMemberName] string propertyName = null)
+		{
+			PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
+		}
+	}
+}

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

@@ -1362,6 +1362,10 @@
     <Compile Include="$(MSBuildThisFileDirectory)Issue10300.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue10438.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue8958.xaml.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Issue9711.xaml.cs">
+      <DependentUpon>Issue9711.xaml</DependentUpon>
+      <SubType>Code</SubType>
+    </Compile>
   </ItemGroup>
   <ItemGroup>
     <EmbeddedResource Include="$(MSBuildThisFileDirectory)Bugzilla22229.xaml">
@@ -1560,6 +1564,7 @@
       <SubType>Designer</SubType>
       <Generator>MSBuild:UpdateDesignTimeXaml</Generator>
     </EmbeddedResource>
+    <EmbeddedResource Include="$(MSBuildThisFileDirectory)Issue9711.xaml" />
   </ItemGroup>
   <ItemGroup>
     <EmbeddedResource Include="$(MSBuildThisFileDirectory)Bugzilla27417Xaml.xaml">
@@ -2009,4 +2014,4 @@
       <Generator>MSBuild:UpdateDesignTimeXaml</Generator>
     </EmbeddedResource>
   </ItemGroup>
-</Project>
+</Project>

Xamarin.Forms.Platform.iOS/Renderers/ListViewRenderer.cs

@@ -4,7 +4,6 @@
 using System.Collections.Specialized;
 using System.ComponentModel;
 using System.Linq;
-using System.Threading.Tasks;
 using Foundation;
 using UIKit;
 using Xamarin.Forms.Internals;
@@ -1105,20 +1104,21 @@ public override nfloat GetHeightForHeader(UITableView tableView, nint section)
 
 			public override UIView GetViewForHeader(UITableView tableView, nint section)
 			{
-				UIView view = null;
-
 				if (!List.IsGroupingEnabled)
-					return view;
+					return null;
 
 				var cell = TemplatedItemsView.TemplatedItems[(int)section];
 				if (cell.HasContextActions)
 					throw new NotSupportedException("Header cells do not support context actions");
 
+				const string reuseIdentifier = "HeaderWrapper";
+				var header = (HeaderWrapperView)tableView.DequeueReusableHeaderFooterView(reuseIdentifier) ?? new HeaderWrapperView(reuseIdentifier);
+				header.Cell = cell;
+
 				var renderer = (CellRenderer)Internals.Registrar.Registered.GetHandlerForObject<IRegisterable>(cell);
-				view = new HeaderWrapperView { Cell = cell };
-				view.AddSubview(renderer.GetCell(cell, null, tableView));
+				header.SetTableViewCell(renderer.GetCell(cell, null, tableView));
 
-				return view;
+				return header;
 			}
 
 			public override void HeaderViewDisplayingEnded(UITableView tableView, UIView headerView, nint section)
@@ -1457,9 +1457,24 @@ void PreserveActivityIndicatorState(Element element)
 		}
 	}
 
-	internal class HeaderWrapperView : UIView
+	class HeaderWrapperView : UITableViewHeaderFooterView
 	{
+		public HeaderWrapperView(string reuseIdentifier) : base((NSString)reuseIdentifier)
+		{
+		}
+
+		UITableViewCell _tableViewCell;
+
 		public Cell Cell { get; set; }
+
+		public void SetTableViewCell(UITableViewCell value)
+		{
+			if (ReferenceEquals(_tableViewCell, value)) return;
+			_tableViewCell?.RemoveFromSuperview();
+			_tableViewCell = value;
+			AddSubview(value);
+		}
+
 		public override void LayoutSubviews()
 		{
 			base.LayoutSubviews();