diff --git a/src/Avalonia.Controls/ContextMenu.cs b/src/Avalonia.Controls/ContextMenu.cs index 5dfa5863f5..5c422dea13 100644 --- a/src/Avalonia.Controls/ContextMenu.cs +++ b/src/Avalonia.Controls/ContextMenu.cs @@ -20,6 +20,8 @@ namespace Avalonia.Controls private static readonly ITemplate DefaultPanel = new FuncTemplate(() => new StackPanel { Orientation = Orientation.Vertical }); private Popup _popup; + private Control _attachedControl; + private IInputElement _previousFocus; /// /// Initializes a new instance of the class. @@ -69,13 +71,16 @@ namespace Avalonia.Controls { var control = (Control)e.Sender; - if (e.OldValue != null) + if (e.OldValue is ContextMenu oldMenu) { control.PointerReleased -= ControlPointerReleased; + oldMenu._attachedControl = null; + ((ISetLogicalParent)oldMenu._popup).SetParent(null); } - if (e.NewValue != null) + if (e.NewValue is ContextMenu newMenu) { + newMenu._attachedControl = control; control.PointerReleased += ControlPointerReleased; } } @@ -91,8 +96,18 @@ namespace Avalonia.Controls /// The control. public void Open(Control control) { - if (control == null) + if (control is null && _attachedControl is null) + { throw new ArgumentNullException(nameof(control)); + } + + if (control is object && _attachedControl is object && control != _attachedControl) + { + throw new ArgumentException( + "Cannot show ContentMenu on a different control to the one it is attached to.", + nameof(control)); + } + if (IsOpen) { return; @@ -146,36 +161,38 @@ namespace Avalonia.Controls return new MenuItemContainerGenerator(this); } - private void CloseCore() - { - SelectedIndex = -1; - IsOpen = false; - - RaiseEvent(new RoutedEventArgs - { - RoutedEvent = MenuClosedEvent, - Source = this, - }); - } - private void PopupOpened(object sender, EventArgs e) { + _previousFocus = FocusManager.Instance?.Current; Focus(); } private void PopupClosed(object sender, EventArgs e) { - var contextMenu = (sender as Popup)?.Child as ContextMenu; - - if (contextMenu != null) + foreach (var i in LogicalChildren) { - foreach (var i in contextMenu.GetLogicalChildren().OfType()) + if (i is MenuItem menuItem) { - i.IsSubMenuOpen = false; + menuItem.IsSubMenuOpen = false; } + } - contextMenu.CloseCore(); + SelectedIndex = -1; + IsOpen = false; + + if (_attachedControl is null) + { + ((ISetLogicalParent)_popup).SetParent(null); } + + // HACK: Reset the focus when the popup is closed. We need to fix this so it's automatic. + FocusManager.Instance?.Focus(_previousFocus); + + RaiseEvent(new RoutedEventArgs + { + RoutedEvent = MenuClosedEvent, + Source = this, + }); } private static void ControlPointerReleased(object sender, PointerReleasedEventArgs e) diff --git a/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs b/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs index 8af71a6c1f..883ad85dc8 100644 --- a/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs +++ b/src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs @@ -94,7 +94,7 @@ namespace Avalonia.Controls.Platform root.Deactivated -= WindowDeactivated; } - _inputManagerSubscription.Dispose(); + _inputManagerSubscription?.Dispose(); Menu = null; _root = null; diff --git a/src/Avalonia.Controls/WindowBase.cs b/src/Avalonia.Controls/WindowBase.cs index 77ae382ae5..281e7ce98b 100644 --- a/src/Avalonia.Controls/WindowBase.cs +++ b/src/Avalonia.Controls/WindowBase.cs @@ -304,7 +304,7 @@ namespace Avalonia.Controls if (scope != null) { - FocusManager.Instance.SetFocusScope(scope); + FocusManager.Instance?.SetFocusScope(scope); } IsActive = true; diff --git a/src/Avalonia.Input/FocusManager.cs b/src/Avalonia.Input/FocusManager.cs index 97b2f7a397..8c9e677726 100644 --- a/src/Avalonia.Input/FocusManager.cs +++ b/src/Avalonia.Input/FocusManager.cs @@ -171,7 +171,7 @@ namespace Avalonia.Input { var scope = control as IFocusScope; - if (scope != null) + if (scope != null && control.VisualRoot?.IsVisible == true) { yield return scope; } diff --git a/tests/Avalonia.LeakTests/ControlTests.cs b/tests/Avalonia.LeakTests/ControlTests.cs index 1da4746516..34f551477a 100644 --- a/tests/Avalonia.LeakTests/ControlTests.cs +++ b/tests/Avalonia.LeakTests/ControlTests.cs @@ -4,9 +4,11 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Runtime.Remoting.Contexts; using Avalonia.Controls; using Avalonia.Controls.Templates; using Avalonia.Diagnostics; +using Avalonia.Input; using Avalonia.Layout; using Avalonia.Platform; using Avalonia.Rendering; @@ -370,9 +372,133 @@ namespace Avalonia.LeakTests } } + [Fact] + public void Control_With_Style_RenderTransform_Is_Freed() + { + // # Issue #3545 + using (Start()) + { + Func run = () => + { + var window = new Window + { + Styles = + { + new Style(x => x.OfType()) + { + Setters = + { + new Setter + { + Property = Visual.RenderTransformProperty, + Value = new RotateTransform(45), + } + } + } + }, + Content = new Canvas() + }; + + window.Show(); + + // Do a layout and make sure that Canvas gets added to visual tree with + // its render transform. + window.LayoutManager.ExecuteInitialLayoutPass(window); + var canvas = Assert.IsType(window.Presenter.Child); + Assert.IsType(canvas.RenderTransform); + + // Clear the content and ensure the Canvas is removed. + 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)); + } + } + + [Fact] + public void Attached_ContextMenu_Is_Freed() + { + using (Start()) + { + void AttachShowAndDetachContextMenu(Control control) + { + var contextMenu = new ContextMenu + { + Items = new[] + { + new MenuItem { Header = "Foo" }, + new MenuItem { Header = "Foo" }, + } + }; + + control.ContextMenu = contextMenu; + contextMenu.Open(control); + contextMenu.Close(); + control.ContextMenu = null; + } + + var window = new Window(); + window.Show(); + + Assert.Same(window, FocusManager.Instance.Current); + + AttachShowAndDetachContextMenu(window); + + dotMemory.Check(memory => + Assert.Equal(0, memory.GetObjects(where => where.Type.Is()).ObjectsCount)); + dotMemory.Check(memory => + Assert.Equal(0, memory.GetObjects(where => where.Type.Is()).ObjectsCount)); + } + } + + [Fact] + public void Standalone_ContextMenu_Is_Freed() + { + using (Start()) + { + void BuildAndShowContextMenu(Control control) + { + var contextMenu = new ContextMenu + { + Items = new[] + { + new MenuItem { Header = "Foo" }, + new MenuItem { Header = "Foo" }, + } + }; + + contextMenu.Open(control); + contextMenu.Close(); + } + + var window = new Window(); + window.Show(); + + Assert.Same(window, FocusManager.Instance.Current); + + BuildAndShowContextMenu(window); + BuildAndShowContextMenu(window); + + dotMemory.Check(memory => + Assert.Equal(0, memory.GetObjects(where => where.Type.Is()).ObjectsCount)); + dotMemory.Check(memory => + Assert.Equal(0, memory.GetObjects(where => where.Type.Is()).ObjectsCount)); + } + } + private IDisposable Start() { - return UnitTestApplication.Start(TestServices.StyledWindow); + return UnitTestApplication.Start(TestServices.StyledWindow.With( + focusManager: new FocusManager(), + keyboardDevice: () => new KeyboardDevice(), + inputManager: new InputManager())); } private class Node diff --git a/tests/Avalonia.UnitTests/MockWindowingPlatform.cs b/tests/Avalonia.UnitTests/MockWindowingPlatform.cs index b8b7512c9e..d40918591e 100644 --- a/tests/Avalonia.UnitTests/MockWindowingPlatform.cs +++ b/tests/Avalonia.UnitTests/MockWindowingPlatform.cs @@ -22,23 +22,13 @@ namespace Avalonia.UnitTests public static Mock CreateWindowMock() { - var windowImpl = new Mock(); - var position = new PixelPoint(); - var clientSize = new Size(800, 600); - - windowImpl.SetupAllProperties(); - windowImpl.Setup(x => x.ClientSize).Returns(() => clientSize); - windowImpl.Setup(x => x.Scaling).Returns(1); - windowImpl.Setup(x => x.Screen).Returns(CreateScreenMock().Object); - windowImpl.Setup(x => x.Position).Returns(() => position); - SetupToplevel(windowImpl); - - windowImpl.Setup(x => x.CreatePopup()).Returns(() => + var win = Mock.Of(x => x.Scaling == 1); + var mock = Mock.Get(win); + mock.Setup(x => x.Show()).Callback(() => { - return CreatePopupMock(windowImpl.Object).Object; + mock.Object.Activated?.Invoke(); }); - - windowImpl.Setup(x => x.Dispose()).Callback(() => + mock.Setup(x => x.CreatePopup()).Returns(() => { windowImpl.Object.Closed?.Invoke(); });