diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index 761c0618daa..4580ef6c58f 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -22,7 +22,7 @@ namespace Avalonia /// /// This class is analogous to DependencyObject in WPF. /// - public class AvaloniaObject : IAvaloniaObject, IAvaloniaObjectDebug, INotifyPropertyChanged, IPriorityValueOwner + public class AvaloniaObject : IAvaloniaObject, IAvaloniaObjectDebug, INotifyPropertyChanged { /// /// The parent object that inherited values are inherited from. @@ -45,21 +45,8 @@ public class AvaloniaObject : IAvaloniaObject, IAvaloniaObjectDebug, INotifyProp /// private EventHandler _propertyChanged; - private DeferredSetter _directDeferredSetter; private ValueStore _values; - - /// - /// Delayed setter helper for direct properties. Used to fix #855. - /// - private DeferredSetter DirectPropertyDeferredSetter - { - get - { - return _directDeferredSetter ?? - (_directDeferredSetter = new DeferredSetter()); - } - } - + private ValueStore Values => _values ?? (_values = new ValueStore(this)); /// /// Initializes a new instance of the class. @@ -225,7 +212,7 @@ public object GetValue(AvaloniaProperty property) } else if (_values != null) { - var result = _values.GetValue(property); + var result = Values.GetValue(property); if (result == AvaloniaProperty.UnsetValue) { @@ -376,12 +363,7 @@ public IDisposable Bind( description, priority); - if (_values == null) - { - _values = new ValueStore(this); - } - - return _values.AddBinding(property, source, priority); + return Values.AddBinding(property, source, priority); } } @@ -414,9 +396,8 @@ public void Revalidate(AvaloniaProperty property) VerifyAccess(); _values?.Revalidate(property); } - - /// - void IPriorityValueOwner.Changed(AvaloniaProperty property, int priority, object oldValue, object newValue) + + internal void PriorityValueChanged(AvaloniaProperty property, int priority, object oldValue, object newValue) { oldValue = (oldValue == AvaloniaProperty.UnsetValue) ? GetDefaultValue(property) : @@ -439,9 +420,8 @@ void IPriorityValueOwner.Changed(AvaloniaProperty property, int priority, object (BindingPriority)priority); } } - - /// - void IPriorityValueOwner.BindingNotificationReceived(AvaloniaProperty property, BindingNotification notification) + + internal void BindingNotificationReceived(AvaloniaProperty property, BindingNotification notification) { UpdateDataValidation(property, notification); } @@ -456,7 +436,7 @@ Delegate[] IAvaloniaObjectDebug.GetPropertyChangedSubscribers() /// Gets all priority values set on the object. /// /// A collection of property/value tuples. - internal IDictionary GetSetValues() => _values?.GetSetValues(); + internal IDictionary GetSetValues() => Values?.GetSetValues(); /// /// Forces revalidation of properties when a property value changes. @@ -566,12 +546,12 @@ protected bool SetAndRaise( T value) { Contract.Requires(setterCallback != null); - return DirectPropertyDeferredSetter.SetAndNotify( + return Values.Setter.SetAndNotify( property, ref field, - (object val, ref T backing, Action notify) => + (object update, ref T backing, Action notify) => { - setterCallback((T)val, ref backing, notify); + setterCallback((T)update, ref backing, notify); return true; }, value); @@ -737,13 +717,8 @@ private void SetStyledValue(AvaloniaProperty property, object value, BindingPrio originalValue?.GetType().FullName ?? "(null)")); } - if (_values == null) - { - _values = new ValueStore(this); - } - LogPropertySet(property, value, priority); - _values.AddValue(property, value, (int)priority); + Values.AddValue(property, value, (int)priority); } /// diff --git a/src/Avalonia.Base/IPriorityValueOwner.cs b/src/Avalonia.Base/IPriorityValueOwner.cs index 5f63f6ef91d..8cbf212381a 100644 --- a/src/Avalonia.Base/IPriorityValueOwner.cs +++ b/src/Avalonia.Base/IPriorityValueOwner.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using Avalonia.Data; +using Avalonia.Utilities; namespace Avalonia { @@ -31,5 +32,7 @@ internal interface IPriorityValueOwner /// Ensures that the current thread is the UI thread. /// void VerifyAccess(); + + DeferredSetter Setter { get; } } } diff --git a/src/Avalonia.Base/PriorityValue.cs b/src/Avalonia.Base/PriorityValue.cs index c474f9098e4..03094e2236f 100644 --- a/src/Avalonia.Base/PriorityValue.cs +++ b/src/Avalonia.Base/PriorityValue.cs @@ -21,7 +21,7 @@ namespace Avalonia /// priority binding that doesn't return . Where there /// are multiple bindings registered with the same priority, the most recently added binding /// has a higher priority. Each time the value changes, the - /// method on the + /// method on the /// owner object is fired with the old and new values. /// internal class PriorityValue @@ -30,7 +30,6 @@ internal class PriorityValue private readonly SingleOrDictionary _levels = new SingleOrDictionary(); private readonly Func _validate; - private static readonly DeferredSetter delayedSetter = new DeferredSetter(); private (object value, int priority) _value; /// @@ -243,12 +242,18 @@ private PriorityLevel GetLevel(int priority) /// The priority level that the value came from. private void UpdateValue(object value, int priority) { - delayedSetter.SetAndNotify(this, + Owner.Setter.SetAndNotify(Property, ref _value, UpdateCore, (value, priority)); } + private bool UpdateCore( + object update, + ref (object value, int priority) backing, + Action notify) + => UpdateCore(((object, int))update, ref backing, notify); + private bool UpdateCore( (object value, int priority) update, ref (object value, int priority) backing, diff --git a/src/Avalonia.Base/Utilities/DeferredSetter.cs b/src/Avalonia.Base/Utilities/DeferredSetter.cs index fdfa1601346..ae6f599005e 100644 --- a/src/Avalonia.Base/Utilities/DeferredSetter.cs +++ b/src/Avalonia.Base/Utilities/DeferredSetter.cs @@ -8,11 +8,10 @@ namespace Avalonia.Utilities { /// /// A utility class to enable deferring assignment until after property-changed notifications are sent. + /// Used to fix #855. /// - /// The type of the object that represents the property. /// The type of value with which to track the delayed assignment. - class DeferredSetter - where TProperty: class + class DeferredSetter { private struct NotifyDisposable : IDisposable { @@ -37,29 +36,44 @@ private class SettingStatus { public bool Notifying { get; set; } - private Queue pendingValues; + private SingleOrQueue pendingValues; - public Queue PendingValues + public SingleOrQueue PendingValues { get { - return pendingValues ?? (pendingValues = new Queue()); + return pendingValues ?? (pendingValues = new SingleOrQueue()); } } } - private readonly ConditionalWeakTable setRecords = new ConditionalWeakTable(); + private Dictionary _setRecords; + private Dictionary SetRecords + => _setRecords ?? (_setRecords = new Dictionary()); + + private SettingStatus GetOrCreateStatus(AvaloniaProperty property) + { + if (!SetRecords.TryGetValue(property, out var status)) + { + status = new SettingStatus(); + SetRecords.Add(property, status); + } + + return status; + } /// /// Mark the property as currently notifying. /// /// The property to mark as notifying. /// Returns a disposable that when disposed, marks the property as done notifying. - private NotifyDisposable MarkNotifying(TProperty property) + private NotifyDisposable MarkNotifying(AvaloniaProperty property) { Contract.Requires(!IsNotifying(property)); - - return new NotifyDisposable(setRecords.GetOrCreateValue(property)); + + SettingStatus status = GetOrCreateStatus(property); + + return new NotifyDisposable(status); } /// @@ -67,19 +81,19 @@ private NotifyDisposable MarkNotifying(TProperty property) /// /// The property. /// If the property is currently notifying listeners. - private bool IsNotifying(TProperty property) - => setRecords.TryGetValue(property, out var value) && value.Notifying; + private bool IsNotifying(AvaloniaProperty property) + => SetRecords.TryGetValue(property, out var value) && value.Notifying; /// /// Add a pending assignment for the property. /// /// The property. /// The value to assign. - private void AddPendingSet(TProperty property, TSetRecord value) + private void AddPendingSet(AvaloniaProperty property, TSetRecord value) { Contract.Requires(IsNotifying(property)); - setRecords.GetOrCreateValue(property).PendingValues.Enqueue(value); + GetOrCreateStatus(property).PendingValues.Enqueue(value); } /// @@ -87,9 +101,9 @@ private void AddPendingSet(TProperty property, TSetRecord value) /// /// The property to check. /// If the property has any pending assignments. - private bool HasPendingSet(TProperty property) + private bool HasPendingSet(AvaloniaProperty property) { - return setRecords.TryGetValue(property, out var status) && status.PendingValues.Count != 0; + return SetRecords.TryGetValue(property, out var status) && !status.PendingValues.Empty; } /// @@ -97,9 +111,9 @@ private bool HasPendingSet(TProperty property) /// /// The property to check. /// The first pending assignment for the property. - private TSetRecord GetFirstPendingSet(TProperty property) + private TSetRecord GetFirstPendingSet(AvaloniaProperty property) { - return setRecords.GetOrCreateValue(property).PendingValues.Dequeue(); + return GetOrCreateStatus(property).PendingValues.Dequeue(); } public delegate bool SetterDelegate(TSetRecord record, ref TValue backing, Action notifyCallback); @@ -115,7 +129,7 @@ private TSetRecord GetFirstPendingSet(TProperty property) /// /// The value to try to set. public bool SetAndNotify( - TProperty property, + AvaloniaProperty property, ref TValue backing, SetterDelegate setterCallback, TSetRecord value) @@ -144,6 +158,7 @@ public bool SetAndNotify( } }); } + return updated; } else if(!object.Equals(value, backing)) diff --git a/src/Avalonia.Base/Utilities/SingleOrQueue.cs b/src/Avalonia.Base/Utilities/SingleOrQueue.cs new file mode 100644 index 00000000000..4a66b72a560 --- /dev/null +++ b/src/Avalonia.Base/Utilities/SingleOrQueue.cs @@ -0,0 +1,58 @@ +using System; +using System.Collections.Generic; +using System.Text; + +namespace Avalonia.Utilities +{ + /// + /// FIFO Queue optimized for holding zero or one items. + /// + /// The type of items held in the queue. + public class SingleOrQueue + { + private T _head; + private Queue _tail; + + private Queue Tail => _tail ?? (_tail = new Queue()); + + private bool HasTail => _tail != null; + + public bool Empty { get; private set; } = true; + + public void Enqueue(T value) + { + if (Empty) + { + _head = value; + } + else + { + Tail.Enqueue(value); + } + + Empty = false; + } + + public T Dequeue() + { + if (Empty) + { + throw new InvalidOperationException("Cannot dequeue from an empty queue!"); + } + + var result = _head; + + if (HasTail && Tail.Count != 0) + { + _head = Tail.Dequeue(); + } + else + { + _head = default; + Empty = true; + } + + return result; + } + } +} diff --git a/src/Avalonia.Base/ValueStore.cs b/src/Avalonia.Base/ValueStore.cs index 8283edab806..c8e2124182c 100644 --- a/src/Avalonia.Base/ValueStore.cs +++ b/src/Avalonia.Base/ValueStore.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using Avalonia.Data; +using Avalonia.Utilities; namespace Avalonia { @@ -91,12 +92,12 @@ public void AddValue(AvaloniaProperty property, object value, int priority) public void BindingNotificationReceived(AvaloniaProperty property, BindingNotification notification) { - ((IPriorityValueOwner)_owner).BindingNotificationReceived(property, notification); + _owner.BindingNotificationReceived(property, notification); } public void Changed(AvaloniaProperty property, int priority, object oldValue, object newValue) { - ((IPriorityValueOwner)_owner).Changed(property, priority, oldValue, newValue); + _owner.PriorityValueChanged(property, priority, oldValue, newValue); } public IDictionary GetSetValues() => throw new NotImplementedException(); @@ -115,7 +116,7 @@ public object GetValue(AvaloniaProperty property) public bool IsAnimating(AvaloniaProperty property) { - return _values.TryGetValue(property, out var value) ? (value as PriorityValue)?.IsAnimating ?? false : false; + return _values.TryGetValue(property, out var value) && value is PriorityValue priority && priority.IsAnimating; } public bool IsSet(AvaloniaProperty property) @@ -148,13 +149,11 @@ private PriorityValue CreatePriorityValue(AvaloniaProperty property) validate2 = v => validate(_owner, v); } - PriorityValue result = new PriorityValue( + return new PriorityValue( this, property, property.PropertyType, validate2); - - return result; } private object Validate(AvaloniaProperty property, object value) @@ -168,5 +167,16 @@ private object Validate(AvaloniaProperty property, object value) return value; } + + private DeferredSetter _defferedSetter; + + public DeferredSetter Setter + { + get + { + return _defferedSetter ?? + (_defferedSetter = new DeferredSetter()); + } + } } } diff --git a/tests/Avalonia.Base.UnitTests/PriorityValueTests.cs b/tests/Avalonia.Base.UnitTests/PriorityValueTests.cs index d6650aa1048..2f1b7862a78 100644 --- a/tests/Avalonia.Base.UnitTests/PriorityValueTests.cs +++ b/tests/Avalonia.Base.UnitTests/PriorityValueTests.cs @@ -1,11 +1,12 @@ // Copyright (c) The Avalonia Project. All rights reserved. // Licensed under the MIT license. See licence.md file in the project root for full license information. +using Avalonia.Utilities; +using Moq; using System; using System.Linq; using System.Reactive.Linq; using System.Reactive.Subjects; -using Moq; using Xunit; namespace Avalonia.Base.UnitTests @@ -21,7 +22,7 @@ public class PriorityValueTests [Fact] public void Initial_Value_Should_Be_UnsetValue() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); Assert.Same(AvaloniaProperty.UnsetValue, target.Value); } @@ -29,7 +30,7 @@ public void Initial_Value_Should_Be_UnsetValue() [Fact] public void First_Binding_Sets_Value() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); target.Add(Single("foo"), 0); @@ -39,7 +40,7 @@ public void First_Binding_Sets_Value() [Fact] public void Changing_Binding_Should_Set_Value() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); var subject = new BehaviorSubject("foo"); target.Add(subject, 0); @@ -51,7 +52,7 @@ public void Changing_Binding_Should_Set_Value() [Fact] public void Setting_Direct_Value_Should_Override_Binding() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); target.Add(Single("foo"), 0); target.SetValue("bar", 0); @@ -62,7 +63,7 @@ public void Setting_Direct_Value_Should_Override_Binding() [Fact] public void Binding_Firing_Should_Override_Direct_Value() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); var source = new BehaviorSubject("initial"); target.Add(source, 0); @@ -76,7 +77,7 @@ public void Binding_Firing_Should_Override_Direct_Value() [Fact] public void Earlier_Binding_Firing_Should_Not_Override_Later() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); var nonActive = new BehaviorSubject("na"); var source = new BehaviorSubject("initial"); @@ -92,7 +93,7 @@ public void Earlier_Binding_Firing_Should_Not_Override_Later() [Fact] public void Binding_Completing_Should_Revert_To_Direct_Value() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); var source = new BehaviorSubject("initial"); target.Add(source, 0); @@ -108,7 +109,7 @@ public void Binding_Completing_Should_Revert_To_Direct_Value() [Fact] public void Binding_With_Lower_Priority_Has_Precedence() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); target.Add(Single("foo"), 1); target.Add(Single("bar"), 0); @@ -120,7 +121,7 @@ public void Binding_With_Lower_Priority_Has_Precedence() [Fact] public void Later_Binding_With_Same_Priority_Should_Take_Precedence() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); target.Add(Single("foo"), 1); target.Add(Single("bar"), 0); @@ -133,7 +134,7 @@ public void Later_Binding_With_Same_Priority_Should_Take_Precedence() [Fact] public void Changing_Binding_With_Lower_Priority_Should_Set_Not_Value() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); var subject = new BehaviorSubject("bar"); target.Add(Single("foo"), 0); @@ -146,7 +147,7 @@ public void Changing_Binding_With_Lower_Priority_Should_Set_Not_Value() [Fact] public void UnsetValue_Should_Fall_Back_To_Next_Binding() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); var subject = new BehaviorSubject("bar"); target.Add(subject, 0); @@ -162,7 +163,7 @@ public void UnsetValue_Should_Fall_Back_To_Next_Binding() [Fact] public void Adding_Value_Should_Call_OnNext() { - var owner = new Mock(); + var owner = GetMockOwner(); var target = new PriorityValue(owner.Object, TestProperty, typeof(string)); target.Add(Single("foo"), 0); @@ -173,7 +174,7 @@ public void Adding_Value_Should_Call_OnNext() [Fact] public void Changing_Value_Should_Call_OnNext() { - var owner = new Mock(); + var owner = GetMockOwner(); var target = new PriorityValue(owner.Object, TestProperty, typeof(string)); var subject = new BehaviorSubject("foo"); @@ -186,7 +187,7 @@ public void Changing_Value_Should_Call_OnNext() [Fact] public void Disposing_A_Binding_Should_Revert_To_Next_Value() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); target.Add(Single("foo"), 0); var disposable = target.Add(Single("bar"), 0); @@ -199,7 +200,7 @@ public void Disposing_A_Binding_Should_Revert_To_Next_Value() [Fact] public void Disposing_A_Binding_Should_Remove_BindingEntry() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); target.Add(Single("foo"), 0); var disposable = target.Add(Single("bar"), 0); @@ -212,7 +213,7 @@ public void Disposing_A_Binding_Should_Remove_BindingEntry() [Fact] public void Completing_A_Binding_Should_Revert_To_Previous_Binding() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); var source = new BehaviorSubject("bar"); target.Add(Single("foo"), 0); @@ -226,7 +227,7 @@ public void Completing_A_Binding_Should_Revert_To_Previous_Binding() [Fact] public void Completing_A_Binding_Should_Revert_To_Lower_Priority() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); var source = new BehaviorSubject("bar"); target.Add(Single("foo"), 1); @@ -240,7 +241,7 @@ public void Completing_A_Binding_Should_Revert_To_Lower_Priority() [Fact] public void Completing_A_Binding_Should_Remove_BindingEntry() { - var target = new PriorityValue(null, TestProperty, typeof(string)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(string)); var subject = new BehaviorSubject("bar"); target.Add(Single("foo"), 0); @@ -254,7 +255,7 @@ public void Completing_A_Binding_Should_Remove_BindingEntry() [Fact] public void Direct_Value_Should_Be_Coerced() { - var target = new PriorityValue(null, TestProperty, typeof(int), x => Math.Min((int)x, 10)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(int), x => Math.Min((int)x, 10)); target.SetValue(5, 0); Assert.Equal(5, target.Value); @@ -265,7 +266,7 @@ public void Direct_Value_Should_Be_Coerced() [Fact] public void Bound_Value_Should_Be_Coerced() { - var target = new PriorityValue(null, TestProperty, typeof(int), x => Math.Min((int)x, 10)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(int), x => Math.Min((int)x, 10)); var source = new Subject(); target.Add(source, 0); @@ -279,7 +280,7 @@ public void Bound_Value_Should_Be_Coerced() public void Revalidate_Should_ReCoerce_Value() { var max = 10; - var target = new PriorityValue(null, TestProperty, typeof(int), x => Math.Min((int)x, max)); + var target = new PriorityValue(GetMockOwner().Object, TestProperty, typeof(int), x => Math.Min((int)x, max)); var source = new Subject(); target.Add(source, 0); @@ -302,5 +303,12 @@ private IObservable Single(T value) { return Observable.Never().StartWith(value); } + + private static Mock GetMockOwner() + { + var owner = new Mock(); + owner.SetupGet(o => o.Setter).Returns(new DeferredSetter()); + return owner; + } } } diff --git a/tests/Avalonia.Base.UnitTests/Utilities/SingleOrQueueTests.cs b/tests/Avalonia.Base.UnitTests/Utilities/SingleOrQueueTests.cs new file mode 100644 index 00000000000..f1ab265bc9d --- /dev/null +++ b/tests/Avalonia.Base.UnitTests/Utilities/SingleOrQueueTests.cs @@ -0,0 +1,50 @@ +using Avalonia.Utilities; +using System; +using System.Collections.Generic; +using System.Text; +using Xunit; + +namespace Avalonia.Base.UnitTests.Utilities +{ + public class SingleOrQueueTests + { + [Fact] + public void New_SingleOrQueue_Is_Empty() + { + Assert.True(new SingleOrQueue().Empty); + } + + [Fact] + public void Dequeue_Throws_When_Empty() + { + var queue = new SingleOrQueue(); + + Assert.Throws(() => queue.Dequeue()); + } + + [Fact] + public void Enqueue_Adds_Element() + { + var queue = new SingleOrQueue(); + + queue.Enqueue(1); + + Assert.False(queue.Empty); + + Assert.Equal(1, queue.Dequeue()); + } + + [Fact] + public void Multiple_Elements_Dequeued_In_Correct_Order() + { + var queue = new SingleOrQueue(); + + queue.Enqueue(1); + queue.Enqueue(2); + queue.Enqueue(3); + Assert.Equal(1, queue.Dequeue()); + Assert.Equal(2, queue.Dequeue()); + Assert.Equal(3, queue.Dequeue()); + } + } +}