diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index 8506414289..2035b6f2aa 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -56,7 +56,7 @@ namespace Avalonia /// /// Delayed setter helper for direct properties. Used to fix #855. /// - private DeferredSetter DirectDelayedSetter + private DeferredSetter DirectPropertyDeferredSetter { get { @@ -554,6 +554,15 @@ namespace Avalonia } } + /// + /// A callback type for encapsulating complex logic for setting direct properties. + /// + /// The type of the property. + /// The value to which to set the property. + /// The backing field for the property. + /// A wrapper for the property-changed notification. + protected delegate void SetAndRaiseCallback(T value, ref T field, Action notifyWrapper); + /// /// Sets the backing field for a direct avalonia property, raising the /// event if the value has changed. @@ -561,61 +570,70 @@ namespace Avalonia /// The type of the property. /// The property. /// The backing field. + /// A callback called to actually set the value to the backing field. /// The value. /// /// True if the value changed, otherwise false. /// - protected bool SetAndRaise(AvaloniaProperty property, ref T field, T value) + protected bool SetAndRaise( + AvaloniaProperty property, + ref T field, + SetAndRaiseCallback setterCallback, + T value) { - VerifyAccess(); - if (!DirectDelayedSetter.IsNotifying(property)) - { - var valueChanged = false; - if (!object.Equals(field, value)) + Contract.Requires(setterCallback != null); + return DirectPropertyDeferredSetter.SetAndNotify( + property, + ref field, + (object val, ref T backing, Action notify) => { - SetAndRaiseCore(property, ref field, value); - - valueChanged = true; - } + setterCallback((T)val, ref backing, notify); + return true; + }, + value, + (object o, ref T backing) => !object.Equals(o, backing)); + } - while (DirectDelayedSetter.HasPendingSet(property)) - { - SetAndRaiseCore(property, ref field, (T)DirectDelayedSetter.GetFirstPendingSet(property)); - valueChanged = true; - } - return valueChanged; - } - else if(!object.Equals(field, value)) - { - DirectDelayedSetter.AddPendingSet(property, value); - } - return false; + /// + /// Sets the backing field for a direct avalonia property, raising the + /// event if the value has changed. + /// + /// The type of the property. + /// The property. + /// The backing field. + /// The value. + /// + /// True if the value changed, otherwise false. + /// + protected bool SetAndRaise(AvaloniaProperty property, ref T field, T value) + { + VerifyAccess(); + return SetAndRaise( + property, + ref field, + (T val, ref T backing, Action notifyWrapper) + => SetAndRaiseCore(property, ref backing, val, notifyWrapper), + value); } - private void SetAndRaiseCore(AvaloniaProperty property, ref T field, T value) + /// + /// Default assignment logic for SetAndRaise. + /// + /// The type of the property. + /// The property. + /// The backing field. + /// The value. + /// A wrapper for the property-changed notification. + /// + /// True if the value changed, otherwise false. + /// + private bool SetAndRaiseCore(AvaloniaProperty property, ref T field, T value, Action notifyWrapper) { var old = field; field = value; - using (DirectDelayedSetter.MarkNotifying(property)) - { - RaisePropertyChanged(property, old, value, BindingPriority.LocalValue); - } - } - - protected void SetAndRaise( - AvaloniaProperty property, - Action> setterCallback, - T value, - Predicate pendingSetCondition) - { - Contract.Requires(setterCallback != null); - Contract.Requires(pendingSetCondition != null); - DirectDelayedSetter.SetAndNotify( - property, - (val, notify) => setterCallback((T)val, notify), - value, - o => pendingSetCondition((T)o)); + notifyWrapper(() => RaisePropertyChanged(property, old, value, BindingPriority.LocalValue)); + return true; } /// diff --git a/src/Avalonia.Base/PriorityValue.cs b/src/Avalonia.Base/PriorityValue.cs index ea10427b10..cad86b5dea 100644 --- a/src/Avalonia.Base/PriorityValue.cs +++ b/src/Avalonia.Base/PriorityValue.cs @@ -48,6 +48,7 @@ namespace Avalonia { Owner = owner; Property = property; + _valueType = valueType; _value = (AvaloniaProperty.UnsetValue, int.MaxValue); _validate = validate; } @@ -231,12 +232,17 @@ namespace Avalonia private void UpdateValue(object value, int priority) { delayedSetter.SetAndNotify(this, + ref _value, UpdateCore, (value, priority), - val => !object.Equals(val.value, val.value)); + ((object value, int) val, ref (object value, int) backing) + => !object.Equals(val.value, backing.value)); } - private void UpdateCore((object value, int priority) update, Action notify) + private bool UpdateCore( + (object value, int priority) update, + ref (object value, int priority) backing, + Action notify) { var val = update.value; var notification = val as BindingNotification; @@ -249,14 +255,14 @@ namespace Avalonia if (TypeUtilities.TryConvertImplicit(_valueType, val, out castValue)) { - var old = this._value.value; + var old = backing.value; if (_validate != null && castValue != AvaloniaProperty.UnsetValue) { castValue = _validate(castValue); } - this._value = (castValue, update.priority); + backing = (castValue, update.priority); if (notification?.HasValue == true) { @@ -284,6 +290,7 @@ namespace Avalonia val, val?.GetType()); } + return true; } } } diff --git a/src/Avalonia.Base/Utilities/DeferredSetter.cs b/src/Avalonia.Base/Utilities/DeferredSetter.cs index b42ca0bf3d..2c8c7f8887 100644 --- a/src/Avalonia.Base/Utilities/DeferredSetter.cs +++ b/src/Avalonia.Base/Utilities/DeferredSetter.cs @@ -10,11 +10,11 @@ namespace Avalonia.Utilities /// A utility class to enable deferring assignment until after property-changed notifications are sent. /// /// The type of the object that represents the property. - /// The type of value with which to track the delayed assignment. - class DeferredSetter + /// The type of value with which to track the delayed assignment. + class DeferredSetter where TProperty: class { - internal struct NotifyDisposable : IDisposable + private struct NotifyDisposable : IDisposable { private readonly SettingStatus status; @@ -33,17 +33,17 @@ namespace Avalonia.Utilities /// /// Information on current setting/notification status of a property. /// - internal class SettingStatus + private class SettingStatus { public bool Notifying { get; set; } - private Queue pendingValues; + private Queue pendingValues; - public Queue PendingValues + public Queue PendingValues { get { - return pendingValues ?? (pendingValues = new Queue()); + return pendingValues ?? (pendingValues = new Queue()); } } } @@ -55,7 +55,7 @@ namespace Avalonia.Utilities /// /// The property to mark as notifying. /// Returns a disposable that when disposed, marks the property as done notifying. - internal NotifyDisposable MarkNotifying(TProperty property) + private NotifyDisposable MarkNotifying(TProperty property) { Contract.Requires(!IsNotifying(property)); @@ -67,7 +67,7 @@ namespace Avalonia.Utilities /// /// The property. /// If the property is currently notifying listeners. - internal bool IsNotifying(TProperty property) + private bool IsNotifying(TProperty property) => setRecords.TryGetValue(property, out var value) && value.Notifying; /// @@ -75,7 +75,7 @@ namespace Avalonia.Utilities /// /// The property. /// The value to assign. - internal void AddPendingSet(TProperty property, TValue value) + private void AddPendingSet(TProperty property, TSetRecord value) { Contract.Requires(IsNotifying(property)); @@ -87,7 +87,7 @@ namespace Avalonia.Utilities /// /// The property to check. /// If the property has any pending assignments. - internal bool HasPendingSet(TProperty property) + private bool HasPendingSet(TProperty property) { return setRecords.TryGetValue(property, out var status) && status.PendingValues.Count != 0; } @@ -97,41 +97,50 @@ namespace Avalonia.Utilities /// /// The property to check. /// The first pending assignment for the property. - internal TValue GetFirstPendingSet(TProperty property) + private TSetRecord GetFirstPendingSet(TProperty property) { return setRecords.GetOrCreateValue(property).PendingValues.Dequeue(); } + public delegate bool SetterDelegate(TSetRecord record, ref TValue backing, Action notifyCallback); + public delegate bool PendingSetPredicate(TSetRecord record, ref TValue backing); + /// /// Set the property and notify listeners while ensuring we don't get into a stack overflow as happens with #855 and #824 /// /// The property to set. + /// The backing field for the property /// /// A callback that actually sets the property. /// The first parameter is the value to set, and the second is a wrapper that takes a callback that sends the property-changed notification. /// /// The value to try to set. /// A predicate to filter what possible values should be added as pending sets (i.e. only values not equal to the current value). - public void SetAndNotify( + public bool SetAndNotify( TProperty property, - Action> setterCallback, - TValue value, - Predicate pendingSetCondition) + ref TValue backing, + SetterDelegate setterCallback, + TSetRecord value, + PendingSetPredicate pendingSetCondition) { Contract.Requires(setterCallback != null); Contract.Requires(pendingSetCondition != null); if (!IsNotifying(property)) { - setterCallback(value, notification => + bool updated = false; + if (pendingSetCondition(value, ref backing)) { - using (MarkNotifying(property)) + updated = setterCallback(value, ref backing, notification => { - notification(); - } - }); + using (MarkNotifying(property)) + { + notification(); + } + }); + } while (HasPendingSet(property)) { - setterCallback(GetFirstPendingSet(property), notification => + updated |= setterCallback(GetFirstPendingSet(property), ref backing, notification => { using (MarkNotifying(property)) { @@ -139,11 +148,13 @@ namespace Avalonia.Utilities } }); } + return updated; } - else if(pendingSetCondition(value)) + else if(pendingSetCondition(value, ref backing)) { AddPendingSet(property, value); } + return false; } } } diff --git a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs index 3d34bae7eb..2e668fda95 100644 --- a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs @@ -151,14 +151,14 @@ namespace Avalonia.Controls.Primitives { if (_updateCount == 0) { - SetAndRaise(SelectedIndexProperty, (val, notifyWrapper) => + SetAndRaise(SelectedIndexProperty, ref _selectedIndex, (int val, ref int backing, Action notifyWrapper) => { - var old = SelectedIndex; + var old = backing; var effective = (val >= 0 && val < Items?.Cast().Count()) ? val : -1; if (old != effective) { - _selectedIndex = effective; + backing = effective; notifyWrapper(() => RaisePropertyChanged( SelectedIndexProperty, @@ -167,7 +167,7 @@ namespace Avalonia.Controls.Primitives BindingPriority.LocalValue)); SelectedItem = ElementAt(Items, effective); } - }, value, val => val != SelectedIndex); + }, value); } else { @@ -191,15 +191,15 @@ namespace Avalonia.Controls.Primitives { if (_updateCount == 0) { - SetAndRaise(SelectedItemProperty, (val, notifyWrapper) => + SetAndRaise(SelectedItemProperty, ref _selectedItem, (object val, ref object backing, Action notifyWrapper) => { - var old = SelectedItem; + var old = backing; var index = IndexOf(Items, val); var effective = index != -1 ? val : null; if (!object.Equals(effective, old)) { - _selectedItem = effective; + backing = effective; notifyWrapper(() => RaisePropertyChanged( @@ -225,7 +225,7 @@ namespace Avalonia.Controls.Primitives SelectedItems.Clear(); } } - }, value, val => val != SelectedItem); + }, value); } else { diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Direct.cs b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Direct.cs index ffea0fd2d6..6d10f50276 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Direct.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Direct.cs @@ -462,7 +462,7 @@ namespace Avalonia.Base.UnitTests Assert.Equal(1, viewModel.SetterInvokedCount); - //here in real life stack overflow exception is thrown issue #855 and #824 + // Issues #855 and #824 were causing a StackOverflowException at this point. target.DoubleValue = 51.001; Assert.Equal(2, viewModel.SetterInvokedCount); diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/RangeBaseTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/RangeBaseTests.cs index 0e67581760..2dfb30a9f0 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/RangeBaseTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/RangeBaseTests.cs @@ -148,7 +148,7 @@ namespace Avalonia.Controls.UnitTests.Primitives Assert.Equal(1, viewModel.SetterInvokedCount); - //here in real life stack overflow exception is thrown issue #855 and #824 + // Issues #855 and #824 were causing a StackOverflowException at this point. target.Value = 51.001; Assert.Equal(2, viewModel.SetterInvokedCount);