From a7ff93450e1fa527cda6aa26d2896be1432cb2bc Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 27 May 2017 14:05:44 +0200 Subject: [PATCH 1/2] Fixed problems with Windows.OpenWindows. - Showing a window was causing it to be added to the collection twice because `Show()` called `IsVisible = true` which called `Show()` - The window wasn't getting removed when the `PlatformImpl` signalled it was closed --- src/Avalonia.Controls/Window.cs | 11 +++ .../WindowTests.cs | 67 +++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/src/Avalonia.Controls/Window.cs b/src/Avalonia.Controls/Window.cs index 32eaf499f0..39f9a8d272 100644 --- a/src/Avalonia.Controls/Window.cs +++ b/src/Avalonia.Controls/Window.cs @@ -238,6 +238,11 @@ namespace Avalonia.Controls /// public override void Show() { + if (IsVisible) + { + return; + } + s_windows.Add(this); EnsureInitialized(); @@ -272,6 +277,11 @@ namespace Avalonia.Controls /// public Task ShowDialog() { + if (IsVisible) + { + throw new InvalidOperationException("The window is already being shown."); + } + s_windows.Add(this); EnsureInitialized(); @@ -360,6 +370,7 @@ namespace Avalonia.Controls protected override void HandleClosed() { IsVisible = false; + s_windows.Remove(this); base.HandleClosed(); } diff --git a/tests/Avalonia.Controls.UnitTests/WindowTests.cs b/tests/Avalonia.Controls.UnitTests/WindowTests.cs index 96afecc966..d2b882310d 100644 --- a/tests/Avalonia.Controls.UnitTests/WindowTests.cs +++ b/tests/Avalonia.Controls.UnitTests/WindowTests.cs @@ -114,5 +114,72 @@ namespace Avalonia.Controls.UnitTests Assert.False(window.IsVisible); } } + + [Fact] + public void Show_Should_Add_Window_To_OpenWindows() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var window = new Window(); + + window.Show(); + + Assert.Equal(new[] { window }, Window.OpenWindows); + + window.Close(); + } + } + + [Fact] + public void Window_Should_Be_Added_To_OpenWindows_Only_Once() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var window = new Window(); + + window.Show(); + window.Show(); + window.IsVisible = true; + + Assert.Equal(new[] { window }, Window.OpenWindows); + + window.Close(); + } + } + + [Fact] + public void Close_Should_Remove_Window_From_OpenWindows() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var window = new Window(); + + window.Show(); + window.Close(); + + Assert.Empty(Window.OpenWindows); + } + } + + [Fact] + public void Impl_Closing_Should_Remove_Window_From_OpenWindows() + { + var windowImpl = new Mock(); + windowImpl.SetupProperty(x => x.Closed); + windowImpl.Setup(x => x.Scaling).Returns(1); + + var services = TestServices.StyledWindow.With( + windowingPlatform: new MockWindowingPlatform(() => windowImpl.Object)); + + using (UnitTestApplication.Start(services)) + { + var window = new Window(); + + window.Show(); + windowImpl.Object.Closed(); + + Assert.Empty(Window.OpenWindows); + } + } } } From 5a819c1c879c3d2669e4722f6a3e1120b56bfbb0 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 27 May 2017 16:50:05 +0200 Subject: [PATCH 2/2] Make Window.OpenWindows readonly And hack around the fact that it's static in unit tests. --- src/Avalonia.Controls/Window.cs | 4 ++-- tests/Avalonia.Controls.UnitTests/WindowTests.cs | 14 ++++++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/Avalonia.Controls/Window.cs b/src/Avalonia.Controls/Window.cs index 39f9a8d272..3802f2b6ea 100644 --- a/src/Avalonia.Controls/Window.cs +++ b/src/Avalonia.Controls/Window.cs @@ -47,12 +47,12 @@ namespace Avalonia.Controls /// public class Window : WindowBase, IStyleable, IFocusScope, ILayoutRoot, INameScope { - private static IList s_windows = new List(); + private static List s_windows = new List(); /// /// Retrieves an enumeration of all Windows in the currently running application. /// - public static IList OpenWindows => s_windows; + public static IReadOnlyList OpenWindows => s_windows; /// /// Defines the property. diff --git a/tests/Avalonia.Controls.UnitTests/WindowTests.cs b/tests/Avalonia.Controls.UnitTests/WindowTests.cs index d2b882310d..e0dd908bbb 100644 --- a/tests/Avalonia.Controls.UnitTests/WindowTests.cs +++ b/tests/Avalonia.Controls.UnitTests/WindowTests.cs @@ -4,6 +4,7 @@ // // ----------------------------------------------------------------------- +using System.Collections.Generic; using Avalonia.Platform; using Avalonia.UnitTests; using Moq; @@ -120,13 +121,12 @@ namespace Avalonia.Controls.UnitTests { using (UnitTestApplication.Start(TestServices.StyledWindow)) { + ClearOpenWindows(); var window = new Window(); window.Show(); Assert.Equal(new[] { window }, Window.OpenWindows); - - window.Close(); } } @@ -135,6 +135,7 @@ namespace Avalonia.Controls.UnitTests { using (UnitTestApplication.Start(TestServices.StyledWindow)) { + ClearOpenWindows(); var window = new Window(); window.Show(); @@ -152,6 +153,7 @@ namespace Avalonia.Controls.UnitTests { using (UnitTestApplication.Start(TestServices.StyledWindow)) { + ClearOpenWindows(); var window = new Window(); window.Show(); @@ -173,6 +175,7 @@ namespace Avalonia.Controls.UnitTests using (UnitTestApplication.Start(services)) { + ClearOpenWindows(); var window = new Window(); window.Show(); @@ -181,5 +184,12 @@ namespace Avalonia.Controls.UnitTests Assert.Empty(Window.OpenWindows); } } + + private void ClearOpenWindows() + { + // HACK: We really need a decent way to have "statics" that can be scoped to + // AvaloniaLocator scopes. + ((IList)Window.OpenWindows).Clear(); + } } }