Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure to account for a null AppWindow #17746

Merged
merged 4 commits into from Oct 3, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion 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
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.
@@ -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
}
}
}
@@ -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);
}
}
28 changes: 28 additions & 0 deletions 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("Success");
}
}
}
5 changes: 4 additions & 1 deletion src/Core/src/Handlers/Window/WindowHandler.Windows.cs
Expand Up @@ -160,7 +160,10 @@ internal static void MapTitleBarDragRectangles(IWindowHandler handler, IWindow w
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)
Expand Down
19 changes: 15 additions & 4 deletions src/Core/src/Platform/Windows/MauiWinUIWindow.cs
Expand Up @@ -38,7 +38,13 @@ public MauiWinUIWindow()
// 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();
}
Expand Down Expand Up @@ -202,9 +208,14 @@ private void SetTileBarButtonColors()
{
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);
}
}

Expand Down
14 changes: 11 additions & 3 deletions src/Core/src/Platform/Windows/NavigationRootManager.cs
Expand Up @@ -19,7 +19,11 @@ public NavigationRootManager(Window platformWindow)
_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)
Expand All @@ -31,8 +35,12 @@ internal void SetTitleBarVisibility(bool isVisible)
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(
Expand Down
2 changes: 0 additions & 2 deletions src/Core/src/Platform/Windows/WindowExtensions.cs
Expand Up @@ -235,8 +235,6 @@ public static float GetDisplayDensity(this UI.Xaml.Window platformWindow)
return 1.0f;
}

var id = platformWindow.AppWindow.Id;

return PlatformMethods.GetDpiForWindow(hwnd) / DeviceDisplay.BaseLogicalDpi;
}

Expand Down