From ade95d87cbb43bfa4fb697a4df30255b01335e9c Mon Sep 17 00:00:00 2001
From: Martin Zikmund <martin@zikmund.dev>
Date: Tue, 2 May 2023 21:45:56 +0200
Subject: [PATCH] fix: Do not throw for already disposed callbacks

---
 .../Given_DependencyProperty.cs               | 64 +++++++++++++++++--
 src/Uno.UI/UI/Xaml/DependencyObjectStore.cs   |  4 +-
 2 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/src/Uno.UI.Tests/DependencyProperty/Given_DependencyProperty.cs b/src/Uno.UI.Tests/DependencyProperty/Given_DependencyProperty.cs
index db1f49162d..34682eedb6 100644
--- a/src/Uno.UI.Tests/DependencyProperty/Given_DependencyProperty.cs
+++ b/src/Uno.UI.Tests/DependencyProperty/Given_DependencyProperty.cs
@@ -1389,7 +1389,7 @@ namespace Uno.UI.Tests.BinderTests
 
 			void OnBrushChanged(ManagedWeakReference instance, DependencyProperty property, DependencyPropertyChangedEventArgs args)
 			{
-				disposable2 = brush.RegisterDisposablePropertyChangedCallback(OnInnerCallbackBrushChanged);
+				disposable2 = brush.RegisterDisposablePropertyChangedCallback(OnInnerAddCallbackBrushChanged);
 			}
 
 			brush.Color = Colors.Red;
@@ -1398,11 +1398,36 @@ namespace Uno.UI.Tests.BinderTests
 			disposable2?.Dispose();
 		}
 
-		private void OnInnerCallbackBrushChanged(ManagedWeakReference instance, DependencyProperty property, DependencyPropertyChangedEventArgs args)
+		private void OnInnerAddCallbackBrushChanged(ManagedWeakReference instance, DependencyProperty property, DependencyPropertyChangedEventArgs args)
 		{
 			Assert.Fail();
 		}
 
+		[TestMethod]
+		public void When_RemoveCallback_OnPropertyChanged()
+		{
+			var brush = new SolidColorBrush();
+			IDisposable disposable = null;
+			IDisposable disposable2 = null;
+			disposable = brush.RegisterDisposablePropertyChangedCallback(OnBrushChanged);
+			disposable2 = brush.RegisterDisposablePropertyChangedCallback(OnInnerRemoveCallbackBrushChanged);
+
+			void OnBrushChanged(ManagedWeakReference instance, DependencyProperty property, DependencyPropertyChangedEventArgs args)
+			{
+				disposable2.Dispose();
+			}
+
+			Action act = () => brush.Color = Colors.Red;
+			act.Should().NotThrow();
+
+			disposable?.Dispose();
+			disposable2?.Dispose();
+		}
+
+		private void OnInnerRemoveCallbackBrushChanged(ManagedWeakReference instance, DependencyProperty property, DependencyPropertyChangedEventArgs args)
+		{
+		}
+
 		[TestMethod]
 		public void When_AddParentChanged_OnParentChanged()
 		{
@@ -1410,13 +1435,13 @@ namespace Uno.UI.Tests.BinderTests
 			var secondParent = new Windows.UI.Xaml.Controls.Border();
 			var border = new Windows.UI.Xaml.Controls.Border();
 			firstParent.Child = border;
+
 			IDisposable disposable = null;
 			IDisposable disposable2 = null;
-			border.RegisterParentChangedCallback(1, OnParentChangedCallback);
-
+			disposable = border.RegisterParentChangedCallback(1, OnParentChangedCallback);
 			void OnParentChangedCallback(object instance, object key, DependencyObjectParentChangedEventArgs args)
 			{
-				border.RegisterParentChangedCallback(2, OnInnerParentChangedCallback);
+				disposable2 = border.RegisterParentChangedCallback(2, OnInnerAddParentChangedCallback);
 			}
 
 			firstParent.Child = null;
@@ -1425,11 +1450,38 @@ namespace Uno.UI.Tests.BinderTests
 			disposable2?.Dispose();
 		}
 
-		private void OnInnerParentChangedCallback(object instance, object key, DependencyObjectParentChangedEventArgs args)
+		private void OnInnerAddParentChangedCallback(object instance, object key, DependencyObjectParentChangedEventArgs args)
 		{
 			Assert.Fail();
 		}
 
+		[TestMethod]
+		public void When_RemoveParentChanged_OnParentChanged()
+		{
+			var firstParent = new Windows.UI.Xaml.Controls.Border();
+			var secondParent = new Windows.UI.Xaml.Controls.Border();
+			var border = new Windows.UI.Xaml.Controls.Border();
+			firstParent.Child = border;
+			IDisposable disposable = null;
+			IDisposable disposable2 = null;
+			disposable = border.RegisterParentChangedCallback(1, OnParentChangedCallback);
+			disposable2 = border.RegisterParentChangedCallback(2, OnInnerAddParentChangedCallback);
+			void OnParentChangedCallback(object instance, object key, DependencyObjectParentChangedEventArgs args)
+			{
+				disposable2.Dispose();
+			}
+
+			Action act = () => firstParent.Child = null;
+			act.Should().NotThrow();
+
+			disposable?.Dispose();
+			disposable2?.Dispose();
+		}
+
+		private void OnInnerRemoveParentChangedCallback(object instance, object key, DependencyObjectParentChangedEventArgs args)
+		{
+		}
+
 		[TestMethod]
 		public void When_NullablePropertyBinding()
 		{
diff --git a/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs b/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs
index a266eb61e1..ac26c06acb 100644
--- a/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs
+++ b/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs
@@ -993,7 +993,7 @@ namespace Windows.UI.Xaml
 				var weakCallbackRef = WeakReferencePool.RentWeakReference(this, callback);
 
 				ParentChangedCallback weakCallback =
-					(s, _, e) => (weakCallbackRef.Target as ParentChangedCallback)?.Invoke(s, key, e);
+					(s, _, e) => (!weakCallbackRef.IsDisposed ? weakCallbackRef.Target as ParentChangedCallback : null)?.Invoke(s, key, e);
 
 				_parentChangedCallbacks = _parentChangedCallbacks.Add(weakCallback);
 
@@ -1761,7 +1761,7 @@ namespace Windows.UI.Xaml
 			var wr = WeakReferencePool.RentWeakReference(null, callback);
 
 			weakDelegate =
-				(instance, s, e) => (wr.Target as ExplicitPropertyChangedCallback)?.Invoke(instance, s, e);
+				(instance, s, e) => (!wr.IsDisposed ? wr.Target as ExplicitPropertyChangedCallback : null)?.Invoke(instance, s, e);
 
 			weakRelease = new WeakReferenceReturnDisposable(wr);
 		}
-- 
GitLab