Fix problems with setter binding priorities.

We need to make sure to set up the bindings as soon as the setter is instanced, even if it's not activated immediately.

src/Avalonia.Base/Reactive/SingleSubscriberObservableBase.cs

@@ -36,7 +36,7 @@ namespace Avalonia.Reactive
             return this;
         }
 
-        void IDisposable.Dispose()
+        public virtual void Dispose()
         {
             Unsubscribed();
             _observer = null;

src/Avalonia.Styling/Styling/ISetter.cs

@@ -16,13 +16,12 @@ namespace Avalonia.Styling
         /// Instances a setter on a control.
         /// </summary>
         /// <param name="target">The control.</param>
-        /// <param name="hasActivator">Whether the parent style has an activator.</param>
         /// <returns>An <see cref="ISetterInstance"/>.</returns>
         /// <remarks>
         /// This method should return an <see cref="ISetterInstance"/> which can be used to apply
         /// the setter to the specified control. Note that it should not apply the setter value 
-        /// until <see cref="ISetterInstance.Activate"/> is called.
+        /// until <see cref="ISetterInstance.Start(bool)"/> is called.
         /// </remarks>
-        ISetterInstance Instance(IStyleable target, bool hasActivator);
+        ISetterInstance Instance(IStyleable target);
     }
 }

src/Avalonia.Styling/Styling/ISetterInstance.cs

@@ -1,20 +1,40 @@
 #nullable enable
 
+using System;
+
 namespace Avalonia.Styling
 {
     /// <summary>
     /// Represents a setter that has been instanced on a control.
     /// </summary>
-    public interface ISetterInstance
+    public interface ISetterInstance : IDisposable
     {
+        /// <summary>
+        /// Starts the setter instance.
+        /// </summary>
+        /// <param name="hasActivator">Whether the parent style has an activator.</param>
+        /// <remarks>
+        /// If <paramref name="hasActivator"/> is false then the setter should be immediately
+        /// applied and <see cref="Activate"/> and <see cref="Deactivate"/> should not be called.
+        /// If true, then bindings etc should be initiated but not produce a value until
+        /// <see cref="Activate"/> called.
+        /// </remarks>
+        public void Start(bool hasActivator);
+
         /// <summary>
         /// Activates the setter.
         /// </summary>
+        /// <remarks>
+        /// Should only be called if hasActivator was true when <see cref="Start(bool)"/> was called.
+        /// </remarks>
         public void Activate();
 
         /// <summary>
         /// Deactivates the setter.
         /// </summary>
+        /// <remarks>
+        /// Should only be called if hasActivator was true when <see cref="Start(bool)"/> was called.
+        /// </remarks>
         public void Deactivate();
     }
 }

src/Avalonia.Styling/Styling/PropertySetterBindingInstance.cs

@@ -1,37 +1,85 @@
 using System;
+using System.Reactive.Subjects;
 using Avalonia.Data;
+using Avalonia.Reactive;
 
 #nullable enable
 
 namespace Avalonia.Styling
 {
-    internal class PropertySetterBindingInstance : ISetterInstance
+    internal class PropertySetterBindingInstance<T> : SingleSubscriberObservableBase<BindingValue<T>>,
+        ISubject<BindingValue<T>>,
+        ISetterInstance
     {
         private readonly IStyleable _target;
-        private readonly AvaloniaProperty _property;
+        private readonly StyledPropertyBase<T>? _styledProperty;
-        private readonly BindingPriority _priority;
+        private readonly DirectPropertyBase<T>? _directProperty;
         private readonly InstancedBinding _binding;
+        private readonly Inner _inner;
+        private BindingValue<T> _value;
         private IDisposable? _subscription;
+        private IDisposable? _subscriptionTwoWay;
         private bool _isActive;
 
         public PropertySetterBindingInstance(
             IStyleable target,
-            AvaloniaProperty property,
+            StyledPropertyBase<T> property,
-            BindingPriority priority,
             IBinding binding)
         {
             _target = target;
-            _property = property;
+            _styledProperty = property;
-            _priority = priority;
+            _binding = binding.Initiate(_target, property);
-            _binding = binding.Initiate(target, property).WithPriority(priority);
+            _inner = new Inner(this);
+        }
+
+        public PropertySetterBindingInstance(
+            IStyleable target,
+            DirectPropertyBase<T> property,
+            IBinding binding)
+        {
+            _target = target;
+            _directProperty = property;
+            _binding = binding.Initiate(_target, property);
+            _inner = new Inner(this);
+        }
+
+        public void Start(bool hasActivator)
+        {
+            _isActive = !hasActivator;
+
+            if (_styledProperty is object)
+            {
+                if (_binding.Mode != BindingMode.OneWayToSource)
+                {
+                    var priority = hasActivator ? BindingPriority.StyleTrigger : BindingPriority.Style;
+                    _subscription = _target.Bind(_styledProperty, this, priority);
+                }
+
+                if (_binding.Mode == BindingMode.TwoWay)
+                {
+                    _subscriptionTwoWay = _target.GetBindingObservable(_styledProperty).Subscribe(this);
+                }
+            }
+            else
+            {
+                if (_binding.Mode != BindingMode.OneWayToSource)
+                {
+                    _subscription = _target.Bind(_directProperty!, this);
+                }
+
+                if (_binding.Mode == BindingMode.TwoWay)
+                {
+                    _subscriptionTwoWay = _target.GetBindingObservable(_directProperty!).Subscribe(this);
+                }
+            }
         }
 
         public void Activate()
         {
             if (!_isActive)
             {
-                _subscription = BindingOperations.Apply(_target, _property, _binding, null);
                 _isActive = true;
+                PublishNext();
             }
         }
 
@@ -39,10 +87,81 @@ namespace Avalonia.Styling
         {
             if (_isActive)
             {
-                _subscription?.Dispose();
-                _subscription = null;
                 _isActive = false;
+                PublishNext();
+            }
+        }
+
+        public override void Dispose()
+        {
+            if (_subscription is object)
+            {
+                var sub = _subscription;
+                _subscription = null;
+                sub.Dispose();
+            }
+
+            if (_subscriptionTwoWay is object)
+            {
+                var sub = _subscriptionTwoWay;
+                _subscriptionTwoWay = null;
+                sub.Dispose();
+            }
+
+            base.Dispose();
+        }
+
+        void IObserver<BindingValue<T>>.OnCompleted()
+        {
+            // This is the observable coming from the target control. It should not complete.
+        }
+
+        void IObserver<BindingValue<T>>.OnError(Exception error)
+        {
+            // This is the observable coming from the target control. It should not error.
+        }
+
+        void IObserver<BindingValue<T>>.OnNext(BindingValue<T> value)
+        {
+            if (value.HasValue && _isActive)
+            {
+                _binding.Subject.OnNext(value.Value);
             }
         }
+
+        protected override void Subscribed()
+        {
+            _subscription = _binding.Observable.Subscribe(_inner);
+        }
+
+        protected override void Unsubscribed()
+        {
+            _subscription?.Dispose();
+            _subscription = null;
+        }
+
+        private void PublishNext()
+        {
+            PublishNext(_isActive ? _value : default);
+        }
+
+        private void ConvertAndPublishNext(object? value)
+        {
+            _value = value is T v ? v : BindingValue<object>.FromUntyped(value).Convert<T>();
+
+            if (_isActive)
+            {
+                PublishNext();
+            }
+        }
+
+        private class Inner : IObserver<object?>
+        {
+            private readonly PropertySetterBindingInstance<T> _owner;
+            public Inner(PropertySetterBindingInstance<T> owner) => _owner = owner;
+            public void OnCompleted() => _owner.PublishCompleted();
+            public void OnError(Exception error) => _owner.PublishError(error);
+            public void OnNext(object? value) => _owner.ConvertAndPublishNext(value);
+        }
     }
 }

src/Avalonia.Styling/Styling/PropertySetterInstance.cs

@@ -1,16 +1,17 @@
 using System;
 using Avalonia.Data;
+using Avalonia.Reactive;
 
 #nullable enable
 
 namespace Avalonia.Styling
 {
-    internal class PropertySetterInstance<T> : ISetterInstance
+    internal class PropertySetterInstance<T> : SingleSubscriberObservableBase<BindingValue<T>>,
+        ISetterInstance
     {
         private readonly IStyleable _target;
         private readonly StyledPropertyBase<T>? _styledProperty;
         private readonly DirectPropertyBase<T>? _directProperty;
-        private readonly BindingPriority _priority;
         private readonly T _value;
         private IDisposable? _subscription;
         private bool _isActive;
@@ -18,41 +19,55 @@ namespace Avalonia.Styling
         public PropertySetterInstance(
             IStyleable target,
             StyledPropertyBase<T> property,
-            BindingPriority priority,
             T value)
         {
             _target = target;
             _styledProperty = property;
-            _priority = priority;
             _value = value;
         }
 
         public PropertySetterInstance(
             IStyleable target,
             DirectPropertyBase<T> property,
-            BindingPriority priority,
             T value)
         {
             _target = target;
             _directProperty = property;
-            _priority = priority;
             _value = value;
         }
 
-        public void Activate()
+        public void Start(bool hasActivator)
         {
-            if (!_isActive)
+            if (hasActivator)
             {
                 if (_styledProperty is object)
                 {
-                    _subscription = _target.SetValue(_styledProperty, _value, _priority);
+                    _subscription = _target.Bind(_styledProperty, this, BindingPriority.StyleTrigger);
+                }
+                else
+                {
+                    _subscription = _target.Bind(_directProperty, this);
+                }
+            }
+            else
+            {
+                if (_styledProperty is object)
+                {
+                    _subscription = _target.SetValue(_styledProperty, _value, BindingPriority.Style);
                 }
                 else
                 {
                     _target.SetValue(_directProperty!, _value);
                 }
+            }
+        }
 
+        public void Activate()
+        {
+            if (!_isActive)
+            {
                 _isActive = true;
+                PublishNext();
             }
         }
 
@@ -60,23 +75,40 @@ namespace Avalonia.Styling
         {
             if (_isActive)
             {
-                if (_subscription is null)
+                _isActive = false;
+                PublishNext();
+            }
+        }
+
+        public override void Dispose()
+        {
+            if (_subscription is object)
+            {
+                var sub = _subscription;
+                _subscription = null;
+                sub.Dispose();
+            }
+            else if (_isActive)
+            {
+                if (_styledProperty is object)
                 {
-                    if (_styledProperty is object)
+                    _target.ClearValue(_styledProperty);
-                    {
-                        _target.ClearValue(_styledProperty);
-                    }
-                    else
-                    {
-                        _target.ClearValue(_directProperty!);
-                    }
                 }
                 else
                 {
-                    _subscription.Dispose();
+                    _target.ClearValue(_directProperty);
-                    _subscription = null;
                 }
             }
+
+            base.Dispose();
+        }
+
+        protected override void Subscribed() => PublishNext();
+        protected override void Unsubscribed() { }
+
+        private void PublishNext()
+        {
+            PublishNext(_isActive ? new BindingValue<T>(_value) : default);
         }
     }
 }

src/Avalonia.Styling/Styling/Setter.cs

@@ -61,7 +61,7 @@ namespace Avalonia.Styling
             }
         }
 
-        public ISetterInstance Instance(IStyleable target, bool hasActivator)
+        public ISetterInstance Instance(IStyleable target)
         {
             target = target ?? throw new ArgumentNullException(nameof(target));
 
@@ -70,60 +70,67 @@ namespace Avalonia.Styling
                 throw new InvalidOperationException("Setter.Property must be set.");
             }
 
-            var priority = hasActivator ? BindingPriority.StyleTrigger : BindingPriority.Style;
+            var value = Value;
 
-            if (Value is IBinding binding)
+            if (value is ITemplate template &&
+                !typeof(ITemplate).IsAssignableFrom(Property.PropertyType))
             {
-                return new PropertySetterBindingInstance(target, Property, priority, binding);
+                value = template.Build();
             }
-            else
+
+            var data = new SetterVisitorData
             {
-                var value = Value;
+                target = target,
-
+                value = value,
-                if (value is ITemplate template &&
+            };
-                    !typeof(ITemplate).IsAssignableFrom(Property.PropertyType))
+
-                {
+            Property.Accept(this, ref data);
-                    value = template.Build();
+            return data.result!;
-                }
-
-                var data = new SetterVisitorData
-                {
-                    target = target,
-                    priority = priority,
-                    value = value,
-                };
-
-                Property.Accept(this, ref data);
-                return data.result!;
-            }
         }
 
         void IAvaloniaPropertyVisitor<SetterVisitorData>.Visit<T>(
             StyledPropertyBase<T> property,
             ref SetterVisitorData data)
         {
-            data.result = new PropertySetterInstance<T>(
+            if (data.value is IBinding binding)
-                data.target,
+            {
-                property,
+                data.result = new PropertySetterBindingInstance<T>(
-                data.priority,
+                    data.target,
-                (T)data.value);
+                    property,
+                    binding);
+            }
+            else
+            {
+                data.result = new PropertySetterInstance<T>(
+                    data.target,
+                    property,
+                    (T)data.value);
+            }
         }
 
         void IAvaloniaPropertyVisitor<SetterVisitorData>.Visit<T>(
             DirectPropertyBase<T> property,
             ref SetterVisitorData data)
         {
-            data.result = new PropertySetterInstance<T>(
+            if (data.value is IBinding binding)
-                data.target,
+            {
-                property,
+                data.result = new PropertySetterBindingInstance<T>(
-                data.priority,
+                    data.target,
-                (T)data.value);
+                    property,
+                    binding);
+            }
+            else
+            {
+                data.result = new PropertySetterInstance<T>(
+                    data.target,
+                    property,
+                    (T)data.value);
+            }
         }
 
         private struct SetterVisitorData
         {
             public IStyleable target;
-            public BindingPriority priority;
             public object? value;
             public ISetterInstance? result;
         }

src/Avalonia.Styling/Styling/StyleInstance.cs

@@ -23,12 +23,14 @@ namespace Avalonia.Styling
             Source = source ?? throw new ArgumentNullException(nameof(source));
             Target = target ?? throw new ArgumentNullException(nameof(target));
 
-            _setters = new List<ISetterInstance>(setters.Count);
+            var setterCount = setters.Count;
+
+            _setters = new List<ISetterInstance>(setterCount);
             _activator = activator;
 
-            foreach (var setter in setters)
+            for (var i = 0; i < setterCount; ++i)
             {
-                _setters.Add(setter.Instance(target, activator is object));
+                _setters.Add(setters[i].Instance(Target));
             }
         }
 
@@ -37,19 +39,26 @@ namespace Avalonia.Styling
 
         public void Start()
         {
-            if (_activator == null)
+            var hasActivator = _activator is object;
+
+            foreach (var setter in _setters)
             {
-                ActivatorChanged(true);
+                setter.Start(hasActivator);
             }
-            else
+
+            if (hasActivator)
             {
-                _activator.Subscribe(this, 0);
+                _activator!.Subscribe(this, 0);
             }
         }
 
         public void Dispose()
         {
-            ActivatorChanged(false);
+            foreach (var setter in _setters)
+            {
+                setter.Dispose();
+            }
+
             _activator?.Dispose();
         }
 

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

@@ -53,7 +53,7 @@ namespace Avalonia.Markup.Xaml.UnitTests
                     }
                 };
 
-                setter.Instance(control, false).Activate();
+                setter.Instance(control).Start(false);
                 Assert.Equal("foo", control.Text);
 
                 control.Text = "bar";

tests/Avalonia.Styling.UnitTests/SetterTests.cs

@@ -33,7 +33,7 @@ namespace Avalonia.Styling.UnitTests
             var style = Mock.Of<IStyle>();
             var setter = new Setter(TextBlock.TextProperty, binding);
 
-            setter.Instance(control, false).Activate();
+            setter.Instance(control).Start(false);
 
             Assert.Equal("foo", control.Text);
         }
@@ -46,7 +46,7 @@ namespace Avalonia.Styling.UnitTests
             var style = Mock.Of<IStyle>();
             var setter = new Setter(Decorator.ChildProperty, template);
 
-            setter.Instance(control, false).Activate();
+            setter.Instance(control).Start(false);
 
             Assert.IsType<Canvas>(control.Child);
         }
@@ -63,8 +63,10 @@ namespace Avalonia.Styling.UnitTests
             };
             var setter = new Setter(Decorator.TagProperty, binding);
 
-            var instance = setter.Instance(control, true);
+            var instance = setter.Instance(control);
+            instance.Start(true);
             instance.Activate();
+
             Assert.Equal("foobar", control.Tag);
 
             // Issue #1218 caused TestConverter.ConvertBack to throw here.
@@ -79,7 +81,7 @@ namespace Avalonia.Styling.UnitTests
             var style = Mock.Of<Style>();
             var setter = new Setter(TextBlock.TagProperty, "foo");
 
-            setter.Instance(control.Object, false).Activate();
+            setter.Instance(control.Object).Start(false);
 
             control.Verify(x => x.SetValue(
                 TextBlock.TagProperty,
@@ -88,18 +90,20 @@ namespace Avalonia.Styling.UnitTests
         }
 
         [Fact]
-        public void Setter_Should_Apply_Value_With_Activator_With_StyleTrigger_Priority()
+        public void Setter_Should_Apply_Value_With_Activator_As_Binding_With_StyleTrigger_Priority()
         {
             var control = new Mock<IStyleable>();
             var style = Mock.Of<Style>();
             var setter = new Setter(TextBlock.TagProperty, "foo");
             var activator = new Subject<bool>();
 
-            setter.Instance(control.Object, true).Activate();
+            var instance = setter.Instance(control.Object);
+            instance.Start(true);
+            instance.Activate();
 
-            control.Verify(x => x.SetValue(
+            control.Verify(x => x.Bind(
                 TextBlock.TagProperty,
-                "foo",
+                It.IsAny<IObservable<BindingValue<object>>>(),
                 BindingPriority.StyleTrigger));
         }
 
@@ -110,11 +114,11 @@ namespace Avalonia.Styling.UnitTests
             var style = Mock.Of<Style>();
             var setter = new Setter(TextBlock.TagProperty, CreateMockBinding(TextBlock.TagProperty));
 
-            setter.Instance(control.Object, false).Activate();
+            setter.Instance(control.Object).Start(false);
 
             control.Verify(x => x.Bind(
                 TextBlock.TagProperty,
-                It.IsAny<IObservable<BindingValue<object>>>(),
+                It.IsAny<PropertySetterBindingInstance<object>>(),
                 BindingPriority.Style));
         }
 
@@ -125,7 +129,9 @@ namespace Avalonia.Styling.UnitTests
             var style = Mock.Of<Style>();
             var setter = new Setter(TextBlock.TagProperty, CreateMockBinding(TextBlock.TagProperty));
 
-            setter.Instance(control.Object, true).Activate();
+            var instance = setter.Instance(control.Object);
+            instance.Start(true);
+            instance.Activate();
 
             control.Verify(x => x.Bind(
                 TextBlock.TagProperty,

tests/Avalonia.Styling.UnitTests/StyleTests.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using Avalonia.Controls;
+using Avalonia.Data;
 using Avalonia.UnitTests;
 using Xunit;
 
@@ -141,6 +142,83 @@ namespace Avalonia.Styling.UnitTests
             Assert.Equal(new[] { "foodefault", "Foo", "Bar", "foodefault" }, values);
         }
 
+        [Fact]
+        public void Later_Styles_Should_Override_Earlier_2()
+        {
+            Styles styles = new Styles
+            {
+                new Style(x => x.OfType<Class1>().Class("foo"))
+                {
+                    Setters =
+                    {
+                        new Setter(Class1.FooProperty, "Foo"),
+                    },
+                },
+
+                new Style(x => x.OfType<Class1>().Class("bar"))
+                {
+                    Setters =
+                    {
+                        new Setter(Class1.FooProperty, "Bar"),
+                    },
+                }
+            };
+
+            var target = new Class1();
+
+            List<string> values = new List<string>();
+            target.GetObservable(Class1.FooProperty).Subscribe(x => values.Add(x));
+
+            styles.TryAttach(target, null);
+            target.Classes.Add("bar");
+            target.Classes.Add("foo");
+            target.Classes.Remove("foo");
+
+            Assert.Equal(new[] { "foodefault", "Bar" }, values);
+        }
+
+        [Fact]
+        public void Later_Styles_Should_Override_Earlier_3()
+        {
+            Styles styles = new Styles
+            {
+                new Style(x => x.OfType<Class1>().Class("foo"))
+                {
+                    Setters =
+                    {
+                        new Setter(Class1.FooProperty, new Binding("Foo")),
+                    },
+                },
+
+                new Style(x => x.OfType<Class1>().Class("bar"))
+                {
+                    Setters =
+                    {
+                        new Setter(Class1.FooProperty, new Binding("Bar")),
+                    },
+                }
+            };
+
+            var target = new Class1
+            {
+                DataContext = new
+                {
+                    Foo = "Foo",
+                    Bar = "Bar",
+                }
+            };
+
+            List<string> values = new List<string>();
+            target.GetObservable(Class1.FooProperty).Subscribe(x => values.Add(x));
+
+            styles.TryAttach(target, null);
+            target.Classes.Add("bar");
+            target.Classes.Add("foo");
+            target.Classes.Remove("foo");
+
+            Assert.Equal(new[] { "foodefault", "Bar" }, values);
+        }
+
         [Fact]
         public void Style_Should_Detach_When_Control_Removed_From_Logical_Tree()
         {