Fixes leaks and EmptyViewTemplate issues

- EmptyViewTemplate should inherit BindingContext even when it changes
- BindingContext must be cleared on removed children to:
  - Behave like standard BindingContext inheritance
  - Avoid unwanted side-effects
  - Avoid leaking views in memory

Author: Alberto Aldegheri

src/Controls/src/Core/BindableLayout/BindableLayout.cs

@@ -313,7 +313,7 @@ void CreateChildren()
 				return;
 			}
 
-			layout.Clear();
+			ClearChildren(layout);
 
 			UpdateEmptyView(layout);
 
@@ -326,6 +326,23 @@ void CreateChildren()
 			}
 		}
 
+		void ClearChildren(IBindableLayout layout)
+		{
+			var index = layout.Children.Count;
+			while (--index >= 0)
+			{
+				var child = (View)layout.Children[index]!;
+				layout.RemoveAt(index);
+
+				// Empty view inherits the BindingContext automatically,
+				// we don't want to mess up with automatic inheritance.
+				if (child == _currentEmptyView) continue;
+				
+				// Given that we've set BindingContext manually on children we have to clear it on removal.
+				child.BindingContext = null;
+			}
+		}
+
 		void UpdateEmptyView(IBindableLayout layout)
 		{
 			if (_currentEmptyView == null)
@@ -369,7 +386,6 @@ View CreateEmptyView(object emptyView, DataTemplate dataTemplate)
 			if (dataTemplate != null)
 			{
 				var view = (View)dataTemplate.CreateContent();
-				view.BindingContext = (layout as BindableObject).BindingContext;
 				return view;
 			}
 
@@ -390,7 +406,15 @@ void ItemsSourceCollectionChanged(object sender, NotifyCollectionChangedEventArg
 
 			e.Apply(
 				insert: (item, index, _) => layout.Insert(CreateItemView(item, layout), index),
-				removeAt: (item, index) => layout.RemoveAt(index),
+				removeAt: (item, index) =>
+				{
+					var child = (View)layout.Children[index]!;
+					layout.RemoveAt(index);
+					
+					// It's our responsibility to clear the BindingContext for the children
+					// Given that we've set them manually in CreateItemView
+					child.BindingContext = null;
+				},
 				reset: CreateChildren);
 
 			// UpdateEmptyView is called from within CreateChildren, therefor skip it for Reset

src/Controls/tests/Core.UnitTests/BindableLayoutTests.cs

@@ -389,6 +389,77 @@ public void ValidateBindableProperties()
 			Assert.Equal(itemTemplateSelector, layout.GetValue(BindableLayout.ItemTemplateSelectorProperty));
 		}
 
+		[Fact]
+		public async Task DoesNotLeakChildListeningToBindingContextOnRemove()
+		{
+			var myViewModel = new MyViewModel();
+			var list = new ObservableCollection<MyViewModel> { myViewModel };
+			
+			var triggeredCount = 0;
+			var itemTemplate = new DataTemplate(() => new MyViewModelBoundComponent(onTextChangedCallback: () => triggeredCount++));
+
+			var layout = new StackLayout { IsPlatformEnabled = true };
+			BindableLayout.SetItemTemplate(layout, itemTemplate);
+			BindableLayout.SetItemsSource(layout, list);
+			
+			list.RemoveAt(0);
+			
+			// Run GC
+			await Task.Yield();
+			GC.Collect();
+			GC.WaitForPendingFinalizers();
+			
+			// The component should be gone
+			Assert.Equal(0, MyViewModelBoundComponent.InstanceCount);
+		}
+		
+		[Fact]
+		public async Task DoesNotLeakChildListeningToBindingContextOnClear()
+		{
+			var myViewModel = new MyViewModel();
+			var list = new ObservableCollection<MyViewModel> { myViewModel };
+			
+			var triggeredCount = 0;
+			var itemTemplate = new DataTemplate(() => new MyViewModelBoundComponent(onTextChangedCallback: () => triggeredCount++));
+
+			var layout = new StackLayout { IsPlatformEnabled = true };
+			BindableLayout.SetItemTemplate(layout, itemTemplate);
+			BindableLayout.SetItemsSource(layout, list);
+			
+			list.Clear();
+			
+			// Run GC
+			await Task.Yield();
+			GC.Collect();
+			GC.WaitForPendingFinalizers();
+			
+			// The component should be gone
+			Assert.Equal(0, MyViewModelBoundComponent.InstanceCount);
+		}
+		
+		[Fact]
+		public async Task EmptyViewTemplateContentInheritsLayoutBindingContext()
+		{
+			var list = new ObservableCollection<string>();
+
+			var bindingContext = "Foo";
+			
+			var layout = new StackLayout { IsPlatformEnabled = true, BindingContext = bindingContext };
+			BindableLayout.SetEmptyViewTemplate(layout, new DataTemplate(() => new Label()));
+			BindableLayout.SetItemsSource(layout, list);
+			
+			// Verify that the empty view is bound to layout's binding context
+			var emptyView = layout.Children.FirstOrDefault() as Label;
+			Assert.NotNull(emptyView);
+			Assert.Equal(bindingContext, emptyView.BindingContext);
+			
+			// Change binding context on layout
+			layout.BindingContext = bindingContext = "Bar";
+
+			// Verify empty view inherited the binding context
+			Assert.Equal(bindingContext, emptyView.BindingContext);
+		}
+
 		[Fact]
 		public async Task DoesNotLeak()
 		{
@@ -530,5 +601,72 @@ public MyDataTemplateSelectorTest(Func<object, BindableObject, DataTemplate> fun
 			protected override DataTemplate OnSelectTemplate(object item, BindableObject container)
 				=> _func(item, container);
 		}
+		
+		class MyViewModel : INotifyPropertyChanged
+		{
+			string _text;
+
+			public event PropertyChangedEventHandler PropertyChanged;
+
+			public string Text
+			{
+				get => _text;
+				set
+				{
+					_text = value;
+					PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Text)));
+				}
+			}
+		}
+
+		/// <summary>
+		/// When binding context is changed, the component watches for changes in the view model `Text` property.
+		/// The event handler is removed correctly when the binding context is changed or cleared.
+		/// </summary>
+		class MyViewModelBoundComponent : ContentView
+		{
+			public static int InstanceCount = 0;
+			
+			readonly Action _onTextChangedCallback;
+			MyViewModel _myViewModel;
+			
+			public string Text => _myViewModel?.Text;
+			
+			public MyViewModelBoundComponent(Action onTextChangedCallback)
+			{
+				++InstanceCount;
+				_onTextChangedCallback = onTextChangedCallback;
+			}
+			
+			~MyViewModelBoundComponent()
+			{
+				--InstanceCount;
+			}
+			
+			protected override void OnBindingContextChanged()
+			{
+				base.OnBindingContextChanged();
+
+				if (_myViewModel != null)
+				{
+					_myViewModel.PropertyChanged -= OnBindingContextFixturePropertyChanged;
+				}
+
+				_myViewModel = BindingContext as MyViewModel;
+
+				if (_myViewModel != null)
+				{
+					_myViewModel.PropertyChanged += OnBindingContextFixturePropertyChanged;
+				}
+			}
+
+			void OnBindingContextFixturePropertyChanged(object sender, PropertyChangedEventArgs e)
+			{
+				if (e.PropertyName == nameof(MyViewModel.Text))
+				{
+					_onTextChangedCallback?.Invoke();
+				}
+			}
+		}
 	}
 }