Merge pull request #3745 from AvaloniaUI/fixes/3738-contextmenu-leak

Fix ContextMenu leak
# Conflicts:
#	src/Avalonia.Controls/Platform/DefaultMenuInteractionHandler.cs
#	tests/Avalonia.LeakTests/ControlTests.cs
#	tests/Avalonia.UnitTests/MockWindowingPlatform.cs

src/Avalonia.Controls/ContextMenu.cs

@@ -20,6 +20,8 @@ namespace Avalonia.Controls
         private static readonly ITemplate<IPanel> DefaultPanel =
             new FuncTemplate<IPanel>(() => new StackPanel { Orientation = Orientation.Vertical });
         private Popup _popup;
+        private Control _attachedControl;
+        private IInputElement _previousFocus;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="ContextMenu"/> 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
         /// <param name="control">The control.</param>
         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<MenuItem>())
+                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)

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;

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;

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;
                 }

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<Window> run = () =>
+                {
+                    var window = new Window
+                    {
+                        Styles =
+                        {
+                            new Style(x => x.OfType<Canvas>())
+                            {
+                                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<Canvas>(window.Presenter.Child);
+                    Assert.IsType<RotateTransform>(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<Canvas>()).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<ContextMenu>()).ObjectsCount));
+                dotMemory.Check(memory =>
+                    Assert.Equal(0, memory.GetObjects(where => where.Type.Is<MenuItem>()).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<ContextMenu>()).ObjectsCount));
+                dotMemory.Check(memory =>
+                    Assert.Equal(0, memory.GetObjects(where => where.Type.Is<MenuItem>()).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

tests/Avalonia.UnitTests/MockWindowingPlatform.cs

@@ -22,23 +22,13 @@ namespace Avalonia.UnitTests
 
         public static Mock<IWindowImpl> CreateWindowMock()
         {
-            var windowImpl = new Mock<IWindowImpl>();
-            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<IWindowImpl>(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();
             });