Merge pull request #3556 from AvaloniaUI/fixes/3545-rendertransform-leak

Fix PropertyChanged notifications when bindings complete

src/Avalonia.Base/AvaloniaObject.cs

@@ -478,7 +478,13 @@ namespace Avalonia
             }
         }
 
-        void IValueSink.Completed(AvaloniaProperty property, IPriorityValueEntry entry) { }
+        void IValueSink.Completed<T>(
+            StyledPropertyBase<T> property,
+            IPriorityValueEntry entry,
+            Optional<T> oldValue) 
+        {
+            ((IValueSink)this).ValueChanged(property, BindingPriority.Unset, oldValue, default);
+        }
 
         /// <summary>
         /// Called for each inherited property when the <see cref="InheritanceParent"/> changes.

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)
         {

src/Avalonia.Base/PropertyStore/IValueSink.cs

@@ -15,6 +15,9 @@ namespace Avalonia.PropertyStore
             Optional<T> oldValue,
             BindingValue<T> newValue);
 
-        void Completed(AvaloniaProperty property, IPriorityValueEntry entry);
+        void Completed<T>(
+            StyledPropertyBase<T> property,
+            IPriorityValueEntry entry,
+            Optional<T> oldValue);
     }
 }

src/Avalonia.Base/PropertyStore/PriorityValue.cs

@@ -117,7 +117,10 @@ namespace Avalonia.PropertyStore
             UpdateEffectiveValue();
         }
 
-        void IValueSink.Completed(AvaloniaProperty property, IPriorityValueEntry entry)
+        void IValueSink.Completed<TValue>(
+            StyledPropertyBase<TValue> property,
+            IPriorityValueEntry entry,
+            Optional<TValue> oldValue)
         {
             _entries.Remove((IPriorityValueEntry<T>)entry);
             UpdateEffectiveValue();

src/Avalonia.Base/ValueStore.cs

@@ -148,7 +148,7 @@ namespace Avalonia
                         _values.Remove(property);
                         _sink.ValueChanged(
                             property,
-                            BindingPriority.LocalValue,
+                            BindingPriority.Unset,
                             old,
                             BindingValue<T>.Unset);
                     }
@@ -190,13 +190,17 @@ namespace Avalonia
             _sink.ValueChanged(property, priority, oldValue, newValue);
         }
 
-        void IValueSink.Completed(AvaloniaProperty property, IPriorityValueEntry entry)
+        void IValueSink.Completed<T>(
+            StyledPropertyBase<T> property,
+            IPriorityValueEntry entry,
+            Optional<T> oldValue)
         {
             if (_values.TryGetValue(property, out var slot))
             {
                 if (slot == entry)
                 {
                     _values.Remove(property);
+                    _sink.Completed(property, entry, oldValue);
                 }
             }
         }

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<BindingValue<string>>("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<BindingValue<string>>("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<BindingValue<string>>("foo");
+            var property = Class1.FooProperty;
+            var raised = 0;
+
+            target.Bind(property, new BehaviorSubject<string>("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<BindingValue<string>>("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()
         {

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);

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);

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<Window> run = () =>
+                {
+                    var window = new Window
+                    {
+                        Styles =
+                        {
+                            new Style(x => x.OfType<Canvas>())
+                            {
+                                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<Canvas>(window.Presenter.Child);
+                    Assert.IsType<RotateTransform>(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<Canvas>()).ObjectsCount));
+            }
+        }
+
         private IDisposable Start()
         {
             return UnitTestApplication.Start(TestServices.StyledWindow);