[iOS] Null Element before disposing trial renderers in uneven ListVie…

…ws (#894)

* Add repro for 55745

* [iOS] Null Element before disposing trial renderers

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

@@ -0,0 +1,174 @@
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+using System.ComponentModel;
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+
+#if UITEST
+using Xamarin.UITest;
+using NUnit.Framework;
+#endif
+
+// Apply the default category of "Issues" to all of the tests in this assembly
+// We use this as a catch-all for tests which haven't been individually categorized
+#if UITEST
+[assembly: NUnit.Framework.Category("Issues")]
+#endif
+
+namespace Xamarin.Forms.Controls.Issues
+{
+	[Preserve(AllMembers = true)]
+	[Issue(IssueTracker.Bugzilla, 55745, "[iOS] NRE in ListView with HasUnevenRows=true after changing content and rebinding", PlatformAffected.iOS)]
+	public class Bugzilla55745 : TestContentPage
+	{
+		const string ButtonId = "button";
+		ViewModel vm;
+
+		protected override void Init()
+		{
+			vm = new ViewModel();
+			BindingContext = vm;
+
+			var listView = new ListView
+			{
+				HasUnevenRows = true,
+				ItemTemplate = new DataTemplate(() =>
+				{
+					var label1 = new Label();
+					label1.SetBinding(Label.TextProperty, nameof(DataViewModel.TextOne));
+					var label2 = new Label();
+					label2.SetBinding(Label.TextProperty, nameof(DataViewModel.TextTwo));
+					return new ViewCell { View = new StackLayout { Children = { label1, label2 } } };
+				})
+			};
+
+			listView.SetBinding(ListView.ItemsSourceProperty, nameof(vm.MyCollection));
+
+			var button = new Button { Text = "Tap me twice. The app should not crash.", AutomationId = ButtonId };
+			button.Clicked += Button_Clicked;
+
+			Content = new StackLayout { Children = { button, listView } };
+		}
+
+		void Button_Clicked(object sender, System.EventArgs e)
+		{
+			vm.ToggleContent();
+		}
+
+		[Preserve(AllMembers = true)]
+		class DataViewModel : INotifyPropertyChanged
+		{
+			string mTextOne;
+
+			string mTextTwo;
+
+			public event PropertyChangedEventHandler PropertyChanged;
+
+			public string TextOne
+			{
+				get { return mTextOne; }
+				set
+				{
+					mTextOne = value;
+					OnPropertyChanged(nameof(TextOne));
+				}
+			}
+
+			public string TextTwo
+			{
+				get { return mTextTwo; }
+				set
+				{
+					mTextTwo = value;
+					OnPropertyChanged(nameof(TextTwo));
+				}
+			}
+
+			protected virtual void OnPropertyChanged(string propertyName)
+			{
+				PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
+			}
+		}
+
+		[Preserve(AllMembers = true)]
+		class ViewModel : INotifyPropertyChanged
+		{
+			public List<DataViewModel> myList = new List<DataViewModel>()
+			{
+				new DataViewModel() { TextOne = "Super", TextTwo = "Juuu"},
+				new DataViewModel() { TextOne = "Michael", TextTwo = "Maier"},
+				new DataViewModel() { TextOne = "House", TextTwo = "Cat"},
+				new DataViewModel() { TextOne = "Flower", TextTwo = "Rock"},
+				new DataViewModel() { TextOne = "Job", TextTwo = "Dog"},
+				new DataViewModel() { TextOne = "Super", TextTwo = "Juuu"},
+				new DataViewModel() { TextOne = "Michael", TextTwo = "Maier"},
+				new DataViewModel() { TextOne = "House", TextTwo = "Cat"},
+				new DataViewModel() { TextOne = "Flower", TextTwo = "Rock"},
+				new DataViewModel() { TextOne = "Job", TextTwo = "Dog"}
+			};
+
+			ObservableCollection<DataViewModel> mMyCollection;
+
+			DataViewModel mSelectedData;
+
+			public ViewModel()
+			{
+				MyCollection = new ObservableCollection<DataViewModel>(myList);
+			}
+
+			public event PropertyChangedEventHandler PropertyChanged;
+
+			public ObservableCollection<DataViewModel> MyCollection
+			{
+				get
+				{
+					return mMyCollection;
+				}
+				set
+				{
+					mMyCollection = value;
+					OnPropertyChanged(nameof(MyCollection));
+				}
+			}
+
+			public DataViewModel SelectedData
+			{
+				get { return mSelectedData; }
+				set
+				{
+					mSelectedData = value;
+					OnPropertyChanged(nameof(SelectedData));
+				}
+			}
+
+			public void ToggleContent()
+			{
+				if (MyCollection.Count < 3)
+				{
+					MyCollection.Clear();
+					MyCollection = new ObservableCollection<DataViewModel>(myList);
+				}
+				else
+				{
+					MyCollection.Clear();
+					MyCollection.Add(myList[2]);
+				}
+			}
+
+			protected virtual void OnPropertyChanged(string propertyName)
+			{
+				PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
+			}
+		}
+
+#if UITEST
+		[Test]
+		public void Bugzilla55745Test()
+		{
+			RunningApp.WaitForElement(q => q.Marked(ButtonId));
+			RunningApp.Tap(q => q.Marked(ButtonId));
+			RunningApp.Tap(q => q.Marked(ButtonId));
+		}
+#endif
+	}
+}

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

@@ -277,6 +277,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Unreported1.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla53909.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)ListViewNRE.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Bugzilla55745.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)_Template.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue1028.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue1075.cs" />

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

@@ -690,8 +690,18 @@ internal nfloat CalculateHeightForCell(UITableView tableView, Cell cell)
 					foreach (Element descendant in target.Descendants())
 					{
 						IVisualElementRenderer renderer = Platform.GetRenderer(descendant as VisualElement);
+
+						// Clear renderer from descendent; this will not happen in Dispose as normal because we need to
+						// unhook the Element from the renderer before disposing it.
 						descendant.ClearValue(Platform.RendererProperty);
-						renderer?.Dispose();
+
+						if (renderer != null)
+						{
+							// Unhook Element (descendant) from renderer before Disposing so we don't set the Element to null
+							renderer.SetElement(null);
+							renderer.Dispose();
+							renderer = null;
+						}
 					}
 
 					return (nfloat)req.Request.Height;

Xamarin.Forms.Platform.iOS/VisualElementRenderer.cs

@@ -251,9 +251,13 @@ protected override void Dispose(bool disposing)
 					_packager = null;
 				}
 
-				Platform.SetRenderer(Element, null);
-				SetElement(null);
-				Element = null;
+				// The ListView can create renderers and unhook them from the Element before Dispose is called.
+				// Thus, it is possible that this work is already completed.
+				if (Element != null)
+				{
+					Platform.SetRenderer(Element, null);
+					SetElement(null);
+				}
 			}
 			base.Dispose(disposing);
 		}