From a0e880581ea1553a5a502114be0af069a3db3fde Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 1 Sep 2016 21:55:41 +0200 Subject: [PATCH 1/4] Use ContentControl in DropDown template. Instead of `ContentPresenter`. This fixes a memory leak whereby `DropDown` wasn't correctly parenting the `Rectangle` created for `SelectionBoxItem` when the selected item is a control. Fixes #706. --- src/Avalonia.Themes.Default/DropDown.xaml | 8 +-- .../DropDownTests.cs | 63 ++++++++++++++++--- 2 files changed, 60 insertions(+), 11 deletions(-) diff --git a/src/Avalonia.Themes.Default/DropDown.xaml b/src/Avalonia.Themes.Default/DropDown.xaml index c33e4af4f4..5a3d44360c 100644 --- a/src/Avalonia.Themes.Default/DropDown.xaml +++ b/src/Avalonia.Themes.Default/DropDown.xaml @@ -10,10 +10,10 @@ BorderBrush="{TemplateBinding BorderBrush}" BorderThickness="{TemplateBinding BorderThickness}"> - + { }; + + root.Child = null; + + Assert.False(((ILogical)target).IsAttachedToLogicalTree); + Assert.False(((ILogical)rectangle).IsAttachedToLogicalTree); + } + private FuncControlTemplate GetTemplate() { return new FuncControlTemplate(parent => @@ -26,8 +71,7 @@ namespace Avalonia.Controls.UnitTests { new ContentControl { - Name = "contentControl", - [~ContentPresenter.ContentProperty] = parent[~DropDown.SelectionBoxItemProperty], + [!ContentControl.ContentProperty] = parent[!DropDown.SelectionBoxItemProperty], }, new ToggleButton { @@ -35,7 +79,12 @@ namespace Avalonia.Controls.UnitTests }, new Popup { - Name = "popup", + Name = "PART_Popup", + Child = new ItemsPresenter + { + Name = "PART_ItemsPresenter", + [!ItemsPresenter.ItemsProperty] = parent[!DropDown.ItemsProperty], + } } } }; From b61adbfd6412762364590839131111a6cb1feb9f Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 1 Sep 2016 23:52:05 +0200 Subject: [PATCH 2/4] Detach PopupRoot from logical tree... When parent `Popup` is detached. Together with previous comit also fixes #706. --- src/Avalonia.Controls/Primitives/Popup.cs | 9 +++++++-- .../Primitives/PopupTests.cs | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Controls/Primitives/Popup.cs b/src/Avalonia.Controls/Primitives/Popup.cs index 81d01ff74f..ae62922cb0 100644 --- a/src/Avalonia.Controls/Primitives/Popup.cs +++ b/src/Avalonia.Controls/Primitives/Popup.cs @@ -268,8 +268,13 @@ namespace Avalonia.Controls.Primitives { base.OnDetachedFromLogicalTree(e); _topLevel = null; - _popupRoot?.Dispose(); - _popupRoot = null; + + if (_popupRoot != null) + { + ((ISetLogicalParent)_popupRoot).SetParent(null); + _popupRoot.Dispose(); + _popupRoot = null; + } } /// diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs index 5581087b5f..13d97920ee 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs @@ -166,6 +166,24 @@ namespace Avalonia.Controls.UnitTests.Primitives } } + [Fact] + public void PopupRoot_Should_Be_Detached_From_Logical_Tree_When_Popup_Is_Detached() + { + using (CreateServices()) + { + var target = new Popup(); + var root = new TestRoot { Child = target }; + + target.Open(); + + var popupRoot = (ILogical)target.PopupRoot; + + Assert.True(popupRoot.IsAttachedToLogicalTree); + root.Child = null; + Assert.False(((ILogical)target).IsAttachedToLogicalTree); + } + } + [Fact] public void PopupRoot_Should_Have_Template_Applied() { From 51c8542030b33f4374d2cb281a4455747b12a564 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Fri, 2 Sep 2016 20:52:12 +0100 Subject: [PATCH 3/4] prevented second subscription of ListenForNonClientClick --- src/Avalonia.Controls/Primitives/Popup.cs | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/Avalonia.Controls/Primitives/Popup.cs b/src/Avalonia.Controls/Primitives/Popup.cs index ae62922cb0..c548d107a1 100644 --- a/src/Avalonia.Controls/Primitives/Popup.cs +++ b/src/Avalonia.Controls/Primitives/Popup.cs @@ -182,6 +182,8 @@ namespace Avalonia.Controls.Primitives /// IVisual IVisualTreeHost.Root => _popupRoot; + bool _ignoreIsOpenChanged = false; + /// /// Opens the popup. /// @@ -220,7 +222,11 @@ namespace Avalonia.Controls.Primitives PopupRootCreated?.Invoke(this, EventArgs.Empty); _popupRoot.Show(); + + _ignoreIsOpenChanged = true; IsOpen = true; + _ignoreIsOpenChanged = false; + Opened?.Invoke(this, EventArgs.Empty); } @@ -283,13 +289,16 @@ namespace Avalonia.Controls.Primitives /// The event args. private void IsOpenChanged(AvaloniaPropertyChangedEventArgs e) { - if ((bool)e.NewValue) + if (!_ignoreIsOpenChanged) { - Open(); - } - else - { - Close(); + if ((bool)e.NewValue) + { + Open(); + } + else + { + Close(); + } } } From cbd672e2f6365b38124ce3429b0ff48a445647c2 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 2 Sep 2016 23:34:26 +0200 Subject: [PATCH 4/4] Moved field to correct place. --- src/Avalonia.Controls/Primitives/Popup.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Avalonia.Controls/Primitives/Popup.cs b/src/Avalonia.Controls/Primitives/Popup.cs index c548d107a1..63f3fb647b 100644 --- a/src/Avalonia.Controls/Primitives/Popup.cs +++ b/src/Avalonia.Controls/Primitives/Popup.cs @@ -68,6 +68,7 @@ namespace Avalonia.Controls.Primitives private PopupRoot _popupRoot; private TopLevel _topLevel; private IDisposable _nonClientListener; + bool _ignoreIsOpenChanged = false; /// /// Initializes static members of the class. @@ -182,8 +183,6 @@ namespace Avalonia.Controls.Primitives /// IVisual IVisualTreeHost.Root => _popupRoot; - bool _ignoreIsOpenChanged = false; - /// /// Opens the popup. ///