From 104f170fa888d32500c8e59371df15911c765f78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Su=C3=A1rez?= Date: Thu, 9 Mar 2023 14:30:49 +0100 Subject: [PATCH 01/12] Correctly propagate BindingContext to Border StrokeShape --- .../Core/BorderGalleries/BorderStyles.xaml | 25 +++++- src/Controls/src/Core/Border.cs | 76 ++++++++++++++++++- .../net-android/PublicAPI.Unshipped.txt | 1 + .../PublicAPI/net-ios/PublicAPI.Unshipped.txt | 1 + .../net-maccatalyst/PublicAPI.Unshipped.txt | 1 + .../net-windows/PublicAPI.Unshipped.txt | 1 + .../PublicAPI/net/PublicAPI.Unshipped.txt | 1 + .../netstandard/PublicAPI.Unshipped.txt | 1 + 8 files changed, 103 insertions(+), 4 deletions(-) diff --git a/src/Controls/samples/Controls.Sample/Pages/Core/BorderGalleries/BorderStyles.xaml b/src/Controls/samples/Controls.Sample/Pages/Core/BorderGalleries/BorderStyles.xaml index 9f59158a1cf7..b666a12be534 100644 --- a/src/Controls/samples/Controls.Sample/Pages/Core/BorderGalleries/BorderStyles.xaml +++ b/src/Controls/samples/Controls.Sample/Pages/Core/BorderGalleries/BorderStyles.xaml @@ -7,7 +7,7 @@ - + + - + \ No newline at end of file diff --git a/src/Controls/src/Core/Border.cs b/src/Controls/src/Core/Border.cs index dd406a7286d5..f6c46b6aed0d 100644 --- a/src/Controls/src/Core/Border.cs +++ b/src/Controls/src/Core/Border.cs @@ -1,4 +1,5 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Collections.ObjectModel; using System.Collections.Specialized; using System.Runtime.CompilerServices; @@ -14,6 +15,10 @@ public class Border : View, IContentView, IBorderView, IPaddingElement float[]? _strokeDashPattern; ReadOnlyCollection? _logicalChildren; + readonly WeakStrokeShapeChangedProxy _strokeShapeProxy = new(); + + ~Border() => _strokeShapeProxy.Unsubscribe(); + internal ObservableCollection InternalChildren { get; } = new(); internal override IReadOnlyList LogicalChildrenInternal => @@ -37,7 +42,41 @@ public Thickness Padding } public static readonly BindableProperty StrokeShapeProperty = - BindableProperty.Create(nameof(StrokeShape), typeof(IShape), typeof(Border), new Rectangle()); + BindableProperty.Create(nameof(StrokeShape), typeof(IShape), typeof(Border), new Rectangle(), + propertyChanging: (bindable, oldvalue, newvalue) => + { + if (newvalue is not null) + (bindable as Border)?.StopNotifyingStrokeShapeChanges(); + }, + propertyChanged: (bindable, oldvalue, newvalue) => + { + if (newvalue is not null) + (bindable as Border)?.NotifyStrokeShapeChanges(); + }); + + void NotifyStrokeShapeChanges() + { + var strokeShape = StrokeShape; + + if (strokeShape is VisualElement visualElement) + { + SetInheritedBindingContext(visualElement, BindingContext); + visualElement.Parent = this; + _strokeShapeProxy.Subscribe(visualElement, (sender, e) => OnPropertyChanged(nameof(StrokeShape))); + } + } + + void StopNotifyingStrokeShapeChanges() + { + var strokeShape = StrokeShape; + + if (strokeShape is VisualElement visualElement) + { + SetInheritedBindingContext(visualElement, null); + visualElement.Parent = null; + _strokeShapeProxy.Unsubscribe(); + } + } public static readonly BindableProperty StrokeProperty = BindableProperty.Create(nameof(Stroke), typeof(Brush), typeof(Border), null); @@ -224,5 +263,38 @@ void OnStrokeDashArrayChanged(object? sender, NotifyCollectionChangedEventArgs e { Handler?.UpdateValue(nameof(IBorderStroke.StrokeDashPattern)); } + + class WeakStrokeShapeChangedProxy : WeakEventProxy + { + void OnStrokeShapeChanged(object? sender, EventArgs e) + { + if (TryGetHandler(out var handler)) + { + handler(sender, e); + } + else + { + Unsubscribe(); + } + } + + public override void Subscribe(VisualElement source, EventHandler handler) + { + if (TryGetSource(out var s)) + s.PropertyChanged -= OnStrokeShapeChanged; + + source.PropertyChanged += OnStrokeShapeChanged; + + base.Subscribe(source, handler); + } + + public override void Unsubscribe() + { + if (TryGetSource(out var s)) + s.PropertyChanged -= OnStrokeShapeChanged; + + base.Unsubscribe(); + } + } } } \ No newline at end of file diff --git a/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt index 36ff82c30d4a..48c6867aa463 100644 --- a/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt @@ -1,5 +1,6 @@ #nullable enable *REMOVED*override Microsoft.Maui.Controls.RefreshView.MeasureOverride(double widthConstraint, double heightConstraint) -> Microsoft.Maui.Graphics.Size +Microsoft.Maui.Controls.Border.~Border() -> void Microsoft.Maui.Controls.Handlers.Compatibility.FrameRenderer.FrameRenderer(Android.Content.Context! context, Microsoft.Maui.IPropertyMapper! mapper) -> void Microsoft.Maui.Controls.Handlers.Compatibility.FrameRenderer.FrameRenderer(Android.Content.Context! context, Microsoft.Maui.IPropertyMapper! mapper, Microsoft.Maui.CommandMapper! commandMapper) -> void Microsoft.Maui.Controls.LayoutOptions.Equals(Microsoft.Maui.Controls.LayoutOptions other) -> bool diff --git a/src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt index 49adee8be5ef..c47cd1f5d943 100644 --- a/src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt @@ -1,5 +1,6 @@ #nullable enable *REMOVED*override Microsoft.Maui.Controls.RefreshView.MeasureOverride(double widthConstraint, double heightConstraint) -> Microsoft.Maui.Graphics.Size +Microsoft.Maui.Controls.Border.~Border() -> void Microsoft.Maui.Controls.LayoutOptions.Equals(Microsoft.Maui.Controls.LayoutOptions other) -> bool Microsoft.Maui.Controls.Region.Equals(Microsoft.Maui.Controls.Region other) -> bool Microsoft.Maui.Controls.Shapes.Matrix.Equals(Microsoft.Maui.Controls.Shapes.Matrix other) -> bool diff --git a/src/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt index 4f0f12ef2002..2ea1f49cd6e0 100644 --- a/src/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt @@ -1,5 +1,6 @@ #nullable enable *REMOVED*override Microsoft.Maui.Controls.RefreshView.MeasureOverride(double widthConstraint, double heightConstraint) -> Microsoft.Maui.Graphics.Size +Microsoft.Maui.Controls.Border.~Border() -> void Microsoft.Maui.Controls.LayoutOptions.Equals(Microsoft.Maui.Controls.LayoutOptions other) -> bool Microsoft.Maui.Controls.Region.Equals(Microsoft.Maui.Controls.Region other) -> bool Microsoft.Maui.Controls.Shapes.Matrix.Equals(Microsoft.Maui.Controls.Shapes.Matrix other) -> bool diff --git a/src/Controls/src/Core/PublicAPI/net-windows/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net-windows/PublicAPI.Unshipped.txt index 0cc1bd8a4665..b78d31e36f5b 100644 --- a/src/Controls/src/Core/PublicAPI/net-windows/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net-windows/PublicAPI.Unshipped.txt @@ -1,5 +1,6 @@ #nullable enable *REMOVED*override Microsoft.Maui.Controls.RefreshView.MeasureOverride(double widthConstraint, double heightConstraint) -> Microsoft.Maui.Graphics.Size +Microsoft.Maui.Controls.Border.~Border() -> void Microsoft.Maui.Controls.LayoutOptions.Equals(Microsoft.Maui.Controls.LayoutOptions other) -> bool Microsoft.Maui.Controls.Region.Equals(Microsoft.Maui.Controls.Region other) -> bool Microsoft.Maui.Controls.Shapes.Matrix.Equals(Microsoft.Maui.Controls.Shapes.Matrix other) -> bool diff --git a/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt index 49eeb11a69b7..70a56b9bf967 100644 --- a/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt @@ -1,5 +1,6 @@ #nullable enable *REMOVED*override Microsoft.Maui.Controls.RefreshView.MeasureOverride(double widthConstraint, double heightConstraint) -> Microsoft.Maui.Graphics.Size +Microsoft.Maui.Controls.Border.~Border() -> void Microsoft.Maui.Controls.LayoutOptions.Equals(Microsoft.Maui.Controls.LayoutOptions other) -> bool Microsoft.Maui.Controls.Region.Equals(Microsoft.Maui.Controls.Region other) -> bool Microsoft.Maui.Controls.Shapes.Matrix.Equals(Microsoft.Maui.Controls.Shapes.Matrix other) -> bool diff --git a/src/Controls/src/Core/PublicAPI/netstandard/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/netstandard/PublicAPI.Unshipped.txt index 328746133294..4e595b543ae6 100644 --- a/src/Controls/src/Core/PublicAPI/netstandard/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/netstandard/PublicAPI.Unshipped.txt @@ -1,5 +1,6 @@ #nullable enable *REMOVED*override Microsoft.Maui.Controls.RefreshView.MeasureOverride(double widthConstraint, double heightConstraint) -> Microsoft.Maui.Graphics.Size +Microsoft.Maui.Controls.Border.~Border() -> void Microsoft.Maui.Controls.LayoutOptions.Equals(Microsoft.Maui.Controls.LayoutOptions other) -> bool Microsoft.Maui.Controls.Region.Equals(Microsoft.Maui.Controls.Region other) -> bool Microsoft.Maui.Controls.Shapes.Matrix.Equals(Microsoft.Maui.Controls.Shapes.Matrix other) -> bool From c88665e75e414c25c20bba79040c103438a46f63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Su=C3=A1rez?= Date: Thu, 9 Mar 2023 14:36:29 +0100 Subject: [PATCH 02/12] Added unit test --- src/Core/tests/UnitTests/Views/BorderTests.cs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/Core/tests/UnitTests/Views/BorderTests.cs b/src/Core/tests/UnitTests/Views/BorderTests.cs index 459c5efc2be2..e7b0a5312260 100644 --- a/src/Core/tests/UnitTests/Views/BorderTests.cs +++ b/src/Core/tests/UnitTests/Views/BorderTests.cs @@ -1,4 +1,5 @@ using Microsoft.Maui.Controls; +using Microsoft.Maui.Controls.Shapes; using Xunit; namespace Microsoft.Maui.UnitTests.Views @@ -29,6 +30,23 @@ public void TestBorderPropagateBindingContext() Assert.True(border.BindingContext == bindingContext); } + [Fact] + public void TestStrokeShapeBindingContext() + { + var context = new object(); + + var parent = new Border + { + BindingContext = context + }; + + var strokeShape = new Rectangle(); + + parent.StrokeShape = strokeShape; + + Assert.Same(context, ((Rectangle)parent.StrokeShape).BindingContext); + } + [Fact] public void TestSetChild() { From 2e8587eba7ab2f4afb193ad87e671f8f346841b5 Mon Sep 17 00:00:00 2001 From: GitHub Actions Autoformatter Date: Thu, 9 Mar 2023 13:41:52 +0000 Subject: [PATCH 03/12] Auto-format source code --- src/Controls/src/Core/Border.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Controls/src/Core/Border.cs b/src/Controls/src/Core/Border.cs index f6c46b6aed0d..7a3706a7e629 100644 --- a/src/Controls/src/Core/Border.cs +++ b/src/Controls/src/Core/Border.cs @@ -282,7 +282,7 @@ public override void Subscribe(VisualElement source, EventHandler handler) { if (TryGetSource(out var s)) s.PropertyChanged -= OnStrokeShapeChanged; - + source.PropertyChanged += OnStrokeShapeChanged; base.Subscribe(source, handler); @@ -292,7 +292,7 @@ public override void Unsubscribe() { if (TryGetSource(out var s)) s.PropertyChanged -= OnStrokeShapeChanged; - + base.Unsubscribe(); } } From ab753059b257b8472fbefdc1a7b1ba9380359fb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Su=C3=A1rez?= Date: Thu, 9 Mar 2023 15:43:15 +0100 Subject: [PATCH 04/12] Fix build errors --- .../src/Core/PublicAPI/net-tizen/PublicAPI.Unshipped.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Controls/src/Core/PublicAPI/net-tizen/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net-tizen/PublicAPI.Unshipped.txt index a8e116cd8486..6a97a37dac85 100644 --- a/src/Controls/src/Core/PublicAPI/net-tizen/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net-tizen/PublicAPI.Unshipped.txt @@ -1,4 +1,5 @@ #nullable enable +Microsoft.Maui.Controls.Border.~Border() -> void Microsoft.Maui.Controls.LayoutOptions.Equals(Microsoft.Maui.Controls.LayoutOptions other) -> bool Microsoft.Maui.Controls.Region.Equals(Microsoft.Maui.Controls.Region other) -> bool Microsoft.Maui.Controls.Shapes.Matrix.Equals(Microsoft.Maui.Controls.Shapes.Matrix other) -> bool From ac8e299973f8396451582115c39511a050da63c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Su=C3=A1rez?= Date: Tue, 14 Mar 2023 13:31:30 +0100 Subject: [PATCH 05/12] Added more tests --- src/Controls/src/Core/Border.cs | 9 ++--- src/Core/tests/UnitTests/Views/BorderTests.cs | 36 ++++++++++++++++++- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/src/Controls/src/Core/Border.cs b/src/Controls/src/Core/Border.cs index 7a3706a7e629..e5403343c684 100644 --- a/src/Controls/src/Core/Border.cs +++ b/src/Controls/src/Core/Border.cs @@ -15,9 +15,9 @@ public class Border : View, IContentView, IBorderView, IPaddingElement float[]? _strokeDashPattern; ReadOnlyCollection? _logicalChildren; - readonly WeakStrokeShapeChangedProxy _strokeShapeProxy = new(); + WeakStrokeShapeChangedProxy? _strokeShapeProxy = null; - ~Border() => _strokeShapeProxy.Unsubscribe(); + ~Border() => _strokeShapeProxy?.Unsubscribe(); internal ObservableCollection InternalChildren { get; } = new(); @@ -62,7 +62,8 @@ void NotifyStrokeShapeChanges() { SetInheritedBindingContext(visualElement, BindingContext); visualElement.Parent = this; - _strokeShapeProxy.Subscribe(visualElement, (sender, e) => OnPropertyChanged(nameof(StrokeShape))); + var proxy = _strokeShapeProxy ??= new(); + proxy.Subscribe(visualElement, (sender, e) => OnPropertyChanged(nameof(StrokeShape))); } } @@ -74,7 +75,7 @@ void StopNotifyingStrokeShapeChanges() { SetInheritedBindingContext(visualElement, null); visualElement.Parent = null; - _strokeShapeProxy.Unsubscribe(); + _strokeShapeProxy?.Unsubscribe(); } } diff --git a/src/Core/tests/UnitTests/Views/BorderTests.cs b/src/Core/tests/UnitTests/Views/BorderTests.cs index e7b0a5312260..a7aeb721c6e2 100644 --- a/src/Core/tests/UnitTests/Views/BorderTests.cs +++ b/src/Core/tests/UnitTests/Views/BorderTests.cs @@ -1,4 +1,6 @@ -using Microsoft.Maui.Controls; +using System; +using System.Threading.Tasks; +using Microsoft.Maui.Controls; using Microsoft.Maui.Controls.Shapes; using Xunit; @@ -47,6 +49,38 @@ public void TestStrokeShapeBindingContext() Assert.Same(context, ((Rectangle)parent.StrokeShape).BindingContext); } + [Fact] + public void BorderStrokeShapeSubscribed() + { + var strokeShape = new RoundRectangle { CornerRadius = new CornerRadius(12) }; + var border = new Border { StrokeShape = strokeShape }; + + bool fired = false; + border.PropertyChanged += (sender, e) => + { + if (e.PropertyName == nameof(Border.StrokeShape)) + fired = true; + }; + + strokeShape.CornerRadius = new CornerRadius(24); + Assert.True(fired, "PropertyChanged did not fire!"); + } + + [Theory(Skip = "Reviewing why is failing")] + [InlineData(typeof(Rectangle))] + [InlineData(typeof(Ellipse))] + public async Task BorderStrokeShapeDoesNotLeak(Type type) + { + var strokeShape = (Shape)Activator.CreateInstance(type); + var reference = new WeakReference(new Border { StrokeShape = strokeShape }); + + await Task.Yield(); + GC.Collect(); + GC.WaitForPendingFinalizers(); + + Assert.False(reference.IsAlive, "Border should not be alive!"); + } + [Fact] public void TestSetChild() { From aad92c980030d5f300c492875290f521d9d53d70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Su=C3=A1rez?= Date: Tue, 14 Mar 2023 13:44:31 +0100 Subject: [PATCH 06/12] Created generic WeakNotifyPropertyChangedProxy --- src/Controls/src/Core/Border.cs | 35 +------------ .../src/Core/Internals/WeakEventProxy.cs | 50 +++++++++++++++++++ 2 files changed, 51 insertions(+), 34 deletions(-) diff --git a/src/Controls/src/Core/Border.cs b/src/Controls/src/Core/Border.cs index e5403343c684..826a96bbfb9f 100644 --- a/src/Controls/src/Core/Border.cs +++ b/src/Controls/src/Core/Border.cs @@ -15,7 +15,7 @@ public class Border : View, IContentView, IBorderView, IPaddingElement float[]? _strokeDashPattern; ReadOnlyCollection? _logicalChildren; - WeakStrokeShapeChangedProxy? _strokeShapeProxy = null; + WeakNotifyPropertyChangedProxy? _strokeShapeProxy = null; ~Border() => _strokeShapeProxy?.Unsubscribe(); @@ -264,38 +264,5 @@ void OnStrokeDashArrayChanged(object? sender, NotifyCollectionChangedEventArgs e { Handler?.UpdateValue(nameof(IBorderStroke.StrokeDashPattern)); } - - class WeakStrokeShapeChangedProxy : WeakEventProxy - { - void OnStrokeShapeChanged(object? sender, EventArgs e) - { - if (TryGetHandler(out var handler)) - { - handler(sender, e); - } - else - { - Unsubscribe(); - } - } - - public override void Subscribe(VisualElement source, EventHandler handler) - { - if (TryGetSource(out var s)) - s.PropertyChanged -= OnStrokeShapeChanged; - - source.PropertyChanged += OnStrokeShapeChanged; - - base.Subscribe(source, handler); - } - - public override void Unsubscribe() - { - if (TryGetSource(out var s)) - s.PropertyChanged -= OnStrokeShapeChanged; - - base.Unsubscribe(); - } - } } } \ No newline at end of file diff --git a/src/Controls/src/Core/Internals/WeakEventProxy.cs b/src/Controls/src/Core/Internals/WeakEventProxy.cs index 15e44d825a4d..fb0b66e00b2c 100644 --- a/src/Controls/src/Core/Internals/WeakEventProxy.cs +++ b/src/Controls/src/Core/Internals/WeakEventProxy.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Specialized; +using System.ComponentModel; // NOTE: warning disabled for netstandard projects #pragma warning disable 0436 @@ -102,4 +103,53 @@ public override void Unsubscribe() base.Unsubscribe(); } } + + /// + /// A "proxy" class for subscribing INotifyPropertyChanged via WeakReference. + /// General usage is to store this in a member variable and call Subscribe()/Unsubscribe() appropriately. + /// Your class should have a finalizer that calls Unsubscribe() to prevent WeakNotifyCollectionChangedProxy objects from leaking. + /// + class WeakNotifyPropertyChangedProxy : WeakEventProxy + { + public WeakNotifyPropertyChangedProxy() { } + + public WeakNotifyPropertyChangedProxy(INotifyPropertyChanged source, PropertyChangedEventHandler handler) + { + Subscribe(source, handler); + } + + void OnPropertyChanged(object? sender, PropertyChangedEventArgs e) + { + if (TryGetHandler(out var handler)) + { + handler(sender, e); + } + else + { + Unsubscribe(); + } + } + + public override void Subscribe(INotifyPropertyChanged source, PropertyChangedEventHandler handler) + { + if (TryGetSource(out var s)) + { + s.PropertyChanged -= OnPropertyChanged; + } + + source.PropertyChanged += OnPropertyChanged; + + base.Subscribe(source, handler); + } + + public override void Unsubscribe() + { + if (TryGetSource(out var s)) + { + s.PropertyChanged -= OnPropertyChanged; + } + + base.Unsubscribe(); + } + } } From 18b2c2082b912d6e27ed4d66eeb63a8a046aadf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Su=C3=A1rez?= Date: Tue, 14 Mar 2023 16:37:13 +0100 Subject: [PATCH 07/12] Updated border --- src/Controls/src/Core/Border.cs | 45 +++++++++++++++++-- src/Core/tests/UnitTests/Views/BorderTests.cs | 18 ++++++++ 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/src/Controls/src/Core/Border.cs b/src/Controls/src/Core/Border.cs index 826a96bbfb9f..be99f795f5df 100644 --- a/src/Controls/src/Core/Border.cs +++ b/src/Controls/src/Core/Border.cs @@ -1,5 +1,4 @@ -using System; -using System.Collections.Generic; +using System.Collections.Generic; using System.Collections.ObjectModel; using System.Collections.Specialized; using System.Runtime.CompilerServices; @@ -16,8 +15,13 @@ public class Border : View, IContentView, IBorderView, IPaddingElement ReadOnlyCollection? _logicalChildren; WeakNotifyPropertyChangedProxy? _strokeShapeProxy = null; + WeakNotifyPropertyChangedProxy? _strokeProxy = null; - ~Border() => _strokeShapeProxy?.Unsubscribe(); + ~Border() + { + _strokeShapeProxy?.Unsubscribe(); + _strokeProxy?.Unsubscribe(); + } internal ObservableCollection InternalChildren { get; } = new(); @@ -80,7 +84,40 @@ void StopNotifyingStrokeShapeChanges() } public static readonly BindableProperty StrokeProperty = - BindableProperty.Create(nameof(Stroke), typeof(Brush), typeof(Border), null); + BindableProperty.Create(nameof(Stroke), typeof(Brush), typeof(Border), null, + propertyChanging: (bindable, oldvalue, newvalue) => + { + if (newvalue is not null) + (bindable as Border)?.StopNotifyingStrokeChanges(); + }, + propertyChanged: (bindable, oldvalue, newvalue) => + { + if (newvalue is not null) + (bindable as Border)?.NotifyStrokeChanges(); + }); + + void NotifyStrokeChanges() + { + var stroke = Stroke; + + if (stroke is not null) + { + SetInheritedBindingContext(stroke, BindingContext); + var proxy = _strokeProxy ??= new(); + proxy.Subscribe(stroke, (sender, e) => OnPropertyChanged(nameof(Stroke))); + } + } + + void StopNotifyingStrokeChanges() + { + var stroke = Stroke; + + if (stroke is not null) + { + SetInheritedBindingContext(stroke, null); + _strokeProxy?.Unsubscribe(); + } + } public static readonly BindableProperty StrokeThicknessProperty = BindableProperty.Create(nameof(StrokeThickness), typeof(double), typeof(Border), 1.0, propertyChanged: StrokeThicknessChanged); diff --git a/src/Core/tests/UnitTests/Views/BorderTests.cs b/src/Core/tests/UnitTests/Views/BorderTests.cs index a7aeb721c6e2..81b00f2bdfb6 100644 --- a/src/Core/tests/UnitTests/Views/BorderTests.cs +++ b/src/Core/tests/UnitTests/Views/BorderTests.cs @@ -2,6 +2,7 @@ using System.Threading.Tasks; using Microsoft.Maui.Controls; using Microsoft.Maui.Controls.Shapes; +using Microsoft.Maui.Graphics; using Xunit; namespace Microsoft.Maui.UnitTests.Views @@ -66,6 +67,23 @@ public void BorderStrokeShapeSubscribed() Assert.True(fired, "PropertyChanged did not fire!"); } + [Fact] + public void BorderStrokeSubscribed() + { + var stroke = new SolidColorBrush(Colors.Red); + var border = new Border { Stroke = stroke }; + + bool fired = false; + border.PropertyChanged += (sender, e) => + { + if (e.PropertyName == nameof(Border.Stroke)) + fired = true; + }; + + stroke.Color = Colors.Green; + Assert.True(fired, "PropertyChanged did not fire!"); + } + [Theory(Skip = "Reviewing why is failing")] [InlineData(typeof(Rectangle))] [InlineData(typeof(Ellipse))] From 68a1f653ffdf91f36f78d8bff429fbfcbe5b8415 Mon Sep 17 00:00:00 2001 From: GitHub Actions Autoformatter Date: Tue, 14 Mar 2023 15:41:15 +0000 Subject: [PATCH 08/12] Auto-format source code --- src/Controls/src/Core/Border.cs | 2 +- src/Core/tests/UnitTests/Views/BorderTests.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Controls/src/Core/Border.cs b/src/Controls/src/Core/Border.cs index be99f795f5df..688c09cde376 100644 --- a/src/Controls/src/Core/Border.cs +++ b/src/Controls/src/Core/Border.cs @@ -84,7 +84,7 @@ void StopNotifyingStrokeShapeChanges() } public static readonly BindableProperty StrokeProperty = - BindableProperty.Create(nameof(Stroke), typeof(Brush), typeof(Border), null, + BindableProperty.Create(nameof(Stroke), typeof(Brush), typeof(Border), null, propertyChanging: (bindable, oldvalue, newvalue) => { if (newvalue is not null) diff --git a/src/Core/tests/UnitTests/Views/BorderTests.cs b/src/Core/tests/UnitTests/Views/BorderTests.cs index 81b00f2bdfb6..23e1f151a4fe 100644 --- a/src/Core/tests/UnitTests/Views/BorderTests.cs +++ b/src/Core/tests/UnitTests/Views/BorderTests.cs @@ -70,7 +70,7 @@ public void BorderStrokeShapeSubscribed() [Fact] public void BorderStrokeSubscribed() { - var stroke = new SolidColorBrush(Colors.Red); + var stroke = new SolidColorBrush(Colors.Red); var border = new Border { Stroke = stroke }; bool fired = false; From 8509689956240bfe3410981826397f75b475be78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Su=C3=A1rez?= Date: Tue, 14 Mar 2023 16:58:06 +0100 Subject: [PATCH 09/12] Changes in Border --- src/Controls/src/Core/Border.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Controls/src/Core/Border.cs b/src/Controls/src/Core/Border.cs index be99f795f5df..038f1b03363a 100644 --- a/src/Controls/src/Core/Border.cs +++ b/src/Controls/src/Core/Border.cs @@ -100,6 +100,9 @@ void NotifyStrokeChanges() { var stroke = Stroke; + if (stroke is ImmutableBrush) + return; + if (stroke is not null) { SetInheritedBindingContext(stroke, BindingContext); @@ -112,6 +115,9 @@ void StopNotifyingStrokeChanges() { var stroke = Stroke; + if (stroke is ImmutableBrush) + return; + if (stroke is not null) { SetInheritedBindingContext(stroke, null); From a6d5419c230b859c1146d7cdb1dcd0a88500fa7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Su=C3=A1rez?= Date: Wed, 15 Mar 2023 11:48:53 +0100 Subject: [PATCH 10/12] Update test --- src/Core/tests/UnitTests/Views/BorderTests.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Core/tests/UnitTests/Views/BorderTests.cs b/src/Core/tests/UnitTests/Views/BorderTests.cs index 23e1f151a4fe..fa307a8e518d 100644 --- a/src/Core/tests/UnitTests/Views/BorderTests.cs +++ b/src/Core/tests/UnitTests/Views/BorderTests.cs @@ -84,14 +84,17 @@ public void BorderStrokeSubscribed() Assert.True(fired, "PropertyChanged did not fire!"); } - [Theory(Skip = "Reviewing why is failing")] + [Theory] [InlineData(typeof(Rectangle))] + [InlineData(typeof(RoundRectangle))] [InlineData(typeof(Ellipse))] public async Task BorderStrokeShapeDoesNotLeak(Type type) { var strokeShape = (Shape)Activator.CreateInstance(type); var reference = new WeakReference(new Border { StrokeShape = strokeShape }); + strokeShape = null; + await Task.Yield(); GC.Collect(); GC.WaitForPendingFinalizers(); From 769e71a1085f639f9ac87f06c821be462c11b3bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Su=C3=A1rez?= Date: Mon, 20 Mar 2023 10:04:35 +0100 Subject: [PATCH 11/12] Fix broken tests --- src/Controls/src/Core/Border.cs | 13 +++++++++---- src/Core/tests/UnitTests/Views/BorderTests.cs | 15 +++++++++++++-- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/Controls/src/Core/Border.cs b/src/Controls/src/Core/Border.cs index 40384c177c63..837be5d8fa9b 100644 --- a/src/Controls/src/Core/Border.cs +++ b/src/Controls/src/Core/Border.cs @@ -1,6 +1,7 @@ using System.Collections.Generic; using System.Collections.ObjectModel; using System.Collections.Specialized; +using System.ComponentModel; using System.Runtime.CompilerServices; using Microsoft.Maui.Controls.Shapes; using Microsoft.Maui.Graphics; @@ -15,7 +16,9 @@ public class Border : View, IContentView, IBorderView, IPaddingElement ReadOnlyCollection? _logicalChildren; WeakNotifyPropertyChangedProxy? _strokeShapeProxy = null; + PropertyChangedEventHandler? _strokeShapeChanged; WeakNotifyPropertyChangedProxy? _strokeProxy = null; + PropertyChangedEventHandler? _strokeChanged; ~Border() { @@ -66,8 +69,9 @@ void NotifyStrokeShapeChanges() { SetInheritedBindingContext(visualElement, BindingContext); visualElement.Parent = this; - var proxy = _strokeShapeProxy ??= new(); - proxy.Subscribe(visualElement, (sender, e) => OnPropertyChanged(nameof(StrokeShape))); + _strokeShapeChanged ??= (sender, e) => OnPropertyChanged(nameof(StrokeShape)); + _strokeShapeProxy ??= new(); + _strokeShapeProxy.Subscribe(visualElement, _strokeShapeChanged); } } @@ -106,8 +110,9 @@ void NotifyStrokeChanges() if (stroke is not null) { SetInheritedBindingContext(stroke, BindingContext); - var proxy = _strokeProxy ??= new(); - proxy.Subscribe(stroke, (sender, e) => OnPropertyChanged(nameof(Stroke))); + _strokeChanged ??= (sender, e) => OnPropertyChanged(nameof(Stroke)); + _strokeProxy ??= new(); + _strokeProxy.Subscribe(stroke, _strokeChanged); } } diff --git a/src/Core/tests/UnitTests/Views/BorderTests.cs b/src/Core/tests/UnitTests/Views/BorderTests.cs index fa307a8e518d..958ccfdd4188 100644 --- a/src/Core/tests/UnitTests/Views/BorderTests.cs +++ b/src/Core/tests/UnitTests/Views/BorderTests.cs @@ -51,7 +51,7 @@ public void TestStrokeShapeBindingContext() } [Fact] - public void BorderStrokeShapeSubscribed() + public async Task BorderStrokeShapeSubscribed() { var strokeShape = new RoundRectangle { CornerRadius = new CornerRadius(12) }; var border = new Border { StrokeShape = strokeShape }; @@ -63,12 +63,18 @@ public void BorderStrokeShapeSubscribed() fired = true; }; + await Task.Yield(); + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.KeepAlive(border); + strokeShape.CornerRadius = new CornerRadius(24); + Assert.True(fired, "PropertyChanged did not fire!"); } [Fact] - public void BorderStrokeSubscribed() + public async Task BorderStrokeSubscribed() { var stroke = new SolidColorBrush(Colors.Red); var border = new Border { Stroke = stroke }; @@ -80,6 +86,11 @@ public void BorderStrokeSubscribed() fired = true; }; + await Task.Yield(); + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.KeepAlive(border); + stroke.Color = Colors.Green; Assert.True(fired, "PropertyChanged did not fire!"); } From 6da1ed13ae9a92ca59e46e5181e95acaf12ea72b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Su=C3=A1rez?= Date: Tue, 21 Mar 2023 11:16:37 +0100 Subject: [PATCH 12/12] Added sample to validate #13988 --- .../Core/BorderGalleries/BorderStyles.xaml | 31 +++++++++++++++++-- .../Core/BorderGalleries/BorderStyles.xaml.cs | 19 +++++++++++- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/src/Controls/samples/Controls.Sample/Pages/Core/BorderGalleries/BorderStyles.xaml b/src/Controls/samples/Controls.Sample/Pages/Core/BorderGalleries/BorderStyles.xaml index b666a12be534..5ae5f46bcf99 100644 --- a/src/Controls/samples/Controls.Sample/Pages/Core/BorderGalleries/BorderStyles.xaml +++ b/src/Controls/samples/Controls.Sample/Pages/Core/BorderGalleries/BorderStyles.xaml @@ -6,7 +6,7 @@ Title="Border using Styles"> - + - +