From 0119cf1af16b18e426b727e5e5d665c20e4e4ce5 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 19 Jul 2022 23:00:24 +0200 Subject: [PATCH] Fix a couple of inheritance bugs. With associated unit tests. --- .../PropertyStore/EffectiveValue`1.cs | 18 +++++-- src/Avalonia.Base/PropertyStore/ValueStore.cs | 42 ++++++++--------- .../AvaloniaObjectTests_Inheritance.cs | 47 +++++++++++++++++++ .../ValueStoreTests_Inheritance.cs | 17 +++++++ 4 files changed, 100 insertions(+), 24 deletions(-) diff --git a/src/Avalonia.Base/PropertyStore/EffectiveValue`1.cs b/src/Avalonia.Base/PropertyStore/EffectiveValue`1.cs index 88d47c3dda..50b11bab33 100644 --- a/src/Avalonia.Base/PropertyStore/EffectiveValue`1.cs +++ b/src/Avalonia.Base/PropertyStore/EffectiveValue`1.cs @@ -257,11 +257,23 @@ namespace Avalonia.PropertyStore public void DisposeAndRaiseUnset(ValueStore owner, StyledPropertyBase property) { - var defaultValue = property.GetDefaultValue(owner.GetType()); + BindingPriority priority; + T oldValue; - if (!EqualityComparer.Default.Equals(defaultValue, Value)) + if (property.Inherits && owner.TryGetInheritedValue(property, out var i)) { - owner.Owner.RaisePropertyChanged(property, Value, defaultValue, BindingPriority.Unset, true); + oldValue = ((EffectiveValue)i).Value; + priority = BindingPriority.Inherited; + } + else + { + oldValue = property.GetDefaultValue(owner.GetType()); + priority = BindingPriority.Unset; + } + + if (!EqualityComparer.Default.Equals(oldValue, Value)) + { + owner.Owner.RaisePropertyChanged(property, Value, oldValue, priority, true); if (property.Inherits) owner.OnInheritedEffectiveValueDisposed(property, Value); } diff --git a/src/Avalonia.Base/PropertyStore/ValueStore.cs b/src/Avalonia.Base/PropertyStore/ValueStore.cs index b100fbcf57..aa24ce880e 100644 --- a/src/Avalonia.Base/PropertyStore/ValueStore.cs +++ b/src/Avalonia.Base/PropertyStore/ValueStore.cs @@ -206,6 +206,25 @@ namespace Avalonia.PropertyStore return default; } + public bool TryGetInheritedValue( + AvaloniaProperty property, + [NotNullWhen(true)] out EffectiveValue? result) + { + Debug.Assert(property.Inherits); + + var i = InheritanceAncestor; + + while (i is not null) + { + if (i.TryGetEffectiveValue(property, out result)) + return true; + i = i.InheritanceAncestor; + } + + result = null; + return false; + } + public void SetInheritanceParent(AvaloniaObject? oldParent, AvaloniaObject? newParent) { var values = DictionaryPool.Get(); @@ -651,7 +670,7 @@ namespace Avalonia.PropertyStore if (_effectiveValues is not null && _effectiveValues.Remove(property)) { if (property.Inherits && --_inheritedValueCount == 0) - OnInheritanceAncestorChanged(InheritanceAncestor?.InheritanceAncestor); + OnInheritanceAncestorChanged(InheritanceAncestor); return true; } @@ -663,7 +682,7 @@ namespace Avalonia.PropertyStore if (_effectiveValues is not null && _effectiveValues.Remove(property, out result)) { if (property.Inherits && --_inheritedValueCount == 0) - OnInheritanceAncestorChanged(InheritanceAncestor?.InheritanceAncestor); + OnInheritanceAncestorChanged(InheritanceAncestor); return true; } @@ -898,25 +917,6 @@ namespace Avalonia.PropertyStore return false; } - private bool TryGetInheritedValue( - AvaloniaProperty property, - [NotNullWhen(true)] out EffectiveValue? result) - { - Debug.Assert(property.Inherits); - - var i = InheritanceAncestor; - - while (i is not null) - { - if (i.TryGetEffectiveValue(property, out result)) - return true; - i = i.InheritanceAncestor; - } - - result = null; - return false; - } - private EffectiveValue? GetEffectiveValue(AvaloniaProperty property) { if (_effectiveValues is not null && _effectiveValues.TryGetValue(property, out var value)) diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Inheritance.cs b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Inheritance.cs index 09832e2c1d..d0768eae1f 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Inheritance.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Inheritance.cs @@ -43,6 +43,53 @@ namespace Avalonia.Base.UnitTests Assert.Equal("bazdefault", child.GetValue(Class1.BazProperty)); } + [Fact] + public void ClearValue_On_Parent_Raises_PropertyChanged_On_Child() + { + Class1 parent = new Class1(); + Class2 child = new Class2 { Parent = parent }; + var raised = 0; + + parent.SetValue(Class1.BazProperty, "changed"); + + child.PropertyChanged += (s, e) => + { + Assert.Same(child, e.Sender); + Assert.Equal("changed", e.OldValue); + Assert.Equal("bazdefault", e.NewValue); + Assert.Equal(BindingPriority.Inherited, e.Priority); + ++raised; + }; + + parent.ClearValue(Class1.BazProperty); + + Assert.Equal(1, raised); + } + + [Fact] + public void ClearValue_On_Child_Raises_PropertyChanged_With_Inherited_Parent_Value() + { + Class1 parent = new Class1(); + Class2 child = new Class2 { Parent = parent }; + var raised = 0; + + parent.SetValue(Class1.BazProperty, "parent"); + child.SetValue(Class1.BazProperty, "child"); + + child.PropertyChanged += (s, e) => + { + Assert.Same(child, e.Sender); + Assert.Equal("child", e.OldValue); + Assert.Equal("parent", e.NewValue); + Assert.Equal(BindingPriority.Inherited, e.Priority); + ++raised; + }; + + child.ClearValue(Class1.BazProperty); + + Assert.Equal(1, raised); + } + [Fact] public void Setting_InheritanceParent_Raises_PropertyChanged_When_Parent_Has_Value_Set() { diff --git a/tests/Avalonia.Base.UnitTests/PropertyStore/ValueStoreTests_Inheritance.cs b/tests/Avalonia.Base.UnitTests/PropertyStore/ValueStoreTests_Inheritance.cs index 549cd6fa73..ed122e4ddc 100644 --- a/tests/Avalonia.Base.UnitTests/PropertyStore/ValueStoreTests_Inheritance.cs +++ b/tests/Avalonia.Base.UnitTests/PropertyStore/ValueStoreTests_Inheritance.cs @@ -80,6 +80,23 @@ namespace Avalonia.Base.UnitTests.PropertyStore Assert.Same(child.GetValueStore(), grandchild.GetValueStore().InheritanceAncestor); } + [Fact] + public void Clearing_Value_In_Child_Updates_InheritanceAncestor() + { + var parent = new Class1(); + var child = new Class1 { Parent = parent }; + var grandchild = new Class1 { Parent = child }; + + parent.Foo = "changed"; + child.Foo = "foochanged"; + child.ClearValue(Class1.FooProperty); + + var parentStore = parent.GetValueStore(); + Assert.Null(parentStore.InheritanceAncestor); + Assert.Same(parentStore, child.GetValueStore().InheritanceAncestor); + Assert.Same(parentStore, grandchild.GetValueStore().InheritanceAncestor); + } + [Fact] public void Adding_Child_Sets_InheritanceAncestor() {