From a66507c45a98225ddd2964ff0275fa41ecffbbcd Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Wed, 1 Mar 2023 17:13:37 -0600 Subject: [PATCH 1/3] [controls] fix memory leak in `VisualElement.Background` Fixes: https://github.com/dotnet/maui/issues/12344 Fixes: https://github.com/dotnet/maui/issues/13557 Context: https://github.com/dotnet-presentations/dotnet-maui-workshop While testing the `Monkey Finder` sample, I found the following scenario causes an issue: 1. Declare a `{StaticResource}` `Brush` at the `Application` level, with a lifetime of the entire application. 2. Set `Background` on an item in a `CollectionView`, `ListView`, etc. 3. Scroll a lot, navigate away, etc. 4. The `Brush` will hold onto any `View`'s indefinitely. The core problem here being `VisualElement` does: void NotifyBackgroundChanges() { if (Background is ImmutableBrush) return; if (Background != null) { Background.Parent = this; Background.PropertyChanged += OnBackgroundChanged; if (Background is GradientBrush gradientBrush) gradientBrush.InvalidateGradientBrushRequested += InvalidateGradientBrushRequested; } } If no user code sets `Background` to `null`, these events remain subscribed. To fix this: 1. Create a `WeakNotifyCollectionChangedProxy` type for event subscription. 2. Don't set `Background.Parent = this` ~~ General Cleanup ~~ Through doing other fixes related to memory leaks & C# events, we've started to gain a collection of `WeakEventProxy`-related types. I created some core `internal` types to be reused: * `abstract WeakEventProxy` * `WeakNotifyCollectionChangedProxy` The following classes now make use of these new shared types: * `BindingExpression` * `BindableLayout` * `ListProxy` * `VisualElement` This should hopefully reduce mistakes and reuse code in this area. ~~ Concerns ~~ Since, we are no longer doing: Background.Parent = this; This is certainly a behavior change. It is now replaced with: SetInheritedBindingContext(Background, BindingContext); I had to update one unit test that was asserting `Background.Parent`, which can assert `Background.BindingContext` instead. As such, this change is definitely sketchy and I wouldn't backport to .NET 7 immediately. We might test it out in a .NET 8 preview first. --- src/Controls/src/Core/BindableLayout.cs | 42 +---- src/Controls/src/Core/BindingExpression.cs | 58 +++---- .../Core/Internals/MaybeNullWhenAttribute.cs | 152 ++++++++++++++++++ .../src/Core/Internals/WeakEventProxy.cs | 105 ++++++++++++ src/Controls/src/Core/ListProxy.cs | 51 +----- .../net-android/PublicAPI.Unshipped.txt | 1 + .../PublicAPI/net-ios/PublicAPI.Unshipped.txt | 1 + .../net-maccatalyst/PublicAPI.Unshipped.txt | 1 + .../net-tizen/PublicAPI.Unshipped.txt | 1 + .../net-windows/PublicAPI.Unshipped.txt | 1 + .../PublicAPI/net/PublicAPI.Unshipped.txt | 1 + .../netstandard/PublicAPI.Unshipped.txt | 1 + src/Controls/src/Core/TypedBinding.cs | 8 +- src/Controls/src/Core/VisualElement.cs | 75 ++++++--- .../tests/Core.UnitTests/BrushUnitTests.cs | 3 +- .../Core.UnitTests/VisualElementTests.cs | 25 ++- 16 files changed, 378 insertions(+), 148 deletions(-) create mode 100644 src/Controls/src/Core/Internals/MaybeNullWhenAttribute.cs create mode 100644 src/Controls/src/Core/Internals/WeakEventProxy.cs diff --git a/src/Controls/src/Core/BindableLayout.cs b/src/Controls/src/Core/BindableLayout.cs index 91b74728c770..82ae3e339e3b 100644 --- a/src/Controls/src/Core/BindableLayout.cs +++ b/src/Controls/src/Core/BindableLayout.cs @@ -201,7 +201,7 @@ internal static void Clear(this IBindableLayout layout) class BindableLayoutController { readonly WeakReference _layoutWeakReference; - readonly WeakCollectionChangedProxy _collectionChangedProxy = new(); + readonly WeakNotifyCollectionChangedProxy _collectionChangedProxy = new(); IEnumerable _itemsSource; DataTemplate _itemTemplate; DataTemplateSelector _itemTemplateSelector; @@ -395,45 +395,5 @@ void ItemsSourceCollectionChanged(object sender, NotifyCollectionChangedEventArg if (e.Action != NotifyCollectionChangedAction.Reset) UpdateEmptyView(layout); } - - class WeakCollectionChangedProxy - { - WeakReference _handler; - WeakReference _source; - - void OnCollectionChanged(object sender, NotifyCollectionChangedEventArgs e) - { - if (_handler.TryGetTarget(out var handler)) - { - handler(sender, e); - } - else - { - Unsubscribe(); - } - } - - public void Subscribe(INotifyCollectionChanged source, NotifyCollectionChangedEventHandler handler) - { - if (_source is not null && _source.TryGetTarget(out var s)) - { - s.CollectionChanged -= OnCollectionChanged; - } - - _source = new WeakReference(source); - _handler = new WeakReference(handler); - source.CollectionChanged += OnCollectionChanged; - } - - public void Unsubscribe() - { - if (_source is not null && _source.TryGetTarget(out var s)) - { - s.CollectionChanged -= OnCollectionChanged; - } - _source = null; - _handler = null; - } - } } } \ No newline at end of file diff --git a/src/Controls/src/Core/BindingExpression.cs b/src/Controls/src/Core/BindingExpression.cs index ab1fb95294f8..4389d1ff920c 100644 --- a/src/Controls/src/Core/BindingExpression.cs +++ b/src/Controls/src/Core/BindingExpression.cs @@ -606,55 +606,39 @@ public BindingPair(BindingExpressionPart part, object source, bool isLast) public object Source { get; private set; } } - internal class WeakPropertyChangedProxy + internal class WeakPropertyChangedProxy : WeakEventProxy { - readonly WeakReference _source = new WeakReference(null); - readonly WeakReference _listener = new WeakReference(null); - readonly PropertyChangedEventHandler _handler; - readonly EventHandler _bchandler; - internal WeakReference Source => _source; + public WeakPropertyChangedProxy() { } - public WeakPropertyChangedProxy() + public WeakPropertyChangedProxy(INotifyPropertyChanged source, PropertyChangedEventHandler listener) { - _handler = new PropertyChangedEventHandler(OnPropertyChanged); - _bchandler = new EventHandler(OnBCChanged); + Subscribe(source, listener); } - public WeakPropertyChangedProxy(INotifyPropertyChanged source, PropertyChangedEventHandler listener) : this() - { - SubscribeTo(source, listener); - } - public void SubscribeTo(INotifyPropertyChanged source, PropertyChangedEventHandler listener) + + public override void Subscribe(INotifyPropertyChanged source, PropertyChangedEventHandler listener) { - source.PropertyChanged += _handler; - var bo = source as BindableObject; - if (bo != null) - bo.BindingContextChanged += _bchandler; - _source.SetTarget(source); - _listener.SetTarget(listener); + source.PropertyChanged += OnPropertyChanged; + if (source is BindableObject bo) + bo.BindingContextChanged += OnBCChanged; + + base.Subscribe(source, listener); } - public void Unsubscribe(bool finalizer = false) + public override void Unsubscribe() { - INotifyPropertyChanged source; - if (_source.TryGetTarget(out source) && source != null) - source.PropertyChanged -= _handler; - var bo = source as BindableObject; - if (bo != null) - bo.BindingContextChanged -= _bchandler; - - // If we are called from a finalizer, WeakReference.SetTarget() can throw InvalidOperationException - if (finalizer) - return; + if (TryGetSource(out var source)) + source.PropertyChanged -= OnPropertyChanged; + if (source is BindableObject bo) + bo.BindingContextChanged -= OnBCChanged; - _source.SetTarget(null); - _listener.SetTarget(null); + base.Unsubscribe(); } void OnPropertyChanged(object sender, PropertyChangedEventArgs e) { - if (_listener.TryGetTarget(out var handler) && handler != null) + if (TryGetHandler(out var handler)) handler(sender, e); else Unsubscribe(); @@ -662,7 +646,7 @@ void OnPropertyChanged(object sender, PropertyChangedEventArgs e) void OnBCChanged(object sender, EventArgs e) { - OnPropertyChanged(sender, new PropertyChangedEventArgs("BindingContext")); + OnPropertyChanged(sender, new PropertyChangedEventArgs(nameof(BindableObject.BindingContext))); } } @@ -672,7 +656,7 @@ class BindingExpressionPart readonly PropertyChangedEventHandler _changeHandler; WeakPropertyChangedProxy _listener; - ~BindingExpressionPart() => _listener?.Unsubscribe(finalizer: true); + ~BindingExpressionPart() => _listener?.Unsubscribe(); public BindingExpressionPart(BindingExpression expression, string content, bool isIndexer = false) { @@ -687,7 +671,7 @@ public BindingExpressionPart(BindingExpression expression, string content, bool public void Subscribe(INotifyPropertyChanged handler) { INotifyPropertyChanged source; - if (_listener != null && _listener.Source.TryGetTarget(out source) && ReferenceEquals(handler, source)) + if (_listener != null && _listener.TryGetSource(out source) && ReferenceEquals(handler, source)) // Already subscribed return; diff --git a/src/Controls/src/Core/Internals/MaybeNullWhenAttribute.cs b/src/Controls/src/Core/Internals/MaybeNullWhenAttribute.cs new file mode 100644 index 000000000000..46a825b8f75a --- /dev/null +++ b/src/Controls/src/Core/Internals/MaybeNullWhenAttribute.cs @@ -0,0 +1,152 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// This was copied from https://github.com/dotnet/runtime/blob/39b9607807f29e48cae4652cd74735182b31182e/src/libraries/System.Private.CoreLib/src/System/Diagnostics/CodeAnalysis/NullableAttributes.cs +// and updated to have the scope of the attributes be internal. + +#nullable disable + +namespace System.Diagnostics.CodeAnalysis +{ +#if !NETCOREAPP + + /// Specifies that null is allowed as an input even if the corresponding type disallows it. + [AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter | AttributeTargets.Property, Inherited = false)] + internal sealed class AllowNullAttribute : Attribute { } + + /// Specifies that null is disallowed as an input even if the corresponding type allows it. + [AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter | AttributeTargets.Property, Inherited = false)] + internal sealed class DisallowNullAttribute : Attribute { } + + /// Specifies that an output may be null even if the corresponding type disallows it. + [AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.ReturnValue, Inherited = false)] + internal sealed class MaybeNullAttribute : Attribute { } + + /// Specifies that an output will not be null even if the corresponding type allows it. + [AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.ReturnValue, Inherited = false)] + internal sealed class NotNullAttribute : Attribute { } + + /// Specifies that when a method returns , the parameter may be null even if the corresponding type disallows it. + [AttributeUsage(AttributeTargets.Parameter, Inherited = false)] + internal sealed class MaybeNullWhenAttribute : Attribute + { + /// Initializes the attribute with the specified return value condition. + /// + /// The return value condition. If the method returns this value, the associated parameter may be null. + /// + public MaybeNullWhenAttribute(bool returnValue) => ReturnValue = returnValue; + + /// Gets the return value condition. + public bool ReturnValue { get; } + } + + /// Specifies that when a method returns , the parameter will not be null even if the corresponding type allows it. + [AttributeUsage(AttributeTargets.Parameter, Inherited = false)] + internal sealed class NotNullWhenAttribute : Attribute + { + /// Initializes the attribute with the specified return value condition. + /// + /// The return value condition. If the method returns this value, the associated parameter will not be null. + /// + public NotNullWhenAttribute(bool returnValue) => ReturnValue = returnValue; + + /// Gets the return value condition. + public bool ReturnValue { get; } + } + + /// Specifies that the output will be non-null if the named parameter is non-null. + [AttributeUsage(AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.ReturnValue, AllowMultiple = true, Inherited = false)] + internal sealed class NotNullIfNotNullAttribute : Attribute + { + /// Initializes the attribute with the associated parameter name. + /// + /// The associated parameter name. The output will be non-null if the argument to the parameter specified is non-null. + /// + public NotNullIfNotNullAttribute(string parameterName) => ParameterName = parameterName; + + /// Gets the associated parameter name. + public string ParameterName { get; } + } + + /// Applied to a method that will never return under any circumstance. + [AttributeUsage(AttributeTargets.Method, Inherited = false)] + internal sealed class DoesNotReturnAttribute : Attribute { } + + /// Specifies that the method will not return if the associated Boolean parameter is passed the specified value. + [AttributeUsage(AttributeTargets.Parameter, Inherited = false)] + internal sealed class DoesNotReturnIfAttribute : Attribute + { + /// Initializes the attribute with the specified parameter value. + /// + /// The condition parameter value. Code after the method will be considered unreachable by diagnostics if the argument to + /// the associated parameter matches this value. + /// + public DoesNotReturnIfAttribute(bool parameterValue) => ParameterValue = parameterValue; + + /// Gets the condition parameter value. + public bool ParameterValue { get; } + } + +#endif + +#if !NETCOREAPP || NETCOREAPP3_1 + + /// Specifies that the method or property will ensure that the listed field and property members have not-null values. + [AttributeUsage(AttributeTargets.Method | AttributeTargets.Property, Inherited = false, AllowMultiple = true)] + internal sealed class MemberNotNullAttribute : Attribute + { + /// Initializes the attribute with a field or property member. + /// + /// The field or property member that is promised to be not-null. + /// + public MemberNotNullAttribute(string member) => Members = new[] { member }; + + /// Initializes the attribute with the list of field and property members. + /// + /// The list of field and property members that are promised to be not-null. + /// + public MemberNotNullAttribute(params string[] members) => Members = members; + + /// Gets field or property member names. + public string[] Members { get; } + } + + /// Specifies that the method or property will ensure that the listed field and property members have not-null values when returning with the specified return value condition. + [AttributeUsage(AttributeTargets.Method | AttributeTargets.Property, Inherited = false, AllowMultiple = true)] + internal sealed class MemberNotNullWhenAttribute : Attribute + { + /// Initializes the attribute with the specified return value condition and a field or property member. + /// + /// The return value condition. If the method returns this value, the associated parameter will not be null. + /// + /// + /// The field or property member that is promised to be not-null. + /// + public MemberNotNullWhenAttribute(bool returnValue, string member) + { + ReturnValue = returnValue; + Members = new[] { member }; + } + + /// Initializes the attribute with the specified return value condition and list of field and property members. + /// + /// The return value condition. If the method returns this value, the associated parameter will not be null. + /// + /// + /// The list of field and property members that are promised to be not-null. + /// + public MemberNotNullWhenAttribute(bool returnValue, params string[] members) + { + ReturnValue = returnValue; + Members = members; + } + + /// Gets the return value condition. + public bool ReturnValue { get; } + + /// Gets field or property member names. + public string[] Members { get; } + } + +#endif +} \ No newline at end of file diff --git a/src/Controls/src/Core/Internals/WeakEventProxy.cs b/src/Controls/src/Core/Internals/WeakEventProxy.cs new file mode 100644 index 000000000000..15e44d825a4d --- /dev/null +++ b/src/Controls/src/Core/Internals/WeakEventProxy.cs @@ -0,0 +1,105 @@ +using System; +using System.Collections.Specialized; + +// NOTE: warning disabled for netstandard projects +#pragma warning disable 0436 +using MaybeNullWhenAttribute = System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute; +#pragma warning restore 0436 + +namespace Microsoft.Maui.Controls +{ + /// + /// An abstract base class for subscribing to an event via WeakReference. + /// See WeakNotifyCollectionChangedProxy below for sublcass usage. + /// + /// The object type that has the event + /// The event handler type of the event + abstract class WeakEventProxy + where TSource : class + where TEventHandler : Delegate + { + WeakReference? _source; + WeakReference? _handler; + + public bool TryGetSource([MaybeNullWhen(false)] out TSource source) + { + if (_source is not null && _source.TryGetTarget(out source)) + { + return source is not null; + } + + source = default; + return false; + } + + public bool TryGetHandler([MaybeNullWhen(false)] out TEventHandler handler) + { + if (_handler is not null && _handler.TryGetTarget(out handler)) + { + return handler is not null; + } + + handler = default; + return false; + } + + public virtual void Subscribe(TSource source, TEventHandler handler) + { + _source = new WeakReference(source); + _handler = new WeakReference(handler); + } + + public virtual void Unsubscribe() + { + _source = null; + _handler = null; + } + } + + /// + /// A "proxy" class for subscribing INotifyCollectionChanged 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 WeakNotifyCollectionChangedProxy : WeakEventProxy + { + public WeakNotifyCollectionChangedProxy() { } + + public WeakNotifyCollectionChangedProxy(INotifyCollectionChanged source, NotifyCollectionChangedEventHandler handler) + { + Subscribe(source, handler); + } + + void OnCollectionChanged(object? sender, NotifyCollectionChangedEventArgs e) + { + if (TryGetHandler(out var handler)) + { + handler(sender, e); + } + else + { + Unsubscribe(); + } + } + + public override void Subscribe(INotifyCollectionChanged source, NotifyCollectionChangedEventHandler handler) + { + if (TryGetSource(out var s)) + { + s.CollectionChanged -= OnCollectionChanged; + } + + source.CollectionChanged += OnCollectionChanged; + base.Subscribe(source, handler); + } + + public override void Unsubscribe() + { + if (TryGetSource(out var s)) + { + s.CollectionChanged -= OnCollectionChanged; + } + base.Unsubscribe(); + } + } +} diff --git a/src/Controls/src/Core/ListProxy.cs b/src/Controls/src/Core/ListProxy.cs index 8c9acd41b685..c9f58dfd46b8 100644 --- a/src/Controls/src/Core/ListProxy.cs +++ b/src/Controls/src/Core/ListProxy.cs @@ -4,7 +4,6 @@ using System.Collections.Generic; using System.Collections.Specialized; using System.Linq; -using System.Runtime.CompilerServices; using Microsoft.Maui.Controls.Internals; using Microsoft.Maui.Dispatching; @@ -12,11 +11,11 @@ namespace Microsoft.Maui.Controls { internal sealed class ListProxy : IReadOnlyList, IListProxy, INotifyCollectionChanged { - IDispatcher _dispatcher; + readonly IDispatcher _dispatcher; readonly ICollection _collection; readonly IList _list; readonly int _windowSize; - readonly ConditionalWeakTable _sourceToWeakHandlers; + readonly WeakNotifyCollectionChangedProxy _proxy; IEnumerator _enumerator; int _enumeratorIndex; @@ -36,20 +35,20 @@ internal ListProxy(IEnumerable enumerable, int windowSize = int.MaxValue, IDispa ProxiedEnumerable = enumerable; _collection = enumerable as ICollection; - _sourceToWeakHandlers = new ConditionalWeakTable(); - if (_collection == null && enumerable is IReadOnlyCollection) - _collection = new ReadOnlyListAdapter((IReadOnlyCollection)enumerable); + if (_collection == null && enumerable is IReadOnlyCollection coll) + _collection = new ReadOnlyListAdapter(coll); _list = enumerable as IList; if (_list == null && enumerable is IReadOnlyList) _list = new ReadOnlyListAdapter((IReadOnlyList)enumerable); - var changed = enumerable as INotifyCollectionChanged; - if (changed != null) - _sourceToWeakHandlers.Add(this, new WeakNotifyProxy(this, changed)); + if (enumerable is INotifyCollectionChanged changed) + _proxy = new WeakNotifyCollectionChangedProxy(changed, OnCollectionChanged); } + ~ListProxy() => _proxy?.Unsubscribe(); + public IEnumerable ProxiedEnumerable { get; } IEnumerator IEnumerable.GetEnumerator() @@ -362,40 +361,6 @@ bool TryGetValue(int index, out object value) return _items.TryGetValue(index, out value); } - class WeakNotifyProxy - { - readonly WeakReference _weakCollection; - readonly WeakReference _weakProxy; - readonly ConditionalWeakTable _sourceToWeakHandlers; - - public WeakNotifyProxy(ListProxy proxy, INotifyCollectionChanged incc) - { - _sourceToWeakHandlers = new ConditionalWeakTable(); - NotifyCollectionChangedEventHandler handler = new NotifyCollectionChangedEventHandler(OnCollectionChanged); - - _sourceToWeakHandlers.Add(proxy, handler); - incc.CollectionChanged += handler; - - _weakProxy = new WeakReference(proxy); - _weakCollection = new WeakReference(incc); - } - - void OnCollectionChanged(object sender, NotifyCollectionChangedEventArgs e) - { - ListProxy proxy; - if (!_weakProxy.TryGetTarget(out proxy)) - { - INotifyCollectionChanged collection; - if (_weakCollection.TryGetTarget(out collection)) - collection.CollectionChanged -= OnCollectionChanged; - - return; - } - - proxy.OnCollectionChanged(sender, e); - } - } - class ProxyEnumerator : IEnumerator { readonly ListProxy _proxy; 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 b58af6e02992..36ff82c30d4a 100644 --- a/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt @@ -5,6 +5,7 @@ Microsoft.Maui.Controls.Handlers.Compatibility.FrameRenderer.FrameRenderer(Andro 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 +Microsoft.Maui.Controls.VisualElement.~VisualElement() -> void override Microsoft.Maui.Controls.LayoutOptions.GetHashCode() -> int override Microsoft.Maui.Controls.Region.GetHashCode() -> int override Microsoft.Maui.Controls.Shapes.Matrix.GetHashCode() -> int 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 ceefe7046f1b..49adee8be5ef 100644 --- a/src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt @@ -3,6 +3,7 @@ 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 +Microsoft.Maui.Controls.VisualElement.~VisualElement() -> void override Microsoft.Maui.Controls.LayoutOptions.GetHashCode() -> int override Microsoft.Maui.Controls.Platform.Compatibility.ShellPageRendererTracker.TitleViewContainer.LayoutSubviews() -> void override Microsoft.Maui.Controls.Region.GetHashCode() -> int 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 29fd1fc277ff..4f0f12ef2002 100644 --- a/src/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt @@ -3,6 +3,7 @@ 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 +Microsoft.Maui.Controls.VisualElement.~VisualElement() -> void override Microsoft.Maui.Controls.LayoutOptions.GetHashCode() -> int override Microsoft.Maui.Controls.Platform.Compatibility.ShellPageRendererTracker.TitleViewContainer.LayoutSubviews() -> void override Microsoft.Maui.Controls.Region.GetHashCode() -> int 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 78ad1703b864..a8e116cd8486 100644 --- a/src/Controls/src/Core/PublicAPI/net-tizen/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net-tizen/PublicAPI.Unshipped.txt @@ -2,6 +2,7 @@ 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 +Microsoft.Maui.Controls.VisualElement.~VisualElement() -> void override Microsoft.Maui.Controls.LayoutOptions.GetHashCode() -> int override Microsoft.Maui.Controls.Region.GetHashCode() -> int override Microsoft.Maui.Controls.Shapes.Matrix.GetHashCode() -> int 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 812f48fbd6a3..0cc1bd8a4665 100644 --- a/src/Controls/src/Core/PublicAPI/net-windows/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net-windows/PublicAPI.Unshipped.txt @@ -3,6 +3,7 @@ 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 +Microsoft.Maui.Controls.VisualElement.~VisualElement() -> void override Microsoft.Maui.Controls.LayoutOptions.GetHashCode() -> int override Microsoft.Maui.Controls.Region.GetHashCode() -> int override Microsoft.Maui.Controls.Shapes.Matrix.GetHashCode() -> int diff --git a/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt index ce1ce9c01052..49eeb11a69b7 100644 --- a/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt @@ -3,6 +3,7 @@ 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 +Microsoft.Maui.Controls.VisualElement.~VisualElement() -> void override Microsoft.Maui.Controls.LayoutOptions.GetHashCode() -> int override Microsoft.Maui.Controls.Region.GetHashCode() -> int override Microsoft.Maui.Controls.Shapes.Matrix.GetHashCode() -> int diff --git a/src/Controls/src/Core/PublicAPI/netstandard/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/netstandard/PublicAPI.Unshipped.txt index 3c4068b91838..328746133294 100644 --- a/src/Controls/src/Core/PublicAPI/netstandard/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/netstandard/PublicAPI.Unshipped.txt @@ -3,6 +3,7 @@ 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 +Microsoft.Maui.Controls.VisualElement.~VisualElement() -> void override Microsoft.Maui.Controls.LayoutOptions.GetHashCode() -> int override Microsoft.Maui.Controls.Region.GetHashCode() -> int override Microsoft.Maui.Controls.Shapes.Matrix.GetHashCode() -> int diff --git a/src/Controls/src/Core/TypedBinding.cs b/src/Controls/src/Core/TypedBinding.cs index 0384fd020980..48dd2c104594 100644 --- a/src/Controls/src/Core/TypedBinding.cs +++ b/src/Controls/src/Core/TypedBinding.cs @@ -252,13 +252,13 @@ class PropertyChangedProxy readonly BindingBase _binding; PropertyChangedEventHandler handler; - ~PropertyChangedProxy() => Listener?.Unsubscribe(finalizer: true); + ~PropertyChangedProxy() => Listener?.Unsubscribe(); public INotifyPropertyChanged Part { get { - if (Listener != null && Listener.Source.TryGetTarget(out var target)) + if (Listener != null && Listener.TryGetSource(out var target)) return target; return null; } @@ -267,12 +267,12 @@ public INotifyPropertyChanged Part if (Listener != null) { //Already subscribed - if (Listener.Source.TryGetTarget(out var source) && ReferenceEquals(value, source)) + if (Listener.TryGetSource(out var source) && ReferenceEquals(value, source)) return; //clear out previous subscription Listener.Unsubscribe(); - Listener.SubscribeTo(value, handler); + Listener.Subscribe(value, handler); } } } diff --git a/src/Controls/src/Core/VisualElement.cs b/src/Controls/src/Core/VisualElement.cs index c4f07b2ecee1..03ec278dbccc 100644 --- a/src/Controls/src/Core/VisualElement.cs +++ b/src/Controls/src/Core/VisualElement.cs @@ -1,6 +1,7 @@ #nullable disable using System; using System.Collections.Generic; +using System.Collections.Specialized; using System.ComponentModel; using System.Globalization; using System.Runtime.InteropServices.ComTypes; @@ -249,44 +250,78 @@ static void OnTransformChanged(BindableObject bindable, object oldValue, object (bindable as VisualElement)?.NotifyBackgroundChanges(); }); + readonly WeakBackgroundChangedProxy _backgroundProxy = new(); + + ~VisualElement() => _backgroundProxy.Unsubscribe(); + void NotifyBackgroundChanges() { - if (Background is ImmutableBrush) + var background = Background; + if (background is ImmutableBrush) return; - if (Background != null) + if (background != null) { - Background.Parent = this; - Background.PropertyChanged += OnBackgroundChanged; - - if (Background is GradientBrush gradientBrush) - gradientBrush.InvalidateGradientBrushRequested += InvalidateGradientBrushRequested; + SetInheritedBindingContext(background, BindingContext); + _backgroundProxy.Subscribe(background, (sender, e) => OnPropertyChanged(nameof(Background))); } } void StopNotifyingBackgroundChanges() { - if (Background is ImmutableBrush) + var background = Background; + if (background is ImmutableBrush) return; - if (Background != null) + if (background != null) { - Background.Parent = null; - Background.PropertyChanged -= OnBackgroundChanged; - - if (Background is GradientBrush gradientBrush) - gradientBrush.InvalidateGradientBrushRequested -= InvalidateGradientBrushRequested; + SetInheritedBindingContext(background, null); + _backgroundProxy.Unsubscribe(); } } - void OnBackgroundChanged(object sender, PropertyChangedEventArgs e) + class WeakBackgroundChangedProxy : WeakEventProxy { - OnPropertyChanged(nameof(Background)); - } + void OnBackgroundChanged(object sender, PropertyChangedEventArgs e) + { + if (TryGetHandler(out var handler)) + { + handler(sender, e); + } + else + { + Unsubscribe(); + } + } - void InvalidateGradientBrushRequested(object sender, EventArgs e) - { - OnPropertyChanged(nameof(Background)); + public override void Subscribe(Brush source, PropertyChangedEventHandler handler) + { + if (TryGetSource(out var s)) + { + s.PropertyChanged -= OnBackgroundChanged; + + if (s is GradientBrush g) + g.PropertyChanged -= OnBackgroundChanged; + } + + source.PropertyChanged += OnBackgroundChanged; + if (source is GradientBrush gradientBrush) + gradientBrush.PropertyChanged += OnBackgroundChanged; + + base.Subscribe(source, handler); + } + + public override void Unsubscribe() + { + if (TryGetSource(out var s)) + { + s.PropertyChanged -= OnBackgroundChanged; + + if (s is GradientBrush g) + g.PropertyChanged -= OnBackgroundChanged; + } + base.Unsubscribe(); + } } internal static readonly BindablePropertyKey BehaviorsPropertyKey = BindableProperty.CreateReadOnly("Behaviors", typeof(IList), typeof(VisualElement), default(IList), diff --git a/src/Controls/tests/Core.UnitTests/BrushUnitTests.cs b/src/Controls/tests/Core.UnitTests/BrushUnitTests.cs index 3b7b3b1db0c9..5bd01b109954 100644 --- a/src/Controls/tests/Core.UnitTests/BrushUnitTests.cs +++ b/src/Controls/tests/Core.UnitTests/BrushUnitTests.cs @@ -65,7 +65,7 @@ public void TestBindingContextPropagation() } [Fact] - public void TestBrushParent() + public void TestBrushBindingContext() { var context = new object(); @@ -84,7 +84,6 @@ public void TestBrushParent() parent.Background = linearGradientBrush; - Assert.Same(parent, parent.Background.Parent); Assert.Same(context, parent.Background.BindingContext); } diff --git a/src/Controls/tests/Core.UnitTests/VisualElementTests.cs b/src/Controls/tests/Core.UnitTests/VisualElementTests.cs index a9106e909324..76dab9721ccb 100644 --- a/src/Controls/tests/Core.UnitTests/VisualElementTests.cs +++ b/src/Controls/tests/Core.UnitTests/VisualElementTests.cs @@ -1,4 +1,7 @@ -using Microsoft.Maui.Primitives; +using System; +using System.Threading.Tasks; +using Microsoft.Maui.Graphics; +using Microsoft.Maui.Primitives; using Xunit; using static Microsoft.Maui.Controls.Core.UnitTests.VisualStateTestHelpers; @@ -95,5 +98,25 @@ public void ContainerChangedFiresWhenMapContainerIsCalled() handlerStub.UpdateValue(nameof(IViewHandler.ContainerView)); Assert.True(fired); } + + [Theory] + [InlineData(typeof(ImmutableBrush), false)] + [InlineData(typeof(SolidColorBrush), false)] + [InlineData(typeof(LinearGradientBrush), true)] + [InlineData(typeof(RadialGradientBrush), true)] + public async Task BackgroundDoesNotLeak(Type type, bool defaultCtor) + { + var brush = defaultCtor ? + (Brush)Activator.CreateInstance(type) : + (Brush)Activator.CreateInstance(type, Colors.CornflowerBlue); + + var reference = new WeakReference(new VisualElement { Background = brush }); + + await Task.Yield(); + GC.Collect(); + GC.WaitForPendingFinalizers(); + + Assert.False(reference.IsAlive, "VisualElement should not be alive!"); + } } } From d87eb348b3dcb545cec7660ee8a61acab24d7014 Mon Sep 17 00:00:00 2001 From: GitHub Actions Autoformatter Date: Thu, 2 Mar 2023 21:18:47 +0000 Subject: [PATCH 2/3] Auto-format source code --- .../Core/Internals/MaybeNullWhenAttribute.cs | 272 +++++++++--------- 1 file changed, 136 insertions(+), 136 deletions(-) diff --git a/src/Controls/src/Core/Internals/MaybeNullWhenAttribute.cs b/src/Controls/src/Core/Internals/MaybeNullWhenAttribute.cs index 46a825b8f75a..a78352348b56 100644 --- a/src/Controls/src/Core/Internals/MaybeNullWhenAttribute.cs +++ b/src/Controls/src/Core/Internals/MaybeNullWhenAttribute.cs @@ -9,144 +9,144 @@ namespace System.Diagnostics.CodeAnalysis { #if !NETCOREAPP - - /// Specifies that null is allowed as an input even if the corresponding type disallows it. - [AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter | AttributeTargets.Property, Inherited = false)] - internal sealed class AllowNullAttribute : Attribute { } - - /// Specifies that null is disallowed as an input even if the corresponding type allows it. - [AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter | AttributeTargets.Property, Inherited = false)] - internal sealed class DisallowNullAttribute : Attribute { } - - /// Specifies that an output may be null even if the corresponding type disallows it. - [AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.ReturnValue, Inherited = false)] - internal sealed class MaybeNullAttribute : Attribute { } - - /// Specifies that an output will not be null even if the corresponding type allows it. - [AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.ReturnValue, Inherited = false)] - internal sealed class NotNullAttribute : Attribute { } - - /// Specifies that when a method returns , the parameter may be null even if the corresponding type disallows it. - [AttributeUsage(AttributeTargets.Parameter, Inherited = false)] - internal sealed class MaybeNullWhenAttribute : Attribute - { - /// Initializes the attribute with the specified return value condition. - /// - /// The return value condition. If the method returns this value, the associated parameter may be null. - /// - public MaybeNullWhenAttribute(bool returnValue) => ReturnValue = returnValue; - - /// Gets the return value condition. - public bool ReturnValue { get; } - } - - /// Specifies that when a method returns , the parameter will not be null even if the corresponding type allows it. - [AttributeUsage(AttributeTargets.Parameter, Inherited = false)] - internal sealed class NotNullWhenAttribute : Attribute - { - /// Initializes the attribute with the specified return value condition. - /// - /// The return value condition. If the method returns this value, the associated parameter will not be null. - /// - public NotNullWhenAttribute(bool returnValue) => ReturnValue = returnValue; - - /// Gets the return value condition. - public bool ReturnValue { get; } - } - - /// Specifies that the output will be non-null if the named parameter is non-null. - [AttributeUsage(AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.ReturnValue, AllowMultiple = true, Inherited = false)] - internal sealed class NotNullIfNotNullAttribute : Attribute - { - /// Initializes the attribute with the associated parameter name. - /// - /// The associated parameter name. The output will be non-null if the argument to the parameter specified is non-null. - /// - public NotNullIfNotNullAttribute(string parameterName) => ParameterName = parameterName; - - /// Gets the associated parameter name. - public string ParameterName { get; } - } - - /// Applied to a method that will never return under any circumstance. - [AttributeUsage(AttributeTargets.Method, Inherited = false)] - internal sealed class DoesNotReturnAttribute : Attribute { } - - /// Specifies that the method will not return if the associated Boolean parameter is passed the specified value. - [AttributeUsage(AttributeTargets.Parameter, Inherited = false)] - internal sealed class DoesNotReturnIfAttribute : Attribute - { - /// Initializes the attribute with the specified parameter value. - /// - /// The condition parameter value. Code after the method will be considered unreachable by diagnostics if the argument to - /// the associated parameter matches this value. - /// - public DoesNotReturnIfAttribute(bool parameterValue) => ParameterValue = parameterValue; - - /// Gets the condition parameter value. - public bool ParameterValue { get; } - } - + + /// Specifies that null is allowed as an input even if the corresponding type disallows it. + [AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter | AttributeTargets.Property, Inherited = false)] + internal sealed class AllowNullAttribute : Attribute { } + + /// Specifies that null is disallowed as an input even if the corresponding type allows it. + [AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter | AttributeTargets.Property, Inherited = false)] + internal sealed class DisallowNullAttribute : Attribute { } + + /// Specifies that an output may be null even if the corresponding type disallows it. + [AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.ReturnValue, Inherited = false)] + internal sealed class MaybeNullAttribute : Attribute { } + + /// Specifies that an output will not be null even if the corresponding type allows it. + [AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.ReturnValue, Inherited = false)] + internal sealed class NotNullAttribute : Attribute { } + + /// Specifies that when a method returns , the parameter may be null even if the corresponding type disallows it. + [AttributeUsage(AttributeTargets.Parameter, Inherited = false)] + internal sealed class MaybeNullWhenAttribute : Attribute + { + /// Initializes the attribute with the specified return value condition. + /// + /// The return value condition. If the method returns this value, the associated parameter may be null. + /// + public MaybeNullWhenAttribute(bool returnValue) => ReturnValue = returnValue; + + /// Gets the return value condition. + public bool ReturnValue { get; } + } + + /// Specifies that when a method returns , the parameter will not be null even if the corresponding type allows it. + [AttributeUsage(AttributeTargets.Parameter, Inherited = false)] + internal sealed class NotNullWhenAttribute : Attribute + { + /// Initializes the attribute with the specified return value condition. + /// + /// The return value condition. If the method returns this value, the associated parameter will not be null. + /// + public NotNullWhenAttribute(bool returnValue) => ReturnValue = returnValue; + + /// Gets the return value condition. + public bool ReturnValue { get; } + } + + /// Specifies that the output will be non-null if the named parameter is non-null. + [AttributeUsage(AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.ReturnValue, AllowMultiple = true, Inherited = false)] + internal sealed class NotNullIfNotNullAttribute : Attribute + { + /// Initializes the attribute with the associated parameter name. + /// + /// The associated parameter name. The output will be non-null if the argument to the parameter specified is non-null. + /// + public NotNullIfNotNullAttribute(string parameterName) => ParameterName = parameterName; + + /// Gets the associated parameter name. + public string ParameterName { get; } + } + + /// Applied to a method that will never return under any circumstance. + [AttributeUsage(AttributeTargets.Method, Inherited = false)] + internal sealed class DoesNotReturnAttribute : Attribute { } + + /// Specifies that the method will not return if the associated Boolean parameter is passed the specified value. + [AttributeUsage(AttributeTargets.Parameter, Inherited = false)] + internal sealed class DoesNotReturnIfAttribute : Attribute + { + /// Initializes the attribute with the specified parameter value. + /// + /// The condition parameter value. Code after the method will be considered unreachable by diagnostics if the argument to + /// the associated parameter matches this value. + /// + public DoesNotReturnIfAttribute(bool parameterValue) => ParameterValue = parameterValue; + + /// Gets the condition parameter value. + public bool ParameterValue { get; } + } + #endif #if !NETCOREAPP || NETCOREAPP3_1 - - /// Specifies that the method or property will ensure that the listed field and property members have not-null values. - [AttributeUsage(AttributeTargets.Method | AttributeTargets.Property, Inherited = false, AllowMultiple = true)] - internal sealed class MemberNotNullAttribute : Attribute - { - /// Initializes the attribute with a field or property member. - /// - /// The field or property member that is promised to be not-null. - /// - public MemberNotNullAttribute(string member) => Members = new[] { member }; - - /// Initializes the attribute with the list of field and property members. - /// - /// The list of field and property members that are promised to be not-null. - /// - public MemberNotNullAttribute(params string[] members) => Members = members; - - /// Gets field or property member names. - public string[] Members { get; } - } - - /// Specifies that the method or property will ensure that the listed field and property members have not-null values when returning with the specified return value condition. - [AttributeUsage(AttributeTargets.Method | AttributeTargets.Property, Inherited = false, AllowMultiple = true)] - internal sealed class MemberNotNullWhenAttribute : Attribute - { - /// Initializes the attribute with the specified return value condition and a field or property member. - /// - /// The return value condition. If the method returns this value, the associated parameter will not be null. - /// - /// - /// The field or property member that is promised to be not-null. - /// - public MemberNotNullWhenAttribute(bool returnValue, string member) - { - ReturnValue = returnValue; - Members = new[] { member }; - } - - /// Initializes the attribute with the specified return value condition and list of field and property members. - /// - /// The return value condition. If the method returns this value, the associated parameter will not be null. - /// - /// - /// The list of field and property members that are promised to be not-null. - /// - public MemberNotNullWhenAttribute(bool returnValue, params string[] members) - { - ReturnValue = returnValue; - Members = members; - } - - /// Gets the return value condition. - public bool ReturnValue { get; } - - /// Gets field or property member names. - public string[] Members { get; } - } - + + /// Specifies that the method or property will ensure that the listed field and property members have not-null values. + [AttributeUsage(AttributeTargets.Method | AttributeTargets.Property, Inherited = false, AllowMultiple = true)] + internal sealed class MemberNotNullAttribute : Attribute + { + /// Initializes the attribute with a field or property member. + /// + /// The field or property member that is promised to be not-null. + /// + public MemberNotNullAttribute(string member) => Members = new[] { member }; + + /// Initializes the attribute with the list of field and property members. + /// + /// The list of field and property members that are promised to be not-null. + /// + public MemberNotNullAttribute(params string[] members) => Members = members; + + /// Gets field or property member names. + public string[] Members { get; } + } + + /// Specifies that the method or property will ensure that the listed field and property members have not-null values when returning with the specified return value condition. + [AttributeUsage(AttributeTargets.Method | AttributeTargets.Property, Inherited = false, AllowMultiple = true)] + internal sealed class MemberNotNullWhenAttribute : Attribute + { + /// Initializes the attribute with the specified return value condition and a field or property member. + /// + /// The return value condition. If the method returns this value, the associated parameter will not be null. + /// + /// + /// The field or property member that is promised to be not-null. + /// + public MemberNotNullWhenAttribute(bool returnValue, string member) + { + ReturnValue = returnValue; + Members = new[] { member }; + } + + /// Initializes the attribute with the specified return value condition and list of field and property members. + /// + /// The return value condition. If the method returns this value, the associated parameter will not be null. + /// + /// + /// The list of field and property members that are promised to be not-null. + /// + public MemberNotNullWhenAttribute(bool returnValue, params string[] members) + { + ReturnValue = returnValue; + Members = members; + } + + /// Gets the return value condition. + public bool ReturnValue { get; } + + /// Gets field or property member names. + public string[] Members { get; } + } + #endif } \ No newline at end of file From 9bc7d0fa3ff75cfc434d39e51ac5d002771a37a7 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Mon, 6 Mar 2023 16:01:40 -0600 Subject: [PATCH 3/3] Add test & subscribe to InvalidateGradientBrushRequested --- src/Controls/src/Core/VisualElement.cs | 13 +++++----- .../Core.UnitTests/VisualElementTests.cs | 24 +++++++++++++++++++ 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/src/Controls/src/Core/VisualElement.cs b/src/Controls/src/Core/VisualElement.cs index 03ec278dbccc..4fde67448b4e 100644 --- a/src/Controls/src/Core/VisualElement.cs +++ b/src/Controls/src/Core/VisualElement.cs @@ -1,7 +1,6 @@ #nullable disable using System; using System.Collections.Generic; -using System.Collections.Specialized; using System.ComponentModel; using System.Globalization; using System.Runtime.InteropServices.ComTypes; @@ -280,9 +279,9 @@ void StopNotifyingBackgroundChanges() } } - class WeakBackgroundChangedProxy : WeakEventProxy + class WeakBackgroundChangedProxy : WeakEventProxy { - void OnBackgroundChanged(object sender, PropertyChangedEventArgs e) + void OnBackgroundChanged(object sender, EventArgs e) { if (TryGetHandler(out var handler)) { @@ -294,19 +293,19 @@ void OnBackgroundChanged(object sender, PropertyChangedEventArgs e) } } - public override void Subscribe(Brush source, PropertyChangedEventHandler handler) + public override void Subscribe(Brush source, EventHandler handler) { if (TryGetSource(out var s)) { s.PropertyChanged -= OnBackgroundChanged; if (s is GradientBrush g) - g.PropertyChanged -= OnBackgroundChanged; + g.InvalidateGradientBrushRequested -= OnBackgroundChanged; } source.PropertyChanged += OnBackgroundChanged; if (source is GradientBrush gradientBrush) - gradientBrush.PropertyChanged += OnBackgroundChanged; + gradientBrush.InvalidateGradientBrushRequested += OnBackgroundChanged; base.Subscribe(source, handler); } @@ -318,7 +317,7 @@ public override void Unsubscribe() s.PropertyChanged -= OnBackgroundChanged; if (s is GradientBrush g) - g.PropertyChanged -= OnBackgroundChanged; + g.InvalidateGradientBrushRequested -= OnBackgroundChanged; } base.Unsubscribe(); } diff --git a/src/Controls/tests/Core.UnitTests/VisualElementTests.cs b/src/Controls/tests/Core.UnitTests/VisualElementTests.cs index 76dab9721ccb..b9dcffad12cb 100644 --- a/src/Controls/tests/Core.UnitTests/VisualElementTests.cs +++ b/src/Controls/tests/Core.UnitTests/VisualElementTests.cs @@ -118,5 +118,29 @@ public async Task BackgroundDoesNotLeak(Type type, bool defaultCtor) Assert.False(reference.IsAlive, "VisualElement should not be alive!"); } + + [Fact] + public void GradientBrushSubscribed() + { + var gradient = new LinearGradientBrush + { + GradientStops = + { + new GradientStop(Colors.White, 0), + new GradientStop(Colors.CornflowerBlue, 1), + } + }; + var visual = new VisualElement { Background = gradient }; + + bool fired = false; + visual.PropertyChanged += (sender, e) => + { + if (e.PropertyName == nameof(VisualElement.Background)) + fired = true; + }; + + gradient.GradientStops.Add(new GradientStop(Colors.CornflowerBlue, 1)); + Assert.True(fired, "PropertyChanged did not fire!"); + } } }