From 1a8cb9bba916fe3df8eb50b320b0a19b0a0fea90 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Thu, 28 Oct 2021 13:22:41 +0100 Subject: [PATCH 01/17] allow window to clear focus manager when it closes, preventing memory leak. --- src/Avalonia.Controls/WindowBase.cs | 6 ++++++ src/Avalonia.Input/ApiCompatBaseline.txt | 3 ++- src/Avalonia.Input/FocusManager.cs | 18 +++++++++++++++++- src/Avalonia.Input/IFocusManager.cs | 8 ++++++++ 4 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Controls/WindowBase.cs b/src/Avalonia.Controls/WindowBase.cs index ee008efb04..5861d0452d 100644 --- a/src/Avalonia.Controls/WindowBase.cs +++ b/src/Avalonia.Controls/WindowBase.cs @@ -193,6 +193,12 @@ namespace Avalonia.Controls try { IsVisible = false; + + if (this is IFocusScope scope) + { + FocusManager.Instance?.RemoveFocusScope(scope); + } + base.HandleClosed(); } finally diff --git a/src/Avalonia.Input/ApiCompatBaseline.txt b/src/Avalonia.Input/ApiCompatBaseline.txt index 98eb8598d8..221ef63476 100644 --- a/src/Avalonia.Input/ApiCompatBaseline.txt +++ b/src/Avalonia.Input/ApiCompatBaseline.txt @@ -3,6 +3,7 @@ MembersMustExist : Member 'public Avalonia.Platform.IPlatformHandle Avalonia.Inp MembersMustExist : Member 'public Avalonia.Interactivity.RoutedEvent Avalonia.Interactivity.RoutedEvent Avalonia.Input.Gestures.DoubleTappedEvent' does not exist in the implementation but it does exist in the contract. MembersMustExist : Member 'public Avalonia.Interactivity.RoutedEvent Avalonia.Interactivity.RoutedEvent Avalonia.Input.Gestures.RightTappedEvent' does not exist in the implementation but it does exist in the contract. MembersMustExist : Member 'public Avalonia.Interactivity.RoutedEvent Avalonia.Interactivity.RoutedEvent Avalonia.Input.Gestures.TappedEvent' does not exist in the implementation but it does exist in the contract. +InterfacesShouldHaveSameMembers : Interface member 'public void Avalonia.Input.IFocusManager.ClearFocusScope(Avalonia.Input.IFocusScope)' is present in the implementation but not in the contract. MembersMustExist : Member 'public Avalonia.Interactivity.RoutedEvent Avalonia.Interactivity.RoutedEvent Avalonia.Input.InputElement.DoubleTappedEvent' does not exist in the implementation but it does exist in the contract. MembersMustExist : Member 'public Avalonia.Interactivity.RoutedEvent Avalonia.Interactivity.RoutedEvent Avalonia.Input.InputElement.TappedEvent' does not exist in the implementation but it does exist in the contract. MembersMustExist : Member 'public void Avalonia.Input.InputElement.add_DoubleTapped(System.EventHandler)' does not exist in the implementation but it does exist in the contract. @@ -10,4 +11,4 @@ MembersMustExist : Member 'public void Avalonia.Input.InputElement.add_Tapped(Sy MembersMustExist : Member 'public void Avalonia.Input.InputElement.remove_DoubleTapped(System.EventHandler)' does not exist in the implementation but it does exist in the contract. MembersMustExist : Member 'public void Avalonia.Input.InputElement.remove_Tapped(System.EventHandler)' does not exist in the implementation but it does exist in the contract. TypesMustExist : Type 'Avalonia.Platform.IStandardCursorFactory' does not exist in the implementation but it does exist in the contract. -Total Issues: 11 +Total Issues: 12 diff --git a/src/Avalonia.Input/FocusManager.cs b/src/Avalonia.Input/FocusManager.cs index 1432092ba1..3e1ec84c03 100644 --- a/src/Avalonia.Input/FocusManager.cs +++ b/src/Avalonia.Input/FocusManager.cs @@ -145,7 +145,7 @@ namespace Avalonia.Input /// Notifies the focus manager of a change in focus scope. /// /// The new focus scope. - public void SetFocusScope(IFocusScope scope) + public void SetFocusScope(IFocusScope? scope) { scope = scope ?? throw new ArgumentNullException(nameof(scope)); @@ -162,6 +162,22 @@ namespace Avalonia.Input Focus(e); } + public void RemoveFocusScope(IFocusScope scope) + { + scope = scope ?? throw new ArgumentNullException(nameof(scope)); + + if (_focusScopes.TryGetValue(scope, out var e)) + { + SetFocusedElement(scope, null); + _focusScopes.Remove(scope); + } + + if (Scope == scope) + { + Scope = null; + } + } + public static bool GetIsFocusScope(IInputElement e) => e is IFocusScope; /// diff --git a/src/Avalonia.Input/IFocusManager.cs b/src/Avalonia.Input/IFocusManager.cs index e1b5087c3d..2510479a8e 100644 --- a/src/Avalonia.Input/IFocusManager.cs +++ b/src/Avalonia.Input/IFocusManager.cs @@ -35,5 +35,13 @@ namespace Avalonia.Input /// when it activates, e.g. when a Window is activated. /// void SetFocusScope(IFocusScope scope); + + /// + /// Notifies the focus manager that a focus scope has been removed. + /// + /// The focus scope to be removed. + /// This should not be called by client code. It is called by an + /// when it deactivates or closes, e.g. when a Window is closed. + void RemoveFocusScope(IFocusScope scope); } } From eb9f3343bf57346ecd5a599e95eb7ac7166f0aa4 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Thu, 28 Oct 2021 13:35:10 +0100 Subject: [PATCH 02/17] rename method in apicompat --- src/Avalonia.Input/ApiCompatBaseline.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Input/ApiCompatBaseline.txt b/src/Avalonia.Input/ApiCompatBaseline.txt index 221ef63476..270c5305e5 100644 --- a/src/Avalonia.Input/ApiCompatBaseline.txt +++ b/src/Avalonia.Input/ApiCompatBaseline.txt @@ -3,7 +3,7 @@ MembersMustExist : Member 'public Avalonia.Platform.IPlatformHandle Avalonia.Inp MembersMustExist : Member 'public Avalonia.Interactivity.RoutedEvent Avalonia.Interactivity.RoutedEvent Avalonia.Input.Gestures.DoubleTappedEvent' does not exist in the implementation but it does exist in the contract. MembersMustExist : Member 'public Avalonia.Interactivity.RoutedEvent Avalonia.Interactivity.RoutedEvent Avalonia.Input.Gestures.RightTappedEvent' does not exist in the implementation but it does exist in the contract. MembersMustExist : Member 'public Avalonia.Interactivity.RoutedEvent Avalonia.Interactivity.RoutedEvent Avalonia.Input.Gestures.TappedEvent' does not exist in the implementation but it does exist in the contract. -InterfacesShouldHaveSameMembers : Interface member 'public void Avalonia.Input.IFocusManager.ClearFocusScope(Avalonia.Input.IFocusScope)' is present in the implementation but not in the contract. +InterfacesShouldHaveSameMembers : Interface member 'public void Avalonia.Input.IFocusManager.RemoveFocusScope(Avalonia.Input.IFocusScope)' is present in the implementation but not in the contract. MembersMustExist : Member 'public Avalonia.Interactivity.RoutedEvent Avalonia.Interactivity.RoutedEvent Avalonia.Input.InputElement.DoubleTappedEvent' does not exist in the implementation but it does exist in the contract. MembersMustExist : Member 'public Avalonia.Interactivity.RoutedEvent Avalonia.Interactivity.RoutedEvent Avalonia.Input.InputElement.TappedEvent' does not exist in the implementation but it does exist in the contract. MembersMustExist : Member 'public void Avalonia.Input.InputElement.add_DoubleTapped(System.EventHandler)' does not exist in the implementation but it does exist in the contract. From 711ab49f0c3786e26d7dea4fedf714fbe4912e57 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Thu, 28 Oct 2021 13:38:08 +0100 Subject: [PATCH 03/17] discard var --- src/Avalonia.Input/FocusManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Input/FocusManager.cs b/src/Avalonia.Input/FocusManager.cs index 3e1ec84c03..f21edd7591 100644 --- a/src/Avalonia.Input/FocusManager.cs +++ b/src/Avalonia.Input/FocusManager.cs @@ -166,7 +166,7 @@ namespace Avalonia.Input { scope = scope ?? throw new ArgumentNullException(nameof(scope)); - if (_focusScopes.TryGetValue(scope, out var e)) + if (_focusScopes.TryGetValue(scope, out _)) { SetFocusedElement(scope, null); _focusScopes.Remove(scope); From 6f8f539a9fe9df86d753a4eba4158f1203ce8120 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Thu, 28 Oct 2021 14:20:27 +0100 Subject: [PATCH 04/17] prevent memory leak of mainwindow. --- .../ClassicDesktopStyleApplicationLifetime.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/ApplicationLifetimes/ClassicDesktopStyleApplicationLifetime.cs b/src/Avalonia.Controls/ApplicationLifetimes/ClassicDesktopStyleApplicationLifetime.cs index 2a42d99ac5..288939310c 100644 --- a/src/Avalonia.Controls/ApplicationLifetimes/ClassicDesktopStyleApplicationLifetime.cs +++ b/src/Avalonia.Controls/ApplicationLifetimes/ClassicDesktopStyleApplicationLifetime.cs @@ -122,12 +122,17 @@ namespace Avalonia.Controls.ApplicationLifetimes lifetimeEvents.ShutdownRequested += OnShutdownRequested; _cts = new CancellationTokenSource(); - MainWindow?.Show(); + ShowMainWindow(); Dispatcher.UIThread.MainLoop(_cts.Token); Environment.ExitCode = _exitCode; return _exitCode; } + private void ShowMainWindow() + { + MainWindow?.Show(); + } + public void Dispose() { if (_activeLifetime == this) From eb65760c51ece696539252ddd13bae137a0310f0 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Thu, 28 Oct 2021 15:07:56 +0100 Subject: [PATCH 05/17] add a comment explain JIT bug workaround. --- .../ClassicDesktopStyleApplicationLifetime.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/ApplicationLifetimes/ClassicDesktopStyleApplicationLifetime.cs b/src/Avalonia.Controls/ApplicationLifetimes/ClassicDesktopStyleApplicationLifetime.cs index 288939310c..a13813056b 100644 --- a/src/Avalonia.Controls/ApplicationLifetimes/ClassicDesktopStyleApplicationLifetime.cs +++ b/src/Avalonia.Controls/ApplicationLifetimes/ClassicDesktopStyleApplicationLifetime.cs @@ -122,7 +122,10 @@ namespace Avalonia.Controls.ApplicationLifetimes lifetimeEvents.ShutdownRequested += OnShutdownRequested; _cts = new CancellationTokenSource(); - ShowMainWindow(); + ShowMainWindow(); // Note due to a bug in the JIT we wrap this in a method, otherwise MainWindow + // gets stuffed into a local var and can not be GCed until after the program stops. + // this method never exits until program end. + Dispatcher.UIThread.MainLoop(_cts.Token); Environment.ExitCode = _exitCode; return _exitCode; From 3fadc669b58a2ed5f5a6f3b8377cd6ff55a963f1 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Thu, 28 Oct 2021 22:25:31 +0100 Subject: [PATCH 06/17] ensure static resource can implicitly convert to brush when used in setters. --- .../MarkupExtensions/StaticResourceExtension.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/StaticResourceExtension.cs b/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/StaticResourceExtension.cs index 4ad68fc63e..db33b88cc3 100644 --- a/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/StaticResourceExtension.cs +++ b/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/StaticResourceExtension.cs @@ -5,6 +5,7 @@ using Avalonia.Controls; using Avalonia.Markup.Data; using Avalonia.Markup.Xaml.Converters; using Avalonia.Markup.Xaml.XamlIl.Runtime; +using Avalonia.Styling; namespace Avalonia.Markup.Xaml.MarkupExtensions { @@ -33,6 +34,11 @@ namespace Avalonia.Markup.Xaml.MarkupExtensions _ => null, }; + if (provideTarget.TargetObject is Setter setter) + { + targetType = setter.Property.PropertyType; + } + // Look upwards though the ambient context for IResourceHosts and IResourceProviders // which might be able to give us the resource. foreach (var e in stack.Parents) From 51ab3266dd27000496e372a8c32bffc61ba5cd51 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Thu, 28 Oct 2021 22:47:31 +0100 Subject: [PATCH 07/17] add a unit test. --- .../StaticResourceExtensionTests.cs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/StaticResourceExtensionTests.cs b/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/StaticResourceExtensionTests.cs index fb3fd6d7d4..340eac0d4f 100644 --- a/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/StaticResourceExtensionTests.cs +++ b/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/StaticResourceExtensionTests.cs @@ -512,6 +512,33 @@ namespace Avalonia.Markup.Xaml.UnitTests.MarkupExtensions var brush = (ISolidColorBrush)border.Background; Assert.Equal(0xff506070, brush.Color.ToUint32()); } + + [Fact] + public void Automatically_Converts_Color_To_SolidColorBrush_From_Setter() + { + using (StyledWindow()) + { + var xaml = @" + + + #ff506070 + + + + + /// The new focus scope. - public void SetFocusScope(IFocusScope? scope) + public void SetFocusScope(IFocusScope scope) { scope = scope ?? throw new ArgumentNullException(nameof(scope));