Merge pull request #5616 from AvaloniaUI/fixes/enumerate-child-windows-before-closing

Parent Windows get chance to cancel closing before any children close.

src/Avalonia.Controls/Window.cs

@@ -4,10 +4,7 @@ using System.ComponentModel;
 using System.Linq;
 using System.Reactive.Linq;
 using System.Threading.Tasks;
-using Avalonia.Controls.Chrome;
 using Avalonia.Controls.Platform;
-using Avalonia.Controls.Primitives;
-using Avalonia.Data;
 using Avalonia.Input;
 using Avalonia.Interactivity;
 using Avalonia.Layout;
@@ -482,10 +479,9 @@ namespace Avalonia.Controls
 
             try
             {
-                if (!ignoreCancel && HandleClosing())
+                if (!ignoreCancel && ShouldCancelClose())
                 {
                     close = false;
-                    return;
                 }
             }
             finally
@@ -497,11 +493,25 @@ namespace Avalonia.Controls
             }
         }
 
+        /// <summary>
+        /// Handles a closing notification from <see cref="IWindowImpl.Closing"/>.
+        /// <returns>true if closing is cancelled. Otherwise false.</returns>
+        /// </summary>
+        protected virtual bool HandleClosing()
+        {
+            if (!ShouldCancelClose())
+            {
+                CloseInternal();
+                return false;
+            }
+            
+            return true;
+        }
+
         private void CloseInternal()
         {
             foreach (var (child, _) in _children.ToList())
             {
-                // if we HandleClosing() before then there will be no children.
                 child.CloseInternal();
             }
 
@@ -515,20 +525,18 @@ namespace Avalonia.Controls
             PlatformImpl?.Dispose();
         }
 
-        /// <summary>
-        /// Handles a closing notification from <see cref="IWindowImpl.Closing"/>.
-        /// </summary>
-        protected virtual bool HandleClosing()
+        private bool ShouldCancelClose(CancelEventArgs args = null)
         {
+            if (args is null)
+            {
+                args = new CancelEventArgs();
+            }
+            
             bool canClose = true;
 
             foreach (var (child, _) in _children.ToList())
             {
-                if (!child.HandleClosing())
-                {
-                    child.CloseInternal();
-                }
-                else
+                if (child.ShouldCancelClose(args))
                 {
                     canClose = false;
                 }
@@ -536,15 +544,12 @@ namespace Avalonia.Controls
 
             if (canClose)
             {
-                var args = new CancelEventArgs();
                 OnClosing(args);
 
                 return args.Cancel;
             }
-            else
-            {
-                return !canClose;
-            }
+
+            return true;
         }
 
         protected virtual void HandleWindowStateChanged(WindowState state)

tests/Avalonia.Controls.UnitTests/WindowTests.cs

@@ -1,7 +1,6 @@
 using System;
 using System.Collections.Generic;
 using System.Threading.Tasks;
-using Avalonia.Layout;
 using Avalonia.Platform;
 using Avalonia.Rendering;
 using Avalonia.UnitTests;
@@ -137,6 +136,129 @@ namespace Avalonia.Controls.UnitTests
                 Assert.Equal(1, count);
             }
         }
+        
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void Child_windows_should_be_closed_before_parent(bool programaticClose)
+        {
+            using (UnitTestApplication.Start(TestServices.StyledWindow))
+            {
+                var window = new Window();
+                var child = new Window();
+
+                int count = 0;
+                int windowClosing = 0;
+                int childClosing = 0;
+                int windowClosed = 0;
+                int childClosed = 0;
+
+                window.Closing += (sender, e) =>
+                {
+                    count++;
+                    windowClosing = count;
+                };
+                
+                child.Closing += (sender, e) =>
+                {
+                    count++;
+                    childClosing = count;
+                };
+                
+                window.Closed += (sender, e) =>
+                {
+                    count++;
+                    windowClosed = count;
+                };
+                
+                child.Closed += (sender, e) =>
+                {
+                    count++;
+                    childClosed = count;
+                };
+
+                window.Show();
+                child.Show(window);
+
+                if (programaticClose)
+                {
+                    window.Close();
+                }
+                else
+                {
+                    var cancel = window.PlatformImpl.Closing();
+
+                    Assert.Equal(false, cancel);
+                }
+
+                Assert.Equal(2, windowClosing);
+                Assert.Equal(1, childClosing);
+                Assert.Equal(4, windowClosed);
+                Assert.Equal(3, childClosed);
+            }
+        }
+        
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void Child_windows_must_not_close_before_parent_has_chance_to_Cancel_OSCloseButton(bool programaticClose)
+        {
+            using (UnitTestApplication.Start(TestServices.StyledWindow))
+            {
+                var window = new Window();
+                var child = new Window();
+
+                int count = 0;
+                int windowClosing = 0;
+                int childClosing = 0;
+                int windowClosed = 0;
+                int childClosed = 0;
+
+                window.Closing += (sender, e) =>
+                {
+                    count++;
+                    windowClosing = count;
+                    e.Cancel = true;
+                };
+                
+                child.Closing += (sender, e) =>
+                {
+                    count++;
+                    childClosing = count;
+                };
+                
+                window.Closed += (sender, e) =>
+                {
+                    count++;
+                    windowClosed = count;
+                };
+                
+                child.Closed += (sender, e) =>
+                {
+                    count++;
+                    childClosed = count;
+                };
+
+                window.Show();
+                child.Show(window);
+                
+                if (programaticClose)
+                {
+                    window.Close();
+                }
+                else
+                {
+                    var cancel = window.PlatformImpl.Closing();
+
+                    Assert.Equal(true, cancel);
+                }
+
+                Assert.Equal(2, windowClosing);
+                Assert.Equal(1, childClosing);
+                Assert.Equal(0, windowClosed);
+                Assert.Equal(0, childClosed);
+            }
+        }
 
         [Fact]
         public void Showing_Should_Start_Renderer()