Merge pull request #10189 from AvaloniaUI/fixes/10110-binding-unset-unsubscribe

Make bindings which produce UnsetValue revert to default value for property

src/Avalonia.Base/AvaloniaObject.cs

@@ -277,8 +277,8 @@ namespace Avalonia
         /// <param name="property">The property.</param>
         /// <returns>True if the property is set, otherwise false.</returns>
         /// <remarks>
-        /// Checks whether a value is assigned to the property, or that there is a binding to the
+        /// Returns true if <paramref name="property"/> is a styled property which has a value
-        /// property that is producing a value other than <see cref="AvaloniaProperty.UnsetValue"/>.
+        /// assigned to it or a binding targeting it; otherwise false.
         /// </remarks>
         public bool IsSet(AvaloniaProperty property)
         {

src/Avalonia.Base/PropertyStore/BindingEntryBase.cs

@@ -16,6 +16,8 @@ namespace Avalonia.PropertyStore
         private IDisposable? _subscription;
         private bool _hasValue;
         private TValue? _value;
+        private TValue? _defaultValue;
+        private bool _isDefaultValueInitialized;
 
         protected BindingEntryBase(
             ValueFrame frame,
@@ -89,6 +91,7 @@ namespace Avalonia.PropertyStore
 
         protected abstract BindingValue<TValue> ConvertAndValidate(TSource value);
         protected abstract BindingValue<TValue> ConvertAndValidate(BindingValue<TSource> value);
+        protected abstract TValue GetDefaultValue(Type ownerType);
 
         protected virtual void Start(bool produceValue)
         {
@@ -104,17 +107,6 @@ namespace Avalonia.PropertyStore
             };
         }
 
-        private void ClearValue()
-        {
-            if (_hasValue)
-            {
-                _hasValue = false;
-                _value = default;
-                if (_subscription is not null)
-                    Frame.Owner?.OnBindingValueCleared(Property, Frame.Priority);
-            }
-        }
-
         private void SetValue(BindingValue<TValue> value)
         {
             static void Execute(BindingEntryBase<TValue, TSource> instance, BindingValue<TValue> value)
@@ -124,24 +116,20 @@ namespace Avalonia.PropertyStore
 
                 LoggingUtils.LogIfNecessary(instance.Frame.Owner.Owner, instance.Property, value);
 
-                if (value.HasValue)
+                var effectiveValue = value.HasValue ? value.Value : instance.GetCachedDefaultValue();
-                {
+
-                    if (!instance._hasValue || !EqualityComparer<TValue>.Default.Equals(instance._value, value.Value))
+                if (!instance._hasValue || !EqualityComparer<TValue>.Default.Equals(instance._value, effectiveValue))
-                    {
-                        instance._value = value.Value;
-                        instance._hasValue = true;
-                        if (instance._subscription is not null && instance._subscription != s_creatingQuiet)
-                            instance.Frame.Owner?.OnBindingValueChanged(instance, instance.Frame.Priority);
-                    }
-                }
-                else if (value.Type != BindingValueType.DoNothing)
                 {
-                    instance.ClearValue();
+                    instance._value = effectiveValue;
+                    instance._hasValue = true;
                     if (instance._subscription is not null && instance._subscription != s_creatingQuiet)
-                        instance.Frame.Owner?.OnBindingValueCleared(instance.Property, instance.Frame.Priority);
+                        instance.Frame.Owner?.OnBindingValueChanged(instance, instance.Frame.Priority);
                 }
             }
 
+            if (value.Type == BindingValueType.DoNothing)
+                return;
+
             if (Dispatcher.UIThread.CheckAccess())
             {
                 Execute(this, value);
@@ -161,5 +149,16 @@ namespace Avalonia.PropertyStore
             _subscription = null;
             Frame.OnBindingCompleted(this);
         }
+
+        private TValue GetCachedDefaultValue()
+        {
+            if (!_isDefaultValueInitialized)
+            {
+                _defaultValue = GetDefaultValue(Frame.Owner!.Owner.GetType());
+                _isDefaultValueInitialized = true;
+            }
+
+            return _defaultValue!;
+        }
     }
 }

src/Avalonia.Base/PropertyStore/LocalValueBindingObserver.cs

@@ -10,6 +10,8 @@ namespace Avalonia.PropertyStore
     {
         private readonly ValueStore _owner;
         private IDisposable? _subscription;
+        private T? _defaultValue;
+        private bool _isDefaultValueInitialized;
 
         public LocalValueBindingObserver(ValueStore owner, StyledProperty<T> property)
         {
@@ -41,26 +43,28 @@ namespace Avalonia.PropertyStore
 
         public void OnNext(T value)
         {
-            static void Execute(ValueStore owner, StyledProperty<T> property, T value)
+            static void Execute(LocalValueBindingObserver<T> instance, T value)
             {
-                if (property.ValidateValue?.Invoke(value) != false)
+                var owner = instance._owner;
-                    owner.SetValue(property, value, BindingPriority.LocalValue);
+                var property = instance.Property;
-                else
+
-                    owner.ClearLocalValue(property);
+                if (property.ValidateValue?.Invoke(value) == false)
+                    value = instance.GetCachedDefaultValue();
+
+                owner.SetValue(property, value, BindingPriority.LocalValue);
             }
 
             if (Dispatcher.UIThread.CheckAccess())
             {
-                Execute(_owner, Property, value);
+                Execute(this, value);
             }
             else
             {
                 // To avoid allocating closure in the outer scope we need to capture variables
                 // locally. This allows us to skip most of the allocations when on UI thread.
-                var instance = _owner;
+                var instance = this;
-                var property = Property;
                 var newValue = value;
-                Dispatcher.UIThread.Post(() => Execute(instance, property, newValue));
+                Dispatcher.UIThread.Post(() => Execute(instance, newValue));
             }
         }
 
@@ -74,11 +78,21 @@ namespace Avalonia.PropertyStore
                 LoggingUtils.LogIfNecessary(owner.Owner, property, value);
 
                 if (value.HasValue)
-                    owner.SetValue(property, value.Value, BindingPriority.LocalValue);
+                {
-                else if (value.Type != BindingValueType.DataValidationError)
+                    var effectiveValue = value.Value;
-                    owner.ClearLocalValue(property);
+                    if (property.ValidateValue?.Invoke(effectiveValue) == false)
+                        effectiveValue = instance.GetCachedDefaultValue();
+                    owner.SetValue(property, effectiveValue, BindingPriority.LocalValue);
+                }
+                else
+                {
+                    owner.SetValue(property, instance.GetCachedDefaultValue(), BindingPriority.LocalValue);
+                }
             }
 
+            if (value.Type is BindingValueType.DoNothing or BindingValueType.DataValidationError)
+                return;
+
             if (Dispatcher.UIThread.CheckAccess())
             {
                 Execute(this, value);
@@ -92,5 +106,16 @@ namespace Avalonia.PropertyStore
                 Dispatcher.UIThread.Post(() => Execute(instance, newValue));
             }
         }
+
+        private T GetCachedDefaultValue()
+        {
+            if (!_isDefaultValueInitialized)
+            {
+                _defaultValue = Property.GetDefaultValue(_owner.Owner.GetType());
+                _isDefaultValueInitialized = true;
+            }
+
+            return _defaultValue!;
+        }
     }
 }

src/Avalonia.Base/PropertyStore/LocalValueUntypedBindingObserver.cs

@@ -1,5 +1,4 @@
 using System;
-using System.Security.Cryptography;
 using Avalonia.Data;
 using Avalonia.Threading;
 
@@ -10,6 +9,8 @@ namespace Avalonia.PropertyStore
     {
         private readonly ValueStore _owner;
         private IDisposable? _subscription;
+        private T? _defaultValue;
+        private bool _isDefaultValueInitialized;
 
         public LocalValueUntypedBindingObserver(ValueStore owner, StyledProperty<T> property)
         {
@@ -49,11 +50,7 @@ namespace Avalonia.PropertyStore
 
                 if (value == AvaloniaProperty.UnsetValue)
                 {
-                    owner.ClearLocalValue(property);
+                    owner.SetValue(property, instance.GetCachedDefaultValue(), BindingPriority.LocalValue);
-                }
-                else if (value == BindingOperations.DoNothing)
-                {
-                    // Do nothing!
                 }
                 else if (UntypedValueUtils.TryConvertAndValidate(property, value, out var typedValue))
                 {
@@ -61,11 +58,14 @@ namespace Avalonia.PropertyStore
                 }
                 else
                 {
-                    owner.ClearLocalValue(property);
+                    owner.SetValue(property, instance.GetCachedDefaultValue(), BindingPriority.LocalValue);
                     LoggingUtils.LogInvalidValue(owner.Owner, property, typeof(T), value);
                 }
             }
 
+            if (value == BindingOperations.DoNothing)
+                return;
+
             if (Dispatcher.UIThread.CheckAccess())
             {
                 Execute(this, value);
@@ -79,5 +79,16 @@ namespace Avalonia.PropertyStore
                 Dispatcher.UIThread.Post(() => Execute(instance, newValue));
             }
         }
+
+        private T GetCachedDefaultValue()
+        {
+            if (!_isDefaultValueInitialized)
+            {
+                _defaultValue = Property.GetDefaultValue(_owner.Owner.GetType());
+                _isDefaultValueInitialized = true;
+            }
+
+            return _defaultValue!;
+        }
     }
 }

src/Avalonia.Base/PropertyStore/SourceUntypedBindingEntry.cs

@@ -31,5 +31,7 @@ namespace Avalonia.PropertyStore
         {
             throw new NotSupportedException();
         }
+
+        protected override TTarget GetDefaultValue(Type ownerType) => Property.GetDefaultValue(ownerType);
     }
 }

src/Avalonia.Base/PropertyStore/TypedBindingEntry.cs

@@ -48,5 +48,7 @@ namespace Avalonia.PropertyStore
             
             return value;
         }
+
+        protected override T GetDefaultValue(Type ownerType) => Property.GetDefaultValue(ownerType);
     }
 }

src/Avalonia.Base/PropertyStore/UntypedBindingEntry.cs

@@ -29,5 +29,10 @@ namespace Avalonia.PropertyStore
         {
             throw new NotSupportedException();
         }
+
+        protected override object? GetDefaultValue(Type ownerType)
+        {
+            return ((IStyledPropertyMetadata)Property.GetMetadata(ownerType)).DefaultValue;
+        }
     }
 }

src/Avalonia.Base/PropertyStore/ValueStore.cs

@@ -380,23 +380,6 @@ namespace Avalonia.PropertyStore
             }
         }
 
-        /// <summary>
-        /// Called by non-LocalValue binding entries to re-evaluate the effective value when the
-        /// binding produces an unset value.
-        /// </summary>
-        /// <param name="property">The bound property.</param>
-        /// <param name="priority">The priority of binding which produced a new value.</param>
-        public void OnBindingValueCleared(AvaloniaProperty property, BindingPriority priority)
-        {
-            Debug.Assert(priority != BindingPriority.LocalValue);
-
-            if (TryGetEffectiveValue(property, out var existing))
-            {
-                if (priority <= existing.Priority)
-                    ReevaluateEffectiveValue(property, existing);
-            }
-        }
-
         /// <summary>
         /// Called by a <see cref="ValueFrame"/> when its <see cref="ValueFrame.IsActive"/>
         /// state changes.

tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs

@@ -372,6 +372,20 @@ namespace Avalonia.Base.UnitTests
             Assert.Null(target.GetValue(property));
         }
 
+        [Fact]
+        public void LocalValue_Bind_Generic_To_ValueType_Accepts_UnsetValue()
+        {
+            var target = new Class1();
+            var source = new Subject<BindingValue<double>>();
+
+            target.Bind(Class1.QuxProperty, source);
+            source.OnNext(6.7);
+            source.OnNext(BindingValue<double>.Unset);
+
+            Assert.Equal(5.6, target.GetValue(Class1.QuxProperty));
+            Assert.True(target.IsSet(Class1.QuxProperty));
+        }
+
         [Fact]
         public void LocalValue_Bind_NonGeneric_To_ValueType_Accepts_UnsetValue()
         {
@@ -383,7 +397,7 @@ namespace Avalonia.Base.UnitTests
             source.OnNext(AvaloniaProperty.UnsetValue);
 
             Assert.Equal(5.6, target.GetValue(Class1.QuxProperty));
-            Assert.False(target.IsSet(Class1.QuxProperty));
+            Assert.True(target.IsSet(Class1.QuxProperty));
         }
 
         [Fact]
@@ -397,7 +411,7 @@ namespace Avalonia.Base.UnitTests
             source.OnNext(AvaloniaProperty.UnsetValue);
 
             Assert.Equal(5.6, target.GetValue(Class1.QuxProperty));
-            Assert.False(target.IsSet(Class1.QuxProperty));
+            Assert.True(target.IsSet(Class1.QuxProperty));
         }
 
         [Fact]
@@ -1285,6 +1299,24 @@ namespace Avalonia.Base.UnitTests
             subscription.Dispose();
         }
 
+        [Theory]
+        [InlineData(BindingPriority.LocalValue)]
+        [InlineData(BindingPriority.Style)]
+        public void Binding_Producing_UnsetValue_Does_Not_Cause_Unsubscribe(BindingPriority priority)
+        {
+            var target = new Class1();
+            var source = new Subject<BindingValue<string>>();
+            
+            target.Bind(Class1.FooProperty, source, priority);
+
+            source.OnNext("foo");
+            Assert.Equal("foo", target.GetValue(Class1.FooProperty));
+            source.OnNext(BindingValue<string>.Unset);
+            Assert.Equal("foodefault", target.GetValue(Class1.FooProperty));
+            source.OnNext("bar");
+            Assert.Equal("bar", target.GetValue(Class1.FooProperty));
+        }
+
         [Fact]
         public void Produces_Correct_Values_And_Base_Values_With_Multiple_Animation_Bindings()
         {

tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Validation.cs

@@ -66,7 +66,7 @@ namespace Avalonia.Base.UnitTests
         }
 
         [Fact]
-        public void Reverts_To_Lower_Priority_If_Style_Binding_Fails_Validation()
+        public void Reverts_To_DefaultValue_If_Style_Binding_Fails_Validation_2()
         {
             var target = new Class1();
             var source = new Subject<int>();
@@ -75,7 +75,21 @@ namespace Avalonia.Base.UnitTests
             target.Bind(Class1.FooProperty, source, BindingPriority.StyleTrigger);
             source.OnNext(150);
 
-            Assert.Equal(10, target.GetValue(Class1.FooProperty));
+            Assert.Equal(11, target.GetValue(Class1.FooProperty));
+        }
+
+        [Theory]
+        [InlineData(BindingPriority.LocalValue)]
+        [InlineData(BindingPriority.Style)]
+        public void Reverts_To_DefaultValue_If_Style_Binding_Fails_Validation_3(BindingPriority priority)
+        {
+            var target = new Class1();
+            var source = new Subject<BindingValue<int>>();
+
+            target.Bind(Class1.FooProperty, source, priority);
+            source.OnNext(150);
+
+            Assert.Equal(11, target.GetValue(Class1.FooProperty));
         }
 
         [Fact]

tests/Avalonia.Markup.Xaml.UnitTests/Xaml/ResourceDictionaryTests.cs

@@ -307,7 +307,30 @@ namespace Avalonia.Markup.Xaml.UnitTests.Xaml
                 Assert.True(buttonResources.ContainsDeferredKey("Red"));
             }
         }
-        
+
+        [Fact]
+        public void Dynamically_Changing_Referenced_Resources_Works_With_DynamicResource()
+        {
+            var xaml = @"
+<UserControl xmlns='https://github.com/avaloniaui'
+     xmlns:x='http://schemas.microsoft.com/winfx/2006/xaml'>
+  <UserControl.Resources>
+    <Color x:Key='color'>Red</Color>
+    <SolidColorBrush x:Key='brush' Color='{DynamicResource color}' />
+  </UserControl.Resources>
+</UserControl>";
+
+            var userControl = (UserControl)AvaloniaRuntimeXamlLoader.Load(xaml);
+
+            Assert.Equal(Colors.Red, ((ISolidColorBrush)userControl.FindResource("brush")!).Color);
+
+            userControl.Resources.Remove("color");
+            Assert.Equal(default, ((ISolidColorBrush)userControl.FindResource("brush")!).Color);
+
+            userControl.Resources.Add("color", Colors.Blue);
+            Assert.Equal(Colors.Blue, ((ISolidColorBrush)userControl.FindResource("brush")!).Color);
+        }
+
         private IDisposable StyledWindow(params (string, string)[] assets)
         {
             var services = TestServices.StyledWindow.With(