From 8badeece7899292eea0ea1d1be06cb9719c16590 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 25 Oct 2023 00:05:35 +0100 Subject: [PATCH] [release/8.0.1xx-rc2.2] Make sure to account for a null AppWindow (#18313) * Make sure to account for a null AppWindow * Update Issue17490.cs * - fix banned api * - fix automationid --------- Co-authored-by: Shane Neuville --- eng/BannedSymbols.txt | 3 +- .../Issues/Issue17490.cs | 73 +++++++++++++++++++ .../Platforms/Windows/PlatformMethods.cs | 12 +++ .../Elements/Window/WindowTests.Windows.cs | 2 +- .../tests/UITests/Tests/Issues/Issue17490.cs | 28 +++++++ .../Handlers/Window/WindowHandler.Windows.cs | 5 +- .../src/Platform/Windows/MauiWinUIWindow.cs | 19 ++++- .../Platform/Windows/NavigationRootManager.cs | 14 +++- .../src/Platform/Windows/WindowExtensions.cs | 2 - 9 files changed, 146 insertions(+), 12 deletions(-) create mode 100644 src/Controls/samples/Controls.Sample.UITests/Issues/Issue17490.cs create mode 100644 src/Controls/samples/Controls.Sample.UITests/Platforms/Windows/PlatformMethods.cs create mode 100644 src/Controls/tests/UITests/Tests/Issues/Issue17490.cs diff --git a/eng/BannedSymbols.txt b/eng/BannedSymbols.txt index a64f5dfd5..6ee1d42da 100644 --- a/eng/BannedSymbols.txt +++ b/eng/BannedSymbols.txt @@ -1,4 +1,5 @@ M:Microsoft.Extensions.DependencyInjection.Extensions.ServiceCollectionDescriptorExtensions.TryAddSingleton`2(Microsoft.Extensions.DependencyInjection.IServiceCollection);Use a Factory method to create the service instead M:Android.Content.Res.ColorStateList.#ctor(System.Int32[][],System.Int32[]);Use Microsoft.Maui.PlatformInterop.Get*ColorStateList() Java methods instead P:Microsoft.Maui.MauiWinUIApplication.Services;Use the IPlatformApplication.Current.Services instead -P:Microsoft.Maui.MauiWinUIApplication.Application;Use the IPlatformApplication.Current.Application instead \ No newline at end of file +P:Microsoft.Maui.MauiWinUIApplication.Application;Use the IPlatformApplication.Current.Application instead +P:Microsoft.UI.Xaml.Window.AppWindow;This API doesn't have null safety. Use GetAppWindow() and make sure to account for the possibility that GetAppWindow() might be null. \ No newline at end of file diff --git a/src/Controls/samples/Controls.Sample.UITests/Issues/Issue17490.cs b/src/Controls/samples/Controls.Sample.UITests/Issues/Issue17490.cs new file mode 100644 index 000000000..73e8e0eb8 --- /dev/null +++ b/src/Controls/samples/Controls.Sample.UITests/Issues/Issue17490.cs @@ -0,0 +1,73 @@ +using System; +using System.Threading.Tasks; +using Microsoft.Maui; +using Microsoft.Maui.Controls; +using Microsoft.Maui.Platform; + +namespace Maui.Controls.Sample.Issues +{ + [Issue(IssueTracker.Github, 17490, "Crash using Pinvoke.SetParent to create Window as Child", PlatformAffected.UWP)] + public class Issue17490 : TestContentPage + { + Label successLabel; + protected override void Init() + { + successLabel = new Label() { Text = "Success", AutomationId = "SuccessLabel" }; + + Content = new VerticalStackLayout() + { + new Label() + { + Text = "This test validates that opening a new WinUI Window parented to this window won't crash." + } + }; + } + + protected override void OnNavigatedTo(NavigatedToEventArgs args) + { + base.OnNavigatedTo(args); + + try + { + var myWindow = new MyWindow(new ContentPage()); + myWindow.Page.Loaded += async (_, _) => + { + await Task.Yield(); + Application.Current.CloseWindow(myWindow); + await Task.Yield(); + (this.Content as VerticalStackLayout) + .Add(successLabel); + }; + + Application.Current.OpenWindow(myWindow); + } + catch (Exception exc) + { + successLabel.Text = $"{exc}"; + } + } + + public class MyWindow : Window + { + public MyWindow(Page page) : base(page) + { + } + +#if WINDOWS + protected override void OnHandlerChanged() + { + base.OnHandlerChanged(); + if (Handler is null) + { + return; + } + + var mainWindowHandle = (Application.Current.MainPage.Window.Handler.PlatformView as MauiWinUIWindow).GetWindowHandle(); + var childWindowHandle = (Handler.PlatformView as MauiWinUIWindow).GetWindowHandle(); + + Platform.PlatformMethods.SetParent(childWindowHandle, mainWindowHandle); + } +#endif + } + } +} diff --git a/src/Controls/samples/Controls.Sample.UITests/Platforms/Windows/PlatformMethods.cs b/src/Controls/samples/Controls.Sample.UITests/Platforms/Windows/PlatformMethods.cs new file mode 100644 index 000000000..0efdf852e --- /dev/null +++ b/src/Controls/samples/Controls.Sample.UITests/Platforms/Windows/PlatformMethods.cs @@ -0,0 +1,12 @@ +#nullable enable +using System; +using System.Runtime.InteropServices; + +namespace Maui.Controls.Sample.Platform +{ + static class PlatformMethods + { + [DllImport("user32.dll")] + public static extern IntPtr SetParent(IntPtr hWndChild, IntPtr hWndNewParent); + } +} diff --git a/src/Controls/tests/DeviceTests/Elements/Window/WindowTests.Windows.cs b/src/Controls/tests/DeviceTests/Elements/Window/WindowTests.Windows.cs index 5cc5dc4a8..b26d63a7a 100644 --- a/src/Controls/tests/DeviceTests/Elements/Window/WindowTests.Windows.cs +++ b/src/Controls/tests/DeviceTests/Elements/Window/WindowTests.Windows.cs @@ -144,7 +144,7 @@ namespace Microsoft.Maui.DeviceTests await CreateHandlerAndAddToWindow(mainPage, async (handler) => { var mauiToolBar = GetPlatformToolbar(handler); - var presenter = handler.PlatformView.AppWindow.Presenter as OverlappedPresenter; + var presenter = handler.PlatformView.GetAppWindow()?.Presenter as OverlappedPresenter; var rootView = GetWindowRootView(handler); var defaultTitleBarHeight = rootView.AppTitleBarActualHeight; Assert.True(defaultTitleBarHeight > 0); diff --git a/src/Controls/tests/UITests/Tests/Issues/Issue17490.cs b/src/Controls/tests/UITests/Tests/Issues/Issue17490.cs new file mode 100644 index 000000000..036fe3592 --- /dev/null +++ b/src/Controls/tests/UITests/Tests/Issues/Issue17490.cs @@ -0,0 +1,28 @@ +using System.Drawing; +using Microsoft.Maui.Appium; +using NUnit.Framework; +using OpenQA.Selenium.Appium.MultiTouch; +using TestUtils.Appium.UITests; + +namespace Microsoft.Maui.AppiumTests.Issues +{ + public class Issue17490 : _IssuesUITest + { + public Issue17490(TestDevice device) : base(device) + { + } + + public override string Issue => "Crash using Pinvoke.SetParent to create Window as Child"; + + [Test] + public void AppDoesntCrashWhenOpeningWinUIWindowParentedToCurrentWindow() + { + UITestContext.IgnoreIfPlatforms(new[] + { + TestDevice.Mac, TestDevice.iOS, TestDevice.Android + }); + + App.WaitForElement("SuccessLabel"); + } + } +} diff --git a/src/Core/src/Handlers/Window/WindowHandler.Windows.cs b/src/Core/src/Handlers/Window/WindowHandler.Windows.cs index 4310b4958..a4e2641e9 100644 --- a/src/Core/src/Handlers/Window/WindowHandler.Windows.cs +++ b/src/Core/src/Handlers/Window/WindowHandler.Windows.cs @@ -160,7 +160,10 @@ namespace Microsoft.Maui.Handlers if (!AppWindowTitleBar.IsCustomizationSupported()) return; - var titleBar = handler.PlatformView.AppWindow.TitleBar; + var titleBar = handler.PlatformView.GetAppWindow()?.TitleBar; + if (titleBar is null) + return; + var titleBarRects = window.TitleBarDragRectangles; if (titleBarRects is null) diff --git a/src/Core/src/Platform/Windows/MauiWinUIWindow.cs b/src/Core/src/Platform/Windows/MauiWinUIWindow.cs index e572d5799..c3c2c6af7 100644 --- a/src/Core/src/Platform/Windows/MauiWinUIWindow.cs +++ b/src/Core/src/Platform/Windows/MauiWinUIWindow.cs @@ -38,7 +38,13 @@ namespace Microsoft.Maui // and then we can react accordingly if (AppWindowTitleBar.IsCustomizationSupported()) { - base.AppWindow.TitleBar.ExtendsContentIntoTitleBar = true; + var titleBar = this.GetAppWindow()?.TitleBar; + + if (titleBar is not null) + { + titleBar.ExtendsContentIntoTitleBar = true; + } + _viewSettings.ColorValuesChanged += _viewSettings_ColorValuesChanged; SetTileBarButtonColors(); } @@ -202,9 +208,14 @@ namespace Microsoft.Maui { if (AppWindowTitleBar.IsCustomizationSupported()) { - base.AppWindow.TitleBar.ButtonBackgroundColor = Colors.Transparent; - base.AppWindow.TitleBar.ButtonInactiveBackgroundColor = Colors.Transparent; - base.AppWindow.TitleBar.ButtonForegroundColor = _viewSettings.GetColorValue(ViewManagement.UIColorType.Foreground); + var titleBar = this.GetAppWindow()?.TitleBar; + + if (titleBar is null) + return; + + titleBar.ButtonBackgroundColor = Colors.Transparent; + titleBar.ButtonInactiveBackgroundColor = Colors.Transparent; + titleBar.ButtonForegroundColor = _viewSettings.GetColorValue(ViewManagement.UIColorType.Foreground); } } diff --git a/src/Core/src/Platform/Windows/NavigationRootManager.cs b/src/Core/src/Platform/Windows/NavigationRootManager.cs index aa900a823..7817562f3 100644 --- a/src/Core/src/Platform/Windows/NavigationRootManager.cs +++ b/src/Core/src/Platform/Windows/NavigationRootManager.cs @@ -19,7 +19,11 @@ namespace Microsoft.Maui.Platform _rootView.BackRequested += OnBackRequested; _rootView.OnApplyTemplateFinished += WindowRootViewOnApplyTemplateFinished; - SetTitleBarVisibility(_platformWindow.AppWindow.TitleBar.ExtendsContentIntoTitleBar); + var titleBar = _platformWindow.GetAppWindow()?.TitleBar; + if (titleBar is not null) + { + SetTitleBarVisibility(titleBar.ExtendsContentIntoTitleBar); + } } internal void SetTitleBarVisibility(bool isVisible) @@ -31,8 +35,12 @@ namespace Microsoft.Maui.Platform var appbarHeight = isVisible ? 32 : 0; if (isVisible && UI.Windowing.AppWindowTitleBar.IsCustomizationSupported()) { - var density = _platformWindow.GetDisplayDensity(); - appbarHeight = (int)(_platformWindow.AppWindow.TitleBar.Height / density); + var titleBar = _platformWindow.GetAppWindow()?.TitleBar; + if (titleBar is not null) + { + var density = _platformWindow.GetDisplayDensity(); + appbarHeight = (int)(titleBar.Height / density); + } } _rootView.UpdateAppTitleBar( diff --git a/src/Core/src/Platform/Windows/WindowExtensions.cs b/src/Core/src/Platform/Windows/WindowExtensions.cs index 8b0ba58f5..30f260d93 100644 --- a/src/Core/src/Platform/Windows/WindowExtensions.cs +++ b/src/Core/src/Platform/Windows/WindowExtensions.cs @@ -235,8 +235,6 @@ namespace Microsoft.Maui.Platform return 1.0f; } - var id = platformWindow.AppWindow.Id; - return PlatformMethods.GetDpiForWindow(hwnd) / DeviceDisplay.BaseLogicalDpi; }