From f16080d00b331c165af114b79417777567c29c49 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 12 Feb 2020 09:36:13 +0100 Subject: [PATCH 1/4] Added failing leak test for #3545. --- tests/Avalonia.LeakTests/ControlTests.cs | 52 ++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tests/Avalonia.LeakTests/ControlTests.cs b/tests/Avalonia.LeakTests/ControlTests.cs index 389b3c8df8..3b288dbf66 100644 --- a/tests/Avalonia.LeakTests/ControlTests.cs +++ b/tests/Avalonia.LeakTests/ControlTests.cs @@ -8,8 +8,10 @@ using Avalonia.Controls; using Avalonia.Controls.Templates; using Avalonia.Diagnostics; using Avalonia.Layout; +using Avalonia.Media; using Avalonia.Platform; using Avalonia.Rendering; +using Avalonia.Styling; using Avalonia.UnitTests; using Avalonia.VisualTree; using JetBrains.dotMemoryUnit; @@ -370,6 +372,56 @@ namespace Avalonia.LeakTests } } + [Fact] + public void Control_With_Style_RenderTransform_Is_Freed() + { + // # Issue #3545 + using (Start()) + { + Func run = () => + { + var window = new Window + { + Styles = + { + new Style(x => x.OfType()) + { + Setters = + { + new Setter + { + Property = Visual.RenderTransformProperty, + Value = new RotateTransform(45), + } + } + } + }, + Content = new Canvas() + }; + + window.Show(); + + // Do a layout and make sure that Canvas gets added to visual tree with + // its render transform. + window.LayoutManager.ExecuteInitialLayoutPass(window); + var canvas = Assert.IsType(window.Presenter.Child); + Assert.IsType(canvas.RenderTransform); + + // Clear the content and ensure the Canvas is removed. + window.Content = null; + window.LayoutManager.ExecuteLayoutPass(); + Assert.Null(window.Presenter.Child); + + return window; + }; + + var result = run(); + + dotMemory.Check(memory => + Assert.Equal(0, memory.GetObjects(where => where.Type.Is()).ObjectsCount)); + } + } + private IDisposable Start() { return UnitTestApplication.Start(TestServices.StyledWindow); From 11a8c01ef14d821586335a3efc962a25bf6dcac2 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 12 Feb 2020 10:21:29 +0100 Subject: [PATCH 2/4] Added failing tests for #3545. `PropertyChanged` is not being fired when binding is disposed. Also change some other unit tests to ensure that priority passed in event args is correct. --- .../AvaloniaObjectTests_Binding.cs | 105 ++++++++++++++++++ .../AvaloniaObjectTests_Direct.cs | 1 + .../AvaloniaObjectTests_SetValue.cs | 1 + 3 files changed, 107 insertions(+) diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs index 4c00d2a1ea..2839fde320 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs @@ -132,6 +132,111 @@ namespace Avalonia.Base.UnitTests Assert.Equal("foo", target.GetValue(property)); } + [Fact] + public void Completing_LocalValue_Binding_Raises_PropertyChanged() + { + var target = new Class1(); + var source = new BehaviorSubject>("foo"); + var property = Class1.FooProperty; + var raised = 0; + + target.Bind(property, source); + Assert.Equal("foo", target.GetValue(property)); + + target.PropertyChanged += (s, e) => + { + Assert.Equal(BindingPriority.Unset, e.Priority); + Assert.Equal(property, e.Property); + Assert.Equal("foo", e.OldValue as string); + Assert.Equal("foodefault", e.NewValue as string); + ++raised; + }; + + source.OnCompleted(); + + Assert.Equal("foodefault", target.GetValue(property)); + Assert.Equal(1, raised); + } + + [Fact] + public void Completing_Style_Binding_Raises_PropertyChanged() + { + var target = new Class1(); + var source = new BehaviorSubject>("foo"); + var property = Class1.FooProperty; + var raised = 0; + + target.Bind(property, source, BindingPriority.Style); + Assert.Equal("foo", target.GetValue(property)); + + target.PropertyChanged += (s, e) => + { + Assert.Equal(BindingPriority.Unset, e.Priority); + Assert.Equal(property, e.Property); + Assert.Equal("foo", e.OldValue as string); + Assert.Equal("foodefault", e.NewValue as string); + ++raised; + }; + + source.OnCompleted(); + + Assert.Equal("foodefault", target.GetValue(property)); + Assert.Equal(1, raised); + } + + [Fact] + public void Completing_LocalValue_Binding_With_Style_Binding_Raises_PropertyChanged() + { + var target = new Class1(); + var source = new BehaviorSubject>("foo"); + var property = Class1.FooProperty; + var raised = 0; + + target.Bind(property, new BehaviorSubject("bar"), BindingPriority.Style); + target.Bind(property, source); + Assert.Equal("foo", target.GetValue(property)); + + target.PropertyChanged += (s, e) => + { + Assert.Equal(BindingPriority.Style, e.Priority); + Assert.Equal(property, e.Property); + Assert.Equal("foo", e.OldValue as string); + Assert.Equal("bar", e.NewValue as string); + ++raised; + }; + + source.OnCompleted(); + + Assert.Equal("bar", target.GetValue(property)); + Assert.Equal(1, raised); + } + + [Fact] + public void Disposing_LocalValue_Binding_Raises_PropertyChanged() + { + var target = new Class1(); + var source = new BehaviorSubject>("foo"); + var property = Class1.FooProperty; + var raised = 0; + + var sub = target.Bind(property, source); + Assert.Equal("foo", target.GetValue(property)); + + target.PropertyChanged += (s, e) => + { + Assert.Equal(BindingPriority.Unset, e.Priority); + Assert.Equal(property, e.Property); + Assert.Equal("foo", e.OldValue as string); + Assert.Equal("foodefault", e.NewValue as string); + ++raised; + }; + + sub.Dispose(); + + Assert.Equal("foodefault", target.GetValue(property)); + Assert.Equal(1, raised); + } + [Fact] public void Setting_Style_Value_Overrides_Binding_Permanently() { diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Direct.cs b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Direct.cs index b1a5b5ae92..ca17afb94f 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Direct.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Direct.cs @@ -188,6 +188,7 @@ namespace Avalonia.Base.UnitTests target.PropertyChanged += (s, e) => { Assert.Same(target, s); + Assert.Equal(BindingPriority.LocalValue, e.Priority); Assert.Equal(Class1.FooProperty, e.Property); Assert.Equal("newvalue", (string)e.OldValue); Assert.Equal("unset", (string)e.NewValue); diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_SetValue.cs b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_SetValue.cs index 40631d04cf..98305cc110 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_SetValue.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_SetValue.cs @@ -30,6 +30,7 @@ namespace Avalonia.Base.UnitTests target.PropertyChanged += (s, e) => { Assert.Same(target, s); + Assert.Equal(BindingPriority.Unset, e.Priority); Assert.Equal(Class1.FooProperty, e.Property); Assert.Equal("newvalue", (string)e.OldValue); Assert.Equal("foodefault", (string)e.NewValue); From 187e45d02134af45dbc65823472b73d3ee3116d1 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 12 Feb 2020 10:22:57 +0100 Subject: [PATCH 3/4] Make ClearLocalValue notify with correct priority. --- src/Avalonia.Base/ValueStore.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Base/ValueStore.cs b/src/Avalonia.Base/ValueStore.cs index 58ebc48652..125f404e2c 100644 --- a/src/Avalonia.Base/ValueStore.cs +++ b/src/Avalonia.Base/ValueStore.cs @@ -148,7 +148,7 @@ namespace Avalonia _values.Remove(property); _sink.ValueChanged( property, - BindingPriority.LocalValue, + BindingPriority.Unset, old, BindingValue.Unset); } From edc8bece4ea2cf8b8296d1e88ed984f8caebb091 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 12 Feb 2020 10:28:01 +0100 Subject: [PATCH 4/4] Raised property changed when binding completes. Fixes #3545 --- src/Avalonia.Base/AvaloniaObject.cs | 8 +++++++- src/Avalonia.Base/PropertyStore/BindingEntry.cs | 4 ++-- src/Avalonia.Base/PropertyStore/IValueSink.cs | 5 ++++- src/Avalonia.Base/PropertyStore/PriorityValue.cs | 5 ++++- src/Avalonia.Base/ValueStore.cs | 6 +++++- 5 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index ddc3d8d081..b0ff591682 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -478,7 +478,13 @@ namespace Avalonia } } - void IValueSink.Completed(AvaloniaProperty property, IPriorityValueEntry entry) { } + void IValueSink.Completed( + StyledPropertyBase property, + IPriorityValueEntry entry, + Optional oldValue) + { + ((IValueSink)this).ValueChanged(property, BindingPriority.Unset, oldValue, default); + } /// /// Called for each inherited property when the changes. diff --git a/src/Avalonia.Base/PropertyStore/BindingEntry.cs b/src/Avalonia.Base/PropertyStore/BindingEntry.cs index 09a0f169df..3249b31d66 100644 --- a/src/Avalonia.Base/PropertyStore/BindingEntry.cs +++ b/src/Avalonia.Base/PropertyStore/BindingEntry.cs @@ -48,10 +48,10 @@ namespace Avalonia.PropertyStore { _subscription?.Dispose(); _subscription = null; - _sink.Completed(Property, this); + _sink.Completed(Property, this, Value); } - public void OnCompleted() => _sink.Completed(Property, this); + public void OnCompleted() => _sink.Completed(Property, this, Value); public void OnError(Exception error) { diff --git a/src/Avalonia.Base/PropertyStore/IValueSink.cs b/src/Avalonia.Base/PropertyStore/IValueSink.cs index 223b0058c1..9012a985ac 100644 --- a/src/Avalonia.Base/PropertyStore/IValueSink.cs +++ b/src/Avalonia.Base/PropertyStore/IValueSink.cs @@ -15,6 +15,9 @@ namespace Avalonia.PropertyStore Optional oldValue, BindingValue newValue); - void Completed(AvaloniaProperty property, IPriorityValueEntry entry); + void Completed( + StyledPropertyBase property, + IPriorityValueEntry entry, + Optional oldValue); } } diff --git a/src/Avalonia.Base/PropertyStore/PriorityValue.cs b/src/Avalonia.Base/PropertyStore/PriorityValue.cs index 2785dc6840..4ef8f650fa 100644 --- a/src/Avalonia.Base/PropertyStore/PriorityValue.cs +++ b/src/Avalonia.Base/PropertyStore/PriorityValue.cs @@ -117,7 +117,10 @@ namespace Avalonia.PropertyStore UpdateEffectiveValue(); } - void IValueSink.Completed(AvaloniaProperty property, IPriorityValueEntry entry) + void IValueSink.Completed( + StyledPropertyBase property, + IPriorityValueEntry entry, + Optional oldValue) { _entries.Remove((IPriorityValueEntry)entry); UpdateEffectiveValue(); diff --git a/src/Avalonia.Base/ValueStore.cs b/src/Avalonia.Base/ValueStore.cs index 125f404e2c..22cded565a 100644 --- a/src/Avalonia.Base/ValueStore.cs +++ b/src/Avalonia.Base/ValueStore.cs @@ -190,13 +190,17 @@ namespace Avalonia _sink.ValueChanged(property, priority, oldValue, newValue); } - void IValueSink.Completed(AvaloniaProperty property, IPriorityValueEntry entry) + void IValueSink.Completed( + StyledPropertyBase property, + IPriorityValueEntry entry, + Optional oldValue) { if (_values.TryGetValue(property, out var slot)) { if (slot == entry) { _values.Remove(property); + _sink.Completed(property, entry, oldValue); } } }