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

Fix ContextMenu leak

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;
@@ -145,36 +160,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

@@ -96,7 +96,7 @@ namespace Avalonia.Controls.Platform
                 root.Deactivated -= WindowDeactivated;
             }
 
-            _inputManagerSubscription!.Dispose();
+            _inputManagerSubscription?.Dispose();
 
             Menu = null;
             _root = null;

src/Avalonia.Controls/WindowBase.cs

@@ -255,7 +255,7 @@ namespace Avalonia.Controls
 
             if (scope != null)
             {
-                FocusManager.Instance.SetFocusScope(scope);
+                FocusManager.Instance?.SetFocusScope(scope);
             }
 
             IsActive = true;

src/Avalonia.Input/FocusManager.cs

@@ -168,7 +168,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

@@ -1,9 +1,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.Media;
 using Avalonia.Platform;
@@ -419,9 +421,83 @@ namespace Avalonia.LeakTests
             }
         }
 
+        [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

@@ -21,6 +21,10 @@ namespace Avalonia.UnitTests
         {
             var win = Mock.Of<IWindowImpl>(x => x.Scaling == 1);
             var mock = Mock.Get(win);
+            mock.Setup(x => x.Show()).Callback(() =>
+            {
+                mock.Object.Activated?.Invoke();
+            });
             mock.Setup(x => x.CreatePopup()).Returns(() =>
             {
                 if (popupImpl != null)