[Android] Prevent ObjectDisposedExceptions in ListViews with Header/FooterTemplates (#1155)

* Update repro to include header/footers with bound props

* [Android] Clear renderers of ListView header/footers

And don't call `RemoveAllViews`, because that causes the ObjectDisposedExceptions.

Xamarin.Forms.ControlGallery.Android/CustomRenderers.cs

@@ -31,7 +31,7 @@ using Xamarin.Forms.Controls.Issues;
 [assembly: ExportRenderer(typeof(Bugzilla42000._42000NumericEntryNoDecimal), typeof(EntryRendererNoDecimal))]
 [assembly: ExportRenderer(typeof(Bugzilla42000._42000NumericEntryNoNegative), typeof(EntryRendererNoNegative))]
 //[assembly: ExportRenderer(typeof(AndroidHelpText.HintLabel), typeof(HintLabel))]
-[assembly: ExportRenderer(typeof(Bugzilla57910QuickCollectNavigationPage), typeof(QuickCollectNavigationPage))]
+[assembly: ExportRenderer(typeof(QuickCollectNavigationPage), typeof(QuickCollectNavigationPageRenderer))]
 
 
 [assembly: ExportRenderer(typeof(Xamarin.Forms.Controls.Issues.NoFlashTestNavigationPage), typeof(Xamarin.Forms.ControlGallery.Android.NoFlashTestNavigationPage))]
@@ -548,7 +548,8 @@ namespace Xamarin.Forms.ControlGallery.Android
 #endif
 	}
 
-	public class QuickCollectNavigationPage
+#pragma warning disable CS0618 // Leaving in old constructor so we can verify it works
+	public class QuickCollectNavigationPageRenderer
 #if FORMS_APPLICATION_ACTIVITY
 		: Xamarin.Forms.Platform.Android.NavigationRenderer
 #else

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

@@ -3,6 +3,7 @@ using Xamarin.Forms.Internals;
 using System;
 using System.Linq;
 using System.ComponentModel;
+using System.Collections.ObjectModel;
 
 #if UITEST
 using Xamarin.UITest;
@@ -13,7 +14,7 @@ namespace Xamarin.Forms.Controls.Issues
 {
 	[Preserve(AllMembers = true)]
 	[Issue(IssueTracker.Bugzilla, 57910, "ObjectDisposedException in Xamarin.Forms.Platform.Android.Renderers.ProgressBarRenderer", PlatformAffected.Android)]
-	public class Bugzilla57910 : Bugzilla57910QuickCollectNavigationPage
+	public class Bugzilla57910 : QuickCollectNavigationPage
 	{
 		const string ButtonId = "btnPush";
 		const string Button2Id = "btnPop";
@@ -61,6 +62,93 @@ namespace Xamarin.Forms.Controls.Issues
 			}
 		}
 
+		[Preserve(AllMembers = true)]
+		class ListHeaderView : ContentView
+		{
+			public ListHeaderView()
+			{
+				Label newLabel = new Label();
+				newLabel.SetBinding(Label.TextProperty, nameof(ListPageViewModel.Header));
+				Content = newLabel;
+			}
+		}
+
+		[Preserve(AllMembers = true)]
+		class ListFooterView : ContentView
+		{
+			public ListFooterView()
+			{
+				Label newLabel = new Label();
+				newLabel.SetBinding(Label.TextProperty, nameof(ListPageViewModel.Footer));
+
+				var stack = new StackLayout { Children = { newLabel } };
+				Content = stack;
+			}
+		}
+
+		[Preserve(AllMembers = true)]
+		class ListPageViewModel : INotifyPropertyChanged
+		{
+			ObservableCollection<ListItemViewModel> _items;
+			string _footer;
+			string _header;
+
+			int _counter;
+			public ListPageViewModel()
+			{
+				_header = "Header!";
+				_footer = "Footer!";
+				_counter = 0;
+				_items = new ObservableCollection<ListItemViewModel>(Enumerable.Range(0, 100).Select(c => new ListItemViewModel()));
+
+				// Need an asynchronous action that happens sometime between creation of the Element and Pop of the containing page
+				Device.StartTimer(TimeSpan.FromMilliseconds((100)), () =>
+				{
+					Header = $"Header! {_counter++}";
+					Footer = $"Footer! {_counter++}";
+
+					return true;
+				});
+			}
+
+			public event PropertyChangedEventHandler PropertyChanged;
+
+			public ObservableCollection<ListItemViewModel> Items
+			{
+				get { return _items; }
+				set
+				{
+					_items = value;
+					OnPropertyChanged(nameof(Items));
+				}
+			}
+
+			public string Header
+			{
+				get { return _header; }
+				set
+				{
+					_header = value;
+					OnPropertyChanged(nameof(Header));
+				}
+			}
+
+			public string Footer
+			{
+				get { return _footer; }
+				set
+				{
+					_footer = value;
+					OnPropertyChanged(nameof(Footer));
+				}
+			}
+
+			protected virtual void OnPropertyChanged(string propertyName)
+			{
+				PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
+			}
+		}
+
 		[Preserve(AllMembers = true)]
 		class ListItemViewModel : INotifyPropertyChanged
 		{
@@ -128,12 +216,20 @@ namespace Xamarin.Forms.Controls.Issues
 				ListView listView = new ListView(ListViewCachingStrategy.RecycleElement)
 				{
 					RowHeight = 70,
-					ItemsSource = Enumerable.Range(0, 100).Select(c => new ListItemViewModel()),
+					ItemTemplate = new DataTemplate(typeof(ListItemView)),
-					ItemTemplate = new DataTemplate(typeof(ListItemView))
+					HeaderTemplate = new DataTemplate(typeof(ListHeaderView)),
+					FooterTemplate = new DataTemplate(typeof(ListFooterView)),
 				};
+
+				listView.SetBinding(ListView.ItemsSourceProperty, nameof(ListPageViewModel.Items));
+				listView.SetBinding(ListView.HeaderProperty, ".");
+				listView.SetBinding(ListView.FooterProperty, ".");
+
 				Button newButton = new Button { Text = "Pop", AutomationId = Button2Id };
 				newButton.Clicked += NewButton_Clicked;
 				Content = new StackLayout { Children = { new Label { Text = Instructions2 }, newButton, listView } };
+
+				BindingContext = new ListPageViewModel();
 			}
 
 			void NewButton_Clicked(object sender, EventArgs e)
@@ -156,12 +252,4 @@ namespace Xamarin.Forms.Controls.Issues
 		}
 #endif
 	}
-
-	[Preserve(AllMembers = true)]
-	public class Bugzilla57910QuickCollectNavigationPage : TestNavigationPage
-	{
-		protected override void Init()
-		{
-		}
-	}
 }

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/TestPages/QuickCollectNavigationPage.cs

@@ -0,0 +1,15 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using Xamarin.Forms.Internals;
+
+namespace Xamarin.Forms.Controls
+{
+	[Preserve(AllMembers = true)]
+	public class QuickCollectNavigationPage : TestNavigationPage
+	{
+		protected override void Init()
+		{
+		}
+	}
+}

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

@@ -284,6 +284,7 @@
     </Compile>
     <Compile Include="$(MSBuildThisFileDirectory)RestartAppTest.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla53179_1.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)TestPages\QuickCollectNavigationPage.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)TestPages\ScreenshotConditionalApp.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla41842.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla42277.cs" />

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

@@ -14,6 +14,7 @@ namespace Xamarin.Forms.Platform.Android
 	public class ListViewRenderer : ViewRenderer<ListView, AListView>, SwipeRefreshLayout.IOnRefreshListener
 	{
 		ListViewAdapter _adapter;
+		bool _disposed;
 		IVisualElementRenderer _headerRenderer;
 		IVisualElementRenderer _footerRenderer;
 		Container _headerView;
@@ -25,6 +26,12 @@ namespace Xamarin.Forms.Platform.Android
 		IListViewController Controller => Element;
 		ITemplatedItemsView<Cell> TemplatedItemsView => Element;
 
+		public ListViewRenderer(Context context) : base(context)
+		{
+			AutoPackage = false;
+		}
+
+		[Obsolete("This constructor is obsolete as of version 3.0. Please use ListViewRenderer(Context) instead.")]
 		public ListViewRenderer()
 		{
 			AutoPackage = false;
@@ -38,29 +45,33 @@ namespace Xamarin.Forms.Platform.Android
 
 		protected override void Dispose(bool disposing)
 		{
+			if (_disposed)
+			{
+				return;
+			}
+
+			_disposed = true;
+
 			if (disposing)
 			{
-				if (_headerView == null)
-					return;
-
 				if (_headerRenderer != null)
 				{
-					(_headerRenderer.View as ViewGroup)?.RemoveAllViews();
+					ClearRenderer(_headerRenderer.View);
 					_headerRenderer.Dispose();
 					_headerRenderer = null;
 				}
 
+				_headerView?.Dispose();
+				_headerView = null;
+
 				if (_footerRenderer != null)
 				{
-					(_footerRenderer.View as ViewGroup)?.RemoveAllViews();
+					ClearRenderer(_footerRenderer.View);
 					_footerRenderer.Dispose();
 					_footerRenderer = null;
 				}
 
-				_headerView.Dispose();
+				_footerView?.Dispose();
-				_headerView = null;
-
-				_footerView.Dispose();
 				_footerView = null;
 
 				if (_adapter != null)
@@ -141,7 +152,7 @@ namespace Xamarin.Forms.Platform.Android
 				nativeListView.Focusable = false;
 				nativeListView.DescendantFocusability = DescendantFocusability.AfterDescendants;
 				nativeListView.OnFocusChangeListener = this;
-				nativeListView.Adapter = _adapter = e.NewElement.IsGroupingEnabled && e.NewElement.OnThisPlatform ().IsFastScrollEnabled () ? new GroupedListViewAdapter (Context, nativeListView, e.NewElement) : new ListViewAdapter(Context, nativeListView, e.NewElement);
+				nativeListView.Adapter = _adapter = e.NewElement.IsGroupingEnabled && e.NewElement.OnThisPlatform().IsFastScrollEnabled() ? new GroupedListViewAdapter(Context, nativeListView, e.NewElement) : new ListViewAdapter(Context, nativeListView, e.NewElement);
 				_adapter.HeaderView = _headerView;
 				_adapter.FooterView = _footerView;
 				_adapter.IsAttachedToWindow = _isAttached;
@@ -151,7 +162,7 @@ namespace Xamarin.Forms.Platform.Android
 				UpdateIsSwipeToRefreshEnabled();
 				UpdateFastScrollEnabled();
 
-				
+
 			}
 		}
 
@@ -272,15 +283,38 @@ namespace Xamarin.Forms.Platform.Android
 				Control.SetSelectionFromTop(realPositionWithHeader, y);
 		}
 
+		void ClearRenderer(AView renderedView)
+		{
+			var element = (renderedView as IVisualElementRenderer)?.Element;
+			var view = element as View;
+			if (view != null)
+			{
+				var renderer = Platform.GetRenderer(view);
+				if (renderer == renderedView)
+					element.ClearValue(Platform.RendererProperty);
+				renderer?.Dispose();
+				renderer = null;
+			}
+			var layout = view as IVisualElementRenderer;
+			layout?.Dispose();
+			layout = null;
+		}
+
 		void UpdateFooter()
 		{
 			var footer = (VisualElement)Controller.FooterElement;
-			if (_footerRenderer != null && (footer == null || Registrar.Registered.GetHandlerType(footer.GetType()) != _footerRenderer.GetType()))
+			if (_footerRenderer != null)
 			{
-				if (_footerView != null)
+				var reflectableType = _footerRenderer as System.Reflection.IReflectableType;
-					_footerView.Child = null;
+				var rendererType = reflectableType != null ? reflectableType.GetTypeInfo().AsType() : _footerRenderer.GetType();
-				_footerRenderer.Dispose();
+				if (footer == null || Registrar.Registered.GetHandlerTypeForObject(footer) != rendererType)
-				_footerRenderer = null;
+				{
+					if (_footerView != null)
+						_footerView.Child = null;
+					ClearRenderer(_footerRenderer.View);
+					_footerRenderer.Dispose();
+					_footerRenderer = null;
+				}
 			}
 
 			if (footer == null)
@@ -290,7 +324,7 @@ namespace Xamarin.Forms.Platform.Android
 				_footerRenderer.SetElement(footer);
 			else
 			{
-				_footerRenderer = Platform.CreateRenderer(footer);
+				_footerRenderer = Platform.CreateRenderer(footer, Context);
 				if (_footerView != null)
 					_footerView.Child = _footerRenderer;
 			}
@@ -301,12 +335,18 @@ namespace Xamarin.Forms.Platform.Android
 		void UpdateHeader()
 		{
 			var header = (VisualElement)Controller.HeaderElement;
-			if (_headerRenderer != null && (header == null || Registrar.Registered.GetHandlerType(header.GetType()) != _headerRenderer.GetType()))
+			if (_headerRenderer != null)
 			{
-				if (_headerView != null)
+				var reflectableType = _headerRenderer as System.Reflection.IReflectableType;
-					_headerView.Child = null;
+				var rendererType = reflectableType != null ? reflectableType.GetTypeInfo().AsType() : _headerRenderer.GetType();
-				_headerRenderer.Dispose();
+				if (header == null || Registrar.Registered.GetHandlerTypeForObject(header) != rendererType)
-				_headerRenderer = null;
+				{
+					if (_headerView != null)
+						_headerView.Child = null;
+					ClearRenderer(_headerRenderer.View);
+					_headerRenderer.Dispose();
+					_headerRenderer = null;
+				}
 			}
 
 			if (header == null)
@@ -316,7 +356,7 @@ namespace Xamarin.Forms.Platform.Android
 				_headerRenderer.SetElement(header);
 			else
 			{
-				_headerRenderer = Platform.CreateRenderer(header);
+				_headerRenderer = Platform.CreateRenderer(header, Context);
 				if (_headerView != null)
 					_headerView.Child = _headerRenderer;
 			}
@@ -350,8 +390,9 @@ namespace Xamarin.Forms.Platform.Android
 
 		void UpdateFastScrollEnabled()
 		{
-			if (Control != null) {
+			if (Control != null)
-				Control.FastScrollEnabled = Element.OnThisPlatform ().IsFastScrollEnabled ();
+			{
+				Control.FastScrollEnabled = Element.OnThisPlatform().IsFastScrollEnabled();
 			}
 		}