From bc0ada009461679ef8517cc30ad1b2e5c0165576 Mon Sep 17 00:00:00 2001 From: Luis von der Eltz Date: Mon, 31 Aug 2020 18:35:40 +0200 Subject: [PATCH 01/11] Preferring user-provided string conversion over TypeDescriptor --- src/Avalonia.Controls/ColumnDefinitions.cs | 10 ++++- .../ViewModels/PropertyViewModel.cs | 37 ++++++++++++++----- 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/src/Avalonia.Controls/ColumnDefinitions.cs b/src/Avalonia.Controls/ColumnDefinitions.cs index ed4f9dbe99..7e355ab357 100644 --- a/src/Avalonia.Controls/ColumnDefinitions.cs +++ b/src/Avalonia.Controls/ColumnDefinitions.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Specialized; using System.Linq; +using System.Text; using Avalonia.Collections; namespace Avalonia.Controls @@ -13,7 +14,7 @@ namespace Avalonia.Controls /// /// Initializes a new instance of the class. /// - public ColumnDefinitions() : base () + public ColumnDefinitions() { } @@ -27,6 +28,11 @@ namespace Avalonia.Controls AddRange(GridLength.ParseLengths(s).Select(x => new ColumnDefinition(x))); } + public override string ToString() + { + return string.Join(",", this.Select(x => x.Width)); + } + /// /// Parses a string representation of column definitions collection. /// @@ -34,4 +40,4 @@ namespace Avalonia.Controls /// The . public static ColumnDefinitions Parse(string s) => new ColumnDefinitions(s); } -} \ No newline at end of file +} diff --git a/src/Avalonia.Diagnostics/Diagnostics/ViewModels/PropertyViewModel.cs b/src/Avalonia.Diagnostics/Diagnostics/ViewModels/PropertyViewModel.cs index ddbdae7ed9..2d69764ae8 100644 --- a/src/Avalonia.Diagnostics/Diagnostics/ViewModels/PropertyViewModel.cs +++ b/src/Avalonia.Diagnostics/Diagnostics/ViewModels/PropertyViewModel.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.ComponentModel; using System.Globalization; using System.Reflection; @@ -8,8 +9,8 @@ namespace Avalonia.Diagnostics.ViewModels internal abstract class PropertyViewModel : ViewModelBase { private const BindingFlags PublicStatic = BindingFlags.Public | BindingFlags.Static; - private static readonly Type[] StringParameter = new[] { typeof(string) }; - private static readonly Type[] StringIFormatProviderParameters = new[] { typeof(string), typeof(IFormatProvider) }; + private static readonly Type[] StringParameter = { typeof(string) }; + private static readonly Type[] StringIFormatProviderParameters = { typeof(string), typeof(IFormatProvider) }; public abstract object Key { get; } public abstract string Name { get; } @@ -25,19 +26,37 @@ namespace Avalonia.Diagnostics.ViewModels return "(null)"; } - var converter = TypeDescriptor.GetConverter(value); - return converter?.ConvertToString(value) ?? value.ToString(); + //Check if there's an user provided ToString(), prefer that over the TypeDescriptor conversion + if (value.GetType().GetMethod(nameof(ToString), System.Type.EmptyTypes) + .DeclaringType != typeof(object)) + { + return value.ToString(); + } + + try + { + var converter = TypeDescriptor.GetConverter(value); + + return converter.ConvertToString(value); + } + catch + { + return value.ToString(); + } } protected static object ConvertFromString(string s, Type targetType) { - var converter = TypeDescriptor.GetConverter(targetType); - - if (converter != null && converter.CanConvertFrom(typeof(string))) + try { - return converter.ConvertFrom(null, CultureInfo.InvariantCulture, s); + var converter = TypeDescriptor.GetConverter(targetType); + + if (converter.CanConvertFrom(typeof(string))) + { + return converter.ConvertFrom(null, CultureInfo.InvariantCulture, s); + } } - else + catch { var method = targetType.GetMethod("Parse", PublicStatic, null, StringIFormatProviderParameters, null); From 3cdfa305b59e2e9eb2784b4da470eb5008ca14c1 Mon Sep 17 00:00:00 2001 From: Luis von der Eltz Date: Tue, 1 Sep 2020 10:27:30 +0200 Subject: [PATCH 02/11] typo, removed using --- .../Diagnostics/ViewModels/PropertyViewModel.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Avalonia.Diagnostics/Diagnostics/ViewModels/PropertyViewModel.cs b/src/Avalonia.Diagnostics/Diagnostics/ViewModels/PropertyViewModel.cs index 2d69764ae8..bd60a46b95 100644 --- a/src/Avalonia.Diagnostics/Diagnostics/ViewModels/PropertyViewModel.cs +++ b/src/Avalonia.Diagnostics/Diagnostics/ViewModels/PropertyViewModel.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Generic; using System.ComponentModel; using System.Globalization; using System.Reflection; @@ -26,7 +25,7 @@ namespace Avalonia.Diagnostics.ViewModels return "(null)"; } - //Check if there's an user provided ToString(), prefer that over the TypeDescriptor conversion + //Check if there's an user-provided ToString(), prefer that over the TypeDescriptor conversion if (value.GetType().GetMethod(nameof(ToString), System.Type.EmptyTypes) .DeclaringType != typeof(object)) { From faad15571ac2beca8b78063c04391ab9b2a809d3 Mon Sep 17 00:00:00 2001 From: Luis von der Eltz Date: Tue, 1 Sep 2020 15:48:16 +0200 Subject: [PATCH 03/11] Filter out CollectionConverter Making sure to call Parse when conversion with TypeConverter is not possible --- .../ViewModels/PropertyViewModel.cs | 57 ++++++++----------- 1 file changed, 25 insertions(+), 32 deletions(-) diff --git a/src/Avalonia.Diagnostics/Diagnostics/ViewModels/PropertyViewModel.cs b/src/Avalonia.Diagnostics/Diagnostics/ViewModels/PropertyViewModel.cs index bd60a46b95..e23d6f1471 100644 --- a/src/Avalonia.Diagnostics/Diagnostics/ViewModels/PropertyViewModel.cs +++ b/src/Avalonia.Diagnostics/Diagnostics/ViewModels/PropertyViewModel.cs @@ -25,54 +25,47 @@ namespace Avalonia.Diagnostics.ViewModels return "(null)"; } - //Check if there's an user-provided ToString(), prefer that over the TypeDescriptor conversion - if (value.GetType().GetMethod(nameof(ToString), System.Type.EmptyTypes) - .DeclaringType != typeof(object)) + var converter = TypeDescriptor.GetConverter(value); + + //CollectionConverter does not deliver any important information. It just displays "(Collection)". + if (!converter.CanConvertTo(typeof(string)) || + converter.GetType() == typeof(CollectionConverter)) { return value.ToString(); } - try - { - var converter = TypeDescriptor.GetConverter(value); + return converter.ConvertToString(value); + } + + private static object InvokeParse(string s, Type targetType) + { + var method = targetType.GetMethod("Parse", PublicStatic, null, StringIFormatProviderParameters, null); - return converter.ConvertToString(value); + if (method != null) + { + return method.Invoke(null, new object[] { s, CultureInfo.InvariantCulture }); } - catch + + method = targetType.GetMethod("Parse", PublicStatic, null, StringParameter, null); + + if (method != null) { - return value.ToString(); + return method.Invoke(null, new object[] { s }); } + + throw new InvalidCastException("Unable to convert value."); } protected static object ConvertFromString(string s, Type targetType) { - try - { - var converter = TypeDescriptor.GetConverter(targetType); + var converter = TypeDescriptor.GetConverter(targetType); - if (converter.CanConvertFrom(typeof(string))) - { - return converter.ConvertFrom(null, CultureInfo.InvariantCulture, s); - } - } - catch + if (converter.CanConvertFrom(typeof(string))) { - var method = targetType.GetMethod("Parse", PublicStatic, null, StringIFormatProviderParameters, null); - - if (method != null) - { - return method.Invoke(null, new object[] { s, CultureInfo.InvariantCulture }); - } - - method = targetType.GetMethod("Parse", PublicStatic, null, StringParameter, null); - - if (method != null) - { - return method.Invoke(null, new object[] { s }); - } + return converter.ConvertFrom(null, CultureInfo.InvariantCulture, s); } - throw new InvalidCastException("Unable to convert value."); + return InvokeParse(s, targetType); } } } From 126737fd1bd5d182bdf803adfa7de949b77be0a1 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 4 Sep 2020 18:04:06 +0200 Subject: [PATCH 04/11] Update ScrollBar visibility with local value priority. Using `SetValue` with `Style` priority caused a new entry to be added to the `IsVisible` priority store each time the value was updated, causing a leak. Setting it with local value priority subtly changes its semantics but hopefully not so that anyone notices. --- src/Avalonia.Controls/Primitives/ScrollBar.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/Primitives/ScrollBar.cs b/src/Avalonia.Controls/Primitives/ScrollBar.cs index fc82fcc7a7..477d24dc99 100644 --- a/src/Avalonia.Controls/Primitives/ScrollBar.cs +++ b/src/Avalonia.Controls/Primitives/ScrollBar.cs @@ -141,7 +141,7 @@ namespace Avalonia.Controls.Primitives _ => throw new InvalidOperationException("Invalid value for ScrollBar.Visibility.") }; - SetValue(IsVisibleProperty, isVisible, BindingPriority.Style); + SetValue(IsVisibleProperty, isVisible); } protected override void OnKeyDown(KeyEventArgs e) From 3c62c43cc689a2e727d3e15f7137f82fd26bfef7 Mon Sep 17 00:00:00 2001 From: Kir-Antipov Date: Sun, 6 Sep 2020 04:37:37 +0300 Subject: [PATCH 05/11] Hid maximize box on Windows for `CanResize = false` --- src/Windows/Avalonia.Win32/WindowImpl.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Windows/Avalonia.Win32/WindowImpl.cs b/src/Windows/Avalonia.Win32/WindowImpl.cs index ddc0cc4e42..4929283874 100644 --- a/src/Windows/Avalonia.Win32/WindowImpl.cs +++ b/src/Windows/Avalonia.Win32/WindowImpl.cs @@ -1007,10 +1007,12 @@ namespace Avalonia.Win32 if (newProperties.IsResizable) { style |= WindowStyles.WS_SIZEFRAME; + style |= WindowStyles.WS_MAXIMIZEBOX; } else { style &= ~WindowStyles.WS_SIZEFRAME; + style &= ~WindowStyles.WS_MAXIMIZEBOX; } SetStyle(style); From 8079ebe1dc3d96e4f2a23897071b16e26a9e505a Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 8 Sep 2020 10:49:48 +0200 Subject: [PATCH 06/11] Added failing test for #4599. --- tests/Avalonia.LeakTests/ControlTests.cs | 36 ++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/Avalonia.LeakTests/ControlTests.cs b/tests/Avalonia.LeakTests/ControlTests.cs index 0b81276240..d61813b50b 100644 --- a/tests/Avalonia.LeakTests/ControlTests.cs +++ b/tests/Avalonia.LeakTests/ControlTests.cs @@ -552,6 +552,42 @@ namespace Avalonia.LeakTests } } + [Fact] + public void ItemsRepeater_Is_Freed() + { + using (Start()) + { + var geometry = new EllipseGeometry { Rect = new Rect(0, 0, 10, 10) }; + + Func run = () => + { + var window = new Window + { + Content = new ItemsRepeater(), + }; + + window.Show(); + + window.LayoutManager.ExecuteInitialLayoutPass(); + Assert.IsType(window.Presenter.Child); + + window.Content = null; + window.LayoutManager.ExecuteLayoutPass(); + Assert.Null(window.Presenter.Child); + + return window; + }; + + var result = run(); + + dotMemory.Check(memory => + Assert.Equal(0, memory.GetObjects(where => where.Type.Is()).ObjectsCount)); + + // We are keeping geometry alive to simulate a resource that outlives the control. + GC.KeepAlive(geometry); + } + } + private IDisposable Start() { return UnitTestApplication.Start(TestServices.StyledWindow.With( From 60c0a44a8737da790d68c73f9d8036b72396aad3 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 8 Sep 2020 10:51:52 +0200 Subject: [PATCH 07/11] Clean up EffectiveViewportChanged earlier. Clean up the `EffectiveViewportChanged` subscriptions before calling `OnDetachedFromVisualTree` so that (un)subscribing to the `EffectiveViewportChanged` event in `OnDetachedFromVisualTree` doesn't cause a leak. --- src/Avalonia.Layout/Layoutable.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Layout/Layoutable.cs b/src/Avalonia.Layout/Layoutable.cs index e62e22f8ec..aca2965ea6 100644 --- a/src/Avalonia.Layout/Layoutable.cs +++ b/src/Avalonia.Layout/Layoutable.cs @@ -758,8 +758,6 @@ namespace Avalonia.Layout protected override void OnDetachedFromVisualTreeCore(VisualTreeAttachmentEventArgs e) { - base.OnDetachedFromVisualTreeCore(e); - if (e.Root is ILayoutRoot r) { if (_layoutUpdated is object) @@ -772,6 +770,8 @@ namespace Avalonia.Layout r.LayoutManager.UnregisterEffectiveViewportListener(this); } } + + base.OnDetachedFromVisualTreeCore(e); } /// From 68504d212d068cf3786eb97199359be1222d4259 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 8 Sep 2020 13:20:22 +0200 Subject: [PATCH 08/11] Use weak event handlers. Seems that C++/WinRT uses weak event handlers by default so we need to do the same here. --- .../Repeater/ItemsRepeater.cs | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/src/Avalonia.Controls/Repeater/ItemsRepeater.cs b/src/Avalonia.Controls/Repeater/ItemsRepeater.cs index 8bc356bdec..772a17e4ea 100644 --- a/src/Avalonia.Controls/Repeater/ItemsRepeater.cs +++ b/src/Avalonia.Controls/Repeater/ItemsRepeater.cs @@ -7,10 +7,10 @@ using System; using System.Collections; using System.Collections.Specialized; using Avalonia.Controls.Templates; -using Avalonia.Data; using Avalonia.Input; using Avalonia.Layout; using Avalonia.Logging; +using Avalonia.Utilities; using Avalonia.VisualTree; namespace Avalonia.Controls @@ -681,8 +681,15 @@ namespace Avalonia.Controls if (oldValue != null) { oldValue.UninitializeForContext(LayoutContext); - oldValue.MeasureInvalidated -= InvalidateMeasureForLayout; - oldValue.ArrangeInvalidated -= InvalidateArrangeForLayout; + + WeakEventHandlerManager.Unsubscribe( + oldValue, + nameof(newValue.MeasureInvalidated), + InvalidateMeasureForLayout); + WeakEventHandlerManager.Unsubscribe( + oldValue, + nameof(newValue.ArrangeInvalidated), + InvalidateArrangeForLayout); // Walk through all the elements and make sure they are cleared foreach (var element in Children) @@ -699,8 +706,15 @@ namespace Avalonia.Controls if (newValue != null) { newValue.InitializeForContext(LayoutContext); - newValue.MeasureInvalidated += InvalidateMeasureForLayout; - newValue.ArrangeInvalidated += InvalidateArrangeForLayout; + + WeakEventHandlerManager.Subscribe( + newValue, + nameof(newValue.MeasureInvalidated), + InvalidateMeasureForLayout); + WeakEventHandlerManager.Subscribe( + newValue, + nameof(newValue.ArrangeInvalidated), + InvalidateArrangeForLayout); } bool isVirtualizingLayout = newValue != null && newValue is VirtualizingLayout; From e579e61f3820cd37e33258b3688cfc974477aa9c Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 8 Sep 2020 13:46:25 +0200 Subject: [PATCH 09/11] Teaks and remove copy-pasta. --- src/Avalonia.Controls/Repeater/ItemsRepeater.cs | 8 ++++---- tests/Avalonia.LeakTests/ControlTests.cs | 5 ----- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/Avalonia.Controls/Repeater/ItemsRepeater.cs b/src/Avalonia.Controls/Repeater/ItemsRepeater.cs index 772a17e4ea..40f1b8dbb9 100644 --- a/src/Avalonia.Controls/Repeater/ItemsRepeater.cs +++ b/src/Avalonia.Controls/Repeater/ItemsRepeater.cs @@ -684,11 +684,11 @@ namespace Avalonia.Controls WeakEventHandlerManager.Unsubscribe( oldValue, - nameof(newValue.MeasureInvalidated), + nameof(AttachedLayout.MeasureInvalidated), InvalidateMeasureForLayout); WeakEventHandlerManager.Unsubscribe( oldValue, - nameof(newValue.ArrangeInvalidated), + nameof(AttachedLayout.ArrangeInvalidated), InvalidateArrangeForLayout); // Walk through all the elements and make sure they are cleared @@ -709,11 +709,11 @@ namespace Avalonia.Controls WeakEventHandlerManager.Subscribe( newValue, - nameof(newValue.MeasureInvalidated), + nameof(AttachedLayout.MeasureInvalidated), InvalidateMeasureForLayout); WeakEventHandlerManager.Subscribe( newValue, - nameof(newValue.ArrangeInvalidated), + nameof(AttachedLayout.ArrangeInvalidated), InvalidateArrangeForLayout); } diff --git a/tests/Avalonia.LeakTests/ControlTests.cs b/tests/Avalonia.LeakTests/ControlTests.cs index d61813b50b..0c7b966f29 100644 --- a/tests/Avalonia.LeakTests/ControlTests.cs +++ b/tests/Avalonia.LeakTests/ControlTests.cs @@ -557,8 +557,6 @@ namespace Avalonia.LeakTests { using (Start()) { - var geometry = new EllipseGeometry { Rect = new Rect(0, 0, 10, 10) }; - Func run = () => { var window = new Window @@ -582,9 +580,6 @@ namespace Avalonia.LeakTests dotMemory.Check(memory => Assert.Equal(0, memory.GetObjects(where => where.Type.Is()).ObjectsCount)); - - // We are keeping geometry alive to simulate a resource that outlives the control. - GC.KeepAlive(geometry); } } From cf7b2966fe126845cc7019f5700c34eaf8652c85 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Tue, 8 Sep 2020 19:34:08 +0100 Subject: [PATCH 10/11] brew extract. --- azure-pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 29d2c62caf..92e4d9b7fb 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -61,7 +61,7 @@ jobs: inputs: script: | brew update - brew install https://raw.githubusercontent.com/Homebrew/homebrew-core/8a004a91a7fcd3f6620d5b01b6541ff0a640ffba/Formula/castxml.rb + brew extract castxml https://raw.githubusercontent.com/Homebrew/homebrew-core/8a004a91a7fcd3f6620d5b01b6541ff0a640ffba/Formula/castxml.rb - task: CmdLine@2 displayName: 'Install Nuke' From 71eaddfedbf4d5abd951966f076862a03d64e68c Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Tue, 8 Sep 2020 19:47:51 +0100 Subject: [PATCH 11/11] dont install castxml on osx. --- azure-pipelines.yml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 92e4d9b7fb..721a0415f4 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -56,13 +56,6 @@ jobs: xcodeVersion: '10' # Options: 8, 9, default, specifyPath args: '-derivedDataPath ./' - - task: CmdLine@2 - displayName: 'Install CastXML' - inputs: - script: | - brew update - brew extract castxml https://raw.githubusercontent.com/Homebrew/homebrew-core/8a004a91a7fcd3f6620d5b01b6541ff0a640ffba/Formula/castxml.rb - - task: CmdLine@2 displayName: 'Install Nuke' inputs: