fix(binding): Avoid reading source property on UpdateSource for non-DP (backport #13698 and #13521 and #13700)

build/uno.winui.props

@@ -1,6 +1,10 @@
 <?xml version="1.0" encoding="utf-8" ?>
 <Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
 
-	<Import Project="../uno.winui.common.props" Condition="'$(WindowsAppSDKWinUI)'!='true'" />
+	<!-- 
+		Note that WinUI cannot be detected in properties or imports, as the nuget props
+		import from nuget packages cannot be controlled.
+	-->
+	<Import Project="../uno.winui.common.props" />
 
 </Project>

build/uno.winui.single-project.targets

@@ -100,7 +100,7 @@
 	</ItemGroup>
 
 	<Choose>
-		<When Condition="'$(WindowsAppSDKWinUI)'!='true'">
+		<When Condition="'$(_UnoIsWinAppSDKDefined)'!='true'">
 			<!-- IDE capabilities -->
 
 			<!-- Sync with https://github.com/dotnet/maui/blob/ffab30545ac146710a9ee61138be33e52ca4b326/src/Templates/src/templates/maui-mobile/Directory.Build.targets -->
@@ -148,7 +148,7 @@
 		</When>
 	</Choose>
 
-	<Target Name="_RemoveRoslynUnoSourceGenerationWinUI" BeforeTargets="CoreCompile;XamlPreCompile" Condition="'$(WindowsAppSDKWinUI)'=='true'">
+	<Target Name="_RemoveRoslynUnoSourceGenerationWinUI" BeforeTargets="CoreCompile;XamlPreCompile" Condition="'$(_UnoIsWinAppSDKDefined)'=='true'">
 		<!---
 		Remove uno source generators when building under WinAppSDK
 		-->

build/uno.winui.targets

@@ -2,12 +2,17 @@
 <Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
 
 	<PropertyGroup>
+
+		<!-- Wrap WinAppSDK detection until a stable property can be used -->
+		<_UnoIsWinAppSDKDefined>false</_UnoIsWinAppSDKDefined>
+		<_UnoIsWinAppSDKDefined Condition="'$(WindowsAppSDKWinUI)'=='true' or '$(UseWinUITools)'=='true'">true</_UnoIsWinAppSDKDefined>
+
 		<_IsUnoWinUIPackage>$(MSBuildThisFile.ToLower().Equals('uno.winui.targets'))</_IsUnoWinUIPackage>
 	</PropertyGroup>
 
-	<Import Project="../uno.winui.common.targets" Condition="'$(WindowsAppSDKWinUI)'!='true'" />
-	<Import Project="../uno.winui.cross-runtime.targets" Condition="'$(WindowsAppSDKWinUI)'!='true'" />
+	<Import Project="../uno.winui.common.targets" Condition="'$(_UnoIsWinAppSDKDefined)'!='true'" />
+	<Import Project="../uno.winui.cross-runtime.targets" Condition="'$(_UnoIsWinAppSDKDefined)'!='true'" />
 	<Import Project="../uno.winui.single-project.targets" />
-	<Import Project="../uno.winui.runtime-replace.targets" Condition="'$(WindowsAppSDKWinUI)'!='true'" />
-	<Import Project="../uno.winui.winappsdk.targets" Condition="'$(WindowsAppSDKWinUI)'=='true'" />
+	<Import Project="../uno.winui.runtime-replace.targets" Condition="'$(_UnoIsWinAppSDKDefined)'!='true'" />
+	<Import Project="../uno.winui.winappsdk.targets" Condition="'$(_UnoIsWinAppSDKDefined)'=='true'" />
 </Project>

src/SourceGenerators/Uno.UI.SourceGenerators/Content/Uno.UI.SourceGenerators.props

@@ -27,7 +27,7 @@
 		<UnoUIUseRoslynSourceGenerators Condition="'$(UnoUIUseRoslynSourceGenerators)'==''">false</UnoUIUseRoslynSourceGenerators>
 
 		<ShouldRunGenerator>true</ShouldRunGenerator>
-		<ShouldRunGenerator Condition="'$(WindowsAppSDKWinUI)'=='true' or $(TargetFramework.StartsWith('uap10.0'))">false</ShouldRunGenerator>
+		<ShouldRunGenerator Condition="'$(_UnoIsWinAppSDKDefined)'=='true' or $(TargetFramework.StartsWith('uap10.0'))">false</ShouldRunGenerator>
 
 		<!--
 		MSBuild below 17.0 implies C# 7.3 or below, which does not support lambda parameters shadowing (https://github.com/dotnet/csharplang/blob/main/meetings/2019/LDM-2019-01-16.md).

src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_ComboBox.cs

@@ -862,6 +862,85 @@ private async Task<RawBitmap> TakeScreenshot(FrameworkElement SUT)
 		}
 #endif
 
+		[TestMethod]
+		public async Task When_SelectedItem_TwoWay_Binding_Clear()
+		{
+			var root = new Grid();
+
+			var comboBox = new ComboBox();
+
+			root.Children.Add(comboBox);
+
+			comboBox.SetBinding(ComboBox.ItemsSourceProperty, new Binding { Path = new("Items") });
+			comboBox.SetBinding(ComboBox.SelectedItemProperty, new Binding { Path = new("Item"), Mode = BindingMode.TwoWay });
+
+			WindowHelper.WindowContent = root;
+
+			var dc = new TwoWayBindingClearViewModel();
+			root.DataContext = dc;
+
+			await WindowHelper.WaitForIdle();
+
+			dc.Dispose();
+			root.DataContext = null;
+
+			Assert.AreEqual(1, dc.ItemGetCount);
+			Assert.AreEqual(1, dc.ItemSetCount);
+		}
+
+		public sealed class TwoWayBindingClearViewModel : IDisposable
+		{
+			public enum Themes
+			{
+				Invalid,
+				Day,
+				Night
+			}
+
+			public TwoWayBindingClearViewModel()
+			{
+				_item = Items[0];
+			}
+
+			public Themes[] Items { get; } = new Themes[] { Themes.Day, Themes.Night };
+			private Themes _item;
+			private bool _isDisposed;
+			public Themes Item
+			{
+				get
+				{
+					ItemGetCount++;
+
+					if (_isDisposed)
+					{
+						return Themes.Invalid;
+					}
+
+					return _item;
+				}
+				set
+				{
+					ItemSetCount++;
+
+					if (_isDisposed)
+					{
+						_item = Themes.Invalid;
+						return;
+					}
+
+					_item = value;
+				}
+			}
+
+			public int ItemGetCount { get; private set; }
+			public int ItemSetCount { get; private set; }
+
+			public void Dispose()
+			{
+				_isDisposed = true;
+			}
+		}
+
 		public class TwoWayBindingItem : System.ComponentModel.INotifyPropertyChanged
 		{
 			private int _selectedNumber;
@@ -932,6 +1011,8 @@ protected override void OnCollectionChanged(NotifyCollectionChangedEventArgs e)
 		}
 
 	}
+
+
 #if __IOS__
 	#region "Helper classes for the iOS Modal Page (UIModalPresentationStyle.pageSheet)"
 	public partial class MultiFrame : Grid

src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Media_Animation/Given_ObjectAnimationUsingKeyFrames.cs

@@ -74,7 +74,9 @@ public async Task When_Animate()
 			await target.GetValue(ct, 3);
 			await Task.Yield();
 
-			target.History.Should().BeEquivalentTo(v1, v2, v3);
+			// v3 is repeated because the target property is not a DependencyProperty
+			// and no deduplication happens in the binding engine.
+			target.History.Should().BeEquivalentTo(v1, v2, v3, v3);
 			sut.State.Should().Be(Timeline.TimelineState.Filling);
 		}
 
@@ -147,7 +149,9 @@ public async Task When_PauseResume()
 			await target.GetValue(ct, 3);
 			await Task.Yield();
 
-			target.History.Should().BeEquivalentTo(v1, v2, v3);
+			// v3 is repeated because the target property is not a DependencyProperty
+			// and no deduplication happens in the binding engine.
+			target.History.Should().BeEquivalentTo(v1, v2, v3, v3);
 			sut.State.Should().Be(Timeline.TimelineState.Filling);
 		}
 
@@ -178,7 +182,9 @@ public async Task When_RepeatCount()
 			await target.GetValue(ct, 9);
 			await Task.Yield();
 
-			target.History.Should().BeEquivalentTo(v1, v2, v3, v1, v2, v3, v1, v2, v3);
+			// v3 is repeated because the target property is not a DependencyProperty
+			// and no deduplication happens in the binding engine.
+			target.History.Should().BeEquivalentTo(v1, v2, v3, v1, v2, v3, v1, v2, v3, v3);
 			sut.State.Should().Be(Timeline.TimelineState.Filling);
 		}
 

src/Uno.UI/DataBinding/BindingPath.cs

@@ -195,8 +195,14 @@ public object? Value
 			{
 				if (!_disposed
 					&& _value != null
-					&& DependencyObjectStore.AreDifferent(value, _value.GetPrecedenceSpecificValue())
-				)
+					&& (
+						!_value.IsDependencyProperty
+
+						// Don't get the source value if we're not accessing a dependency property.
+						// WinUI does not read the property value before setting the value for a
+						// non-dependency property source.
+						|| DependencyObjectStore.AreDifferent(value, _value.GetPrecedenceSpecificValue())
+					))
 				{
 					_value.Value = value;
 				}
@@ -428,7 +434,7 @@ private static void TryPrependItem(
 			}
 
 			var itemPath = path.Substring(start, length);
-			var item = new BindingItem(head, itemPath, fallbackValue, precedence, allowPrivateMembers);
+			var item = new BindingItem(head, itemPath, precedence, allowPrivateMembers);
 
 			head = item;
 			tail ??= item;
@@ -526,12 +532,10 @@ private sealed class BindingItem : IBindingItem, IDisposable
 			private delegate void PropertyChangedHandler(object? previousValue, object? newValue, bool shouldRaiseValueChanged);
 
 			private ManagedWeakReference? _dataContextWeakStorage;
+			private Flags _flags;
 
 			private readonly SerialDisposable _propertyChanged = new SerialDisposable();
-			private bool _disposed;
 			private readonly DependencyPropertyValuePrecedences? _precedence;
-			private readonly object? _fallbackValue;
-			private readonly bool _allowPrivateMembers;
 			private ValueGetterHandler? _valueGetter;
 			private ValueGetterHandler? _precedenceSpecificGetter;
 			private ValueGetterHandler? _substituteValueGetter;
@@ -543,26 +547,35 @@ private sealed class BindingItem : IBindingItem, IDisposable
 
 			private Type? _dataContextType;
 
-			public BindingItem(BindingItem next, string property, object fallbackValue) :
-				this(next, property, fallbackValue, null, false)
+			[Flags]
+			private enum Flags
 			{
+				None = 0,
+				Disposed = 1 << 0,
+				AllowPrivateMembers = 1 << 1,
+				IsDependencyProperty = 1 << 2,
+				IsDependencyPropertyValueSet = 1 << 3,
 			}
 
-			internal BindingItem(BindingItem? next, string property, object? fallbackValue, DependencyPropertyValuePrecedences? precedence, bool allowPrivateMembers)
+			public BindingItem(BindingItem next, string property) :
+				this(next, property, null, false)
+			{
+			}
+
+			internal BindingItem(BindingItem? next, string property, DependencyPropertyValuePrecedences? precedence, bool allowPrivateMembers)
 			{
 				Next = next;
 				PropertyName = property;
 				_precedence = precedence;
-				_fallbackValue = fallbackValue;
-				_allowPrivateMembers = allowPrivateMembers;
+				AllowPrivateMembers = allowPrivateMembers;
 			}
 
 			public object? DataContext
 			{
 				get => _dataContextWeakStorage?.Target;
 				set
 				{
-					if (!_disposed)
+					if (!IsDisposed)
 					{
 						// Historically, Uno was processing property changes using INPC. Since the inclusion of DependencyObject
 						// values changes are now filtered by DependencyProperty updates, making equality updates at this location
@@ -642,7 +655,7 @@ public Type? PropertyType
 				{
 					if (DataContext != null)
 					{
-						return BindingPropertyHelper.GetPropertyType(_dataContextType!, PropertyName, _allowPrivateMembers);
+						return BindingPropertyHelper.GetPropertyType(_dataContextType!, PropertyName, AllowPrivateMembers);
 					}
 					else
 					{
@@ -737,6 +750,7 @@ private void ClearCachedGetters()
 
 				if (_dataContextType != currentType && _dataContextType != null)
 				{
+					IsDependencyPropertyValueSet = false;
 					_valueGetter = null;
 					_precedenceSpecificGetter = null;
 					_substituteValueGetter = null;
@@ -823,15 +837,15 @@ private void BuildValueGetter()
 			{
 				if (_valueGetter == null && _dataContextType != null)
 				{
-					_valueGetter = BindingPropertyHelper.GetValueGetter(_dataContextType, PropertyName, _precedence, _allowPrivateMembers);
+					_valueGetter = BindingPropertyHelper.GetValueGetter(_dataContextType, PropertyName, _precedence, AllowPrivateMembers);
 				}
 			}
 
 			private void BuildPrecedenceSpecificValueGetter()
 			{
 				if (_precedenceSpecificGetter == null && _dataContextType != null)
 				{
-					_precedenceSpecificGetter = BindingPropertyHelper.GetValueGetter(_dataContextType, PropertyName, _precedence, _allowPrivateMembers);
+					_precedenceSpecificGetter = BindingPropertyHelper.GetValueGetter(_dataContextType, PropertyName, _precedence, AllowPrivateMembers);
 				}
 			}
 
@@ -989,10 +1003,63 @@ private IDisposable SubscribeToPropertyChanged()
 
 			public void Dispose()
 			{
-				_disposed = true;
+				IsDisposed = true;
 				_propertyChanged.Dispose();
 			}
 
+			private bool IsDisposed
+			{
+				get => (_flags & Flags.Disposed) != 0;
+				set => SetFlag(value, Flags.Disposed);
+			}
+
+			private bool AllowPrivateMembers
+			{
+				get => (_flags & Flags.AllowPrivateMembers) != 0;
+				set => SetFlag(value, Flags.AllowPrivateMembers);
+			}
+
+			private bool IsDependencyPropertyValueSet
+			{
+				get => (_flags & Flags.IsDependencyPropertyValueSet) != 0;
+				set => SetFlag(value, Flags.IsDependencyPropertyValueSet);
+			}
+
+			internal bool IsDependencyProperty
+			{
+				get
+				{
+					if (!IsDependencyPropertyValueSet)
+					{
+						var isDP =
+							_dataContextType is not null
+							&& DependencyProperty.GetProperty(_dataContextType!, PropertyName) is not null;
+
+						SetFlag(isDP, Flags.IsDependencyProperty);
+
+						IsDependencyPropertyValueSet = true;
+
+						return isDP;
+					}
+					else
+					{
+						return (_flags & Flags.IsDependencyProperty) != 0;
+					}
+				}
+			}
+
+			private void SetFlag(bool value, Flags flag)
+			{
+				if (!value)
+				{
+					_flags &= ~flag;
+				}
+				else
+				{
+					_flags |= flag;
+				}
+			}
+
 			/// <summary>
 			/// Property changed value handler, used to avoid creating a delegate for processing
 			/// </summary>