[Android] Keep track of created ConditionalFocusLayouts (#2857) fixes #2829

* keep track of created ConditionalFocusLayout's so they can all be disposed of

Xamarin.Forms.ControlGallery.Android/CustomRenderers.cs

@@ -23,6 +23,8 @@ using Android.Text;
 using Android.Text.Method;
 using Xamarin.Forms.Controls.Issues;
 
+
+[assembly: ExportRenderer(typeof(Xamarin.Forms.Controls.Effects.AttachedStateEffectLabel), typeof(AttachedStateEffectLabelRenderer))]
 [assembly: ExportRenderer(typeof(Bugzilla31395.CustomContentView), typeof(CustomContentRenderer))]
 [assembly: ExportRenderer(typeof(NativeListView), typeof(NativeListViewRenderer))]
 [assembly: ExportRenderer(typeof(NativeListView2), typeof(NativeAndroidListViewRenderer))]
@@ -45,6 +47,23 @@ using Xamarin.Forms.Controls.Issues;
 #endif
 namespace Xamarin.Forms.ControlGallery.Android
 {
+	public class AttachedStateEffectLabelRenderer : LabelRenderer
+	{
+		public AttachedStateEffectLabelRenderer(Context context) : base(context)
+		{
+		}
+
+		protected override void Dispose(bool disposing)
+		{
+			foreach(var effect in Element.Effects.OfType<Controls.Effects.AttachedStateEffect>())
+			{
+				effect.Detached(Element);
+			}
+
+			base.Dispose(disposing);
+		}
+	}
+
 	public class NativeDroidMasterDetail : Xamarin.Forms.Platform.Android.AppCompat.MasterDetailPageRenderer
 	{
 		MasterDetailPage _page;

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Effects/AttachedStateEffectLabel.cs

@@ -0,0 +1,15 @@
+using System;
+using System.Collections.Generic;
+using System.Text;
+using Xamarin.Forms.Internals;
+
+namespace Xamarin.Forms.Controls.Effects
+{
+	[Preserve(AllMembers = true)]
+	public class AttachedStateEffectLabel : Label
+	{
+		// Android renderers don't detach effects when the renderers get disposed
+		// so this is a hack setup to detach those effects when testing if dispose is called from a renderer
+		// https://github.com/xamarin/Xamarin.Forms/issues/2520
+	}
+}

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Effects/AttachedStateEffectList.cs

@@ -41,7 +41,6 @@ namespace Xamarin.Forms.Controls.Effects
 				{
 					item.StateChanged -= AttachedStateEffectStateChanged;
 				}
-
 			}
 		}
 
@@ -54,6 +53,8 @@ namespace Xamarin.Forms.Controls.Effects
 			foreach (AttachedStateEffect item in this)
 			{
 				allAttached &= item.State == AttachedStateEffect.AttachedState.Attached;
+
+				if (item.State != AttachedStateEffect.AttachedState.Unknown)
 					allDetached &= item.State == AttachedStateEffect.AttachedState.Detached;
 			}
 

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

@@ -0,0 +1,159 @@
+using System;
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+using System.ComponentModel;
+using System.Linq;
+using System.Text;
+using Xamarin.Forms.Controls.Effects;
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+
+#if UITEST
+using Xamarin.UITest;
+using NUnit.Framework;
+#endif
+
+namespace Xamarin.Forms.Controls.Issues
+{
+	[Preserve(AllMembers = true)]
+	[Issue(IssueTracker.Github, 2829, "[Android] Renderers associated with ListView cells are occasionaly not being disposed of which causes left over events to propagate to disposed views",
+		PlatformAffected.Android)]
+	public class Issue2829 : TestNavigationPage
+	{
+		AttachedStateEffectList attachedStateEffectList = new AttachedStateEffectList();
+		const string kScrollMe = "kScrollMe";
+		const string kSuccess = "SUCCESS";
+		const string kCreateListViewButton = "kCreateListViewButton";
+		StackLayout layout = null;
+
+		protected override void Init()
+		{
+			var label = new Label() { Text = "Click the button then click back" };
+
+			layout = new StackLayout()
+			{
+				Children =
+					{
+						label,
+						new Button()
+						{
+							AutomationId = kCreateListViewButton,
+							Text    = "Create ListView",
+							Command = new Command(() =>
+							{
+								attachedStateEffectList.ToList().ForEach(x=> attachedStateEffectList.Remove(x));
+								label.Text = "FAILURE";
+								Navigation.PushAsync(CreateListViewPage());
+							})
+						}
+					}
+			};
+
+			var page = new ContentPage()
+			{
+				Content = layout
+			};
+
+
+			PushAsync(page);
+			attachedStateEffectList.AllEventsDetached += (_, __) =>
+			{
+				label.Text = kSuccess;
+			};
+		}
+
+		ListView CreateListView()
+		{
+			ListView view = new ListView(ListViewCachingStrategy.RecycleElement);
+			view.ItemTemplate = new DataTemplate(() =>
+			{
+				ViewCell cell = new ViewCell();
+				AttachedStateEffectLabel label = new AttachedStateEffectLabel();
+				label.TextColor = Color.Black;
+				label.BackgroundColor = Color.White;
+				label.SetBinding(Label.TextProperty, "Text");
+				attachedStateEffectList.Add(label);
+				label.BindingContextChanged += (_, __) =>
+				{
+					if (label.AutomationId == null)
+						label.AutomationId = ((Data)label.BindingContext).Text.ToString();
+				};
+
+				cell.View = new ContentView()
+				{
+					Content = new StackLayout()
+					{
+						Orientation = StackOrientation.Horizontal,
+						Children =
+						{
+							label,
+							new Image{ Source = "coffee.png"}
+						}
+					},
+					HeightRequest = 40
+				};
+
+				return cell;
+			});
+			var data = new ObservableCollection<Data>(Enumerable.Range(0, 72).Select(index => new Data() { Text = index.ToString() }));
+			view.ItemsSource = data;
+
+			return view;
+		}
+
+		Page CreateListViewPage()
+		{
+			var view = CreateListView();
+			var data = view.ItemsSource as ObservableCollection<Data>;
+
+			Button scrollMe = new Button()
+			{
+				Text = "Scroll ListView",
+				AutomationId = kScrollMe,
+				Command = new Command(() =>
+				{
+					view.ScrollTo(data.Last(), ScrollToPosition.MakeVisible, true);
+				})
+			};
+
+			return new ContentPage()
+			{
+				Content = new StackLayout()
+				{
+					Children = { scrollMe, view }
+				}
+			};
+		}
+
+		[Preserve(AllMembers = true)]
+		public class Data : INotifyPropertyChanged
+		{
+			private string _text;
+
+			public string Text
+			{
+				get => _text;
+				set
+				{
+					_text = value;
+					PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Text)));
+				}
+			}
+
+			public event PropertyChangedEventHandler PropertyChanged;
+		}
+
+#if UITEST
+		[Test]
+		public void ViewCellsAllDisposed()
+		{
+			RunningApp.Tap(kCreateListViewButton);
+			RunningApp.WaitForElement("0");
+			RunningApp.Tap(kScrollMe);
+			RunningApp.WaitForElement("70");
+			RunningApp.Back();
+			RunningApp.WaitForElement(kSuccess);
+		}
+#endif
+	}
+}

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

@@ -238,6 +238,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla59457.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla59580.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Effects\AttachedStateEffect.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Effects\AttachedStateEffectLabel.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Effects\AttachedStateEffectList.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue2767.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)GitHub1878.cs" />
@@ -249,6 +250,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Issue1705_2.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue1396.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue1415.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Issue2829.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue2653.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue2247.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)GroupListViewHeaderIndexOutOfRange.cs" />

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

@@ -27,6 +27,7 @@ namespace Xamarin.Forms.Platform.Android
 		protected readonly ListView _listView;
 		readonly AListView _realListView;
 		readonly Dictionary<DataTemplate, int> _templateToId = new Dictionary<DataTemplate, int>();
+		readonly List<ConditionalFocusLayout> _layoutsCreated = new List<ConditionalFocusLayout>();
 		int _dataTemplateIncrementer = 2; // lets start at not 0 because ... 
 
 		// We will use _dataTemplateIncrementer to get the proper ViewType key for the item's DataTemplate and store these keys in  _templateToId.
@@ -226,7 +227,10 @@ namespace Xamarin.Forms.Platform.Android
 				convertView = layout.GetChildAt(0);
 			}
 			else
+			{
 				layout = new ConditionalFocusLayout(_context) { Orientation = Orientation.Vertical };
+				_layoutsCreated.Add(layout);
+			}
 
 			if (((cachingStrategy & ListViewCachingStrategy.RecycleElement) != 0) && convertView != null)
 			{
@@ -465,19 +469,26 @@ namespace Xamarin.Forms.Platform.Android
 
 		void DisposeCells()
 		{
-			var cellCount = _realListView?.ChildCount ?? 0;
+			var cellCount = _layoutsCreated.Count;
+
 			for (int i = 0; i < cellCount; i++)
 			{
-				var layout = _realListView.GetChildAt(i) as ConditionalFocusLayout;
+				var layout = _layoutsCreated[i];
 
-				// Headers and footers will be skipped. They are disposed elsewhere.
-				if (layout == null || layout.IsDisposed())
+				if (layout.IsDisposed())
 					continue;
 				
+				DisposeOfConditionalFocusLayout(layout);
+			}
+
+			_layoutsCreated.Clear();
+		}
+
+		void DisposeOfConditionalFocusLayout(ConditionalFocusLayout layout)
+		{
 			var renderedView = layout?.GetChildAt(0);
 
 			var element = (renderedView as INativeElementView)?.Element;
-
 			var view = (element as ViewCell)?.View;
 
 			if (view != null)
@@ -494,7 +505,6 @@ namespace Xamarin.Forms.Platform.Android
 			renderedView?.Dispose();
 			renderedView = null;
 		}
-		}
 
 		// TODO: We can optimize this by storing the last position, group index and global index
 		// and increment/decrement from that starting place.