Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions src/Controls/tests/TestCases.HostApp/Issues/Issue27101.xaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?xml version="1.0" encoding="UTF-8"?>
<local:TestContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
xmlns:local="clr-namespace:Maui.Controls.Sample.Issues"
x:Class="Maui.Controls.Sample.Issues.Issue27101MainPage">
<local:TestContentPage.Resources>
<ResourceDictionary>
<Style x:Key="Issue27101ButtonStyle" TargetType="Button">
<Setter Property="TextColor" Value="White" />
<Setter Property="BackgroundColor" Value="Purple" />
<Setter Property="FontFamily" Value="OpenSansRegular"/>
<Setter Property="FontSize" Value="14"/>
<Setter Property="BorderWidth" Value="0"/>
<Setter Property="CornerRadius" Value="8"/>
<Setter Property="Padding" Value="14,10"/>
<Setter Property="MinimumHeightRequest" Value="44"/>
<Setter Property="MinimumWidthRequest" Value="44"/>
<Setter Property="VisualStateManager.VisualStateGroups">
<VisualStateGroupList>
<VisualStateGroup x:Name="CommonStates">
<VisualState x:Name="Normal" />
<VisualState Name="Focused" />
<VisualState x:Name="Disabled">
<VisualState.Setters>
<Setter Property="TextColor" Value="{AppThemeBinding Light=Black, Dark=LightGray}" />
<Setter Property="BackgroundColor" Value="{AppThemeBinding Light=LightGray, Dark=Gray}" />
</VisualState.Setters>
</VisualState>
<VisualState Name="PointerOver">
<VisualState.Setters>
<Setter Property="BackgroundColor" Value="Pink" />
</VisualState.Setters>
</VisualState>
<VisualState Name="Pressed">
<VisualState.Setters>
<Setter Property="BackgroundColor" Value="Red" />
</VisualState.Setters>
</VisualState>
</VisualStateGroup>
</VisualStateGroupList>
</Setter>
</Style>
</ResourceDictionary>
</local:TestContentPage.Resources>
<local:TestContentPage.Content>
<VerticalStackLayout
Padding="30,0"
Spacing="25">
<Button
x:Name="NavigateButton"
AutomationId="NavigateButton"
Text="Navigate to Second Page"
Clicked="OnNavigateButtonClicked"
HorizontalOptions="Fill"
Style="{StaticResource Issue27101ButtonStyle}"/>
</VerticalStackLayout>
</local:TestContentPage.Content>
</local:TestContentPage>
27 changes: 27 additions & 0 deletions src/Controls/tests/TestCases.HostApp/Issues/Issue27101.xaml.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
namespace Maui.Controls.Sample.Issues
{
[Issue(IssueTracker.Github, 27101, "PlatformView cannot be null here Exception in Windows")]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — added PlatformAffected.UWP to the [Issue] attribute in 5dfc963. (Used the positional form to match the existing pattern in Issue7156.xaml.cs.)

public class Issue27101 : NavigationPage
{
public Issue27101() : base(new Issue27101MainPage())
{
}
}

public partial class Issue27101MainPage : TestContentPage
{
public Issue27101MainPage()
{
InitializeComponent();
}

protected override void Init()
{
}

void OnNavigateButtonClicked(object sender, EventArgs e)
{
Navigation.PushAsync(new Issue27101SecondPage());
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?xml version="1.0" encoding="UTF-8"?>
<ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
xmlns:local="clr-namespace:Maui.Controls.Sample.Issues"
x:Class="Maui.Controls.Sample.Issues.Issue27101SecondPage">
<ContentPage.Resources>
<ResourceDictionary>
<Style x:Key="Issue27101ButtonStyle" TargetType="Button">
<Setter Property="TextColor" Value="White" />
<Setter Property="BackgroundColor" Value="Purple" />
<Setter Property="FontFamily" Value="OpenSansRegular"/>
<Setter Property="FontSize" Value="14"/>
<Setter Property="BorderWidth" Value="0"/>
<Setter Property="CornerRadius" Value="8"/>
<Setter Property="Padding" Value="14,10"/>
<Setter Property="MinimumHeightRequest" Value="44"/>
<Setter Property="MinimumWidthRequest" Value="44"/>
<Setter Property="VisualStateManager.VisualStateGroups">
<VisualStateGroupList>
<VisualStateGroup x:Name="CommonStates">
<VisualState x:Name="Normal" />
<VisualState Name="Focused" />
<VisualState x:Name="Disabled">
<VisualState.Setters>
<Setter Property="TextColor" Value="{AppThemeBinding Light=Black, Dark=LightGray}" />
<Setter Property="BackgroundColor" Value="{AppThemeBinding Light=LightGray, Dark=Gray}" />
</VisualState.Setters>
</VisualState>
<VisualState Name="PointerOver">
<VisualState.Setters>
<Setter Property="BackgroundColor" Value="Pink" />
</VisualState.Setters>
</VisualState>
<VisualState Name="Pressed">
<VisualState.Setters>
<Setter Property="BackgroundColor" Value="Red" />
</VisualState.Setters>
</VisualState>
</VisualStateGroup>
</VisualStateGroupList>
</Setter>
</Style>
</ResourceDictionary>
</ContentPage.Resources>
<ContentPage.Content>
<VerticalStackLayout
Padding="30,0"
Spacing="25">
<Button
x:Name="NavigateBackButton"
AutomationId="NavigateBackButton"
Text="Navigate Back"
Clicked="OnNavigateBackButtonClicked"
HorizontalOptions="Fill"
Style="{StaticResource Issue27101ButtonStyle}"/>
</VerticalStackLayout>
</ContentPage.Content>
</ContentPage>
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
namespace Maui.Controls.Sample.Issues
{
public partial class Issue27101SecondPage : ContentPage
{
public Issue27101SecondPage()
{
InitializeComponent();
}

async void OnNavigateBackButtonClicked(object sender, EventArgs e)
{
await Navigation.PopAsync();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
using NUnit.Framework;
using UITest.Appium;
using UITest.Core;

namespace Microsoft.Maui.TestCases.Tests.Issues;

public class Issue27101 : _IssuesUITest
{
public Issue27101(TestDevice device)
: base(device)
{ }

public override string Issue => "PlatformView cannot be null here Exception in Windows";

[Test]
[Category(UITestCategories.Button)]
public void NoCrashWhenNavigatingBackFromPageWithFocusedButton()
{
// The crash reproduces only when a Button with VisualStates currently holds focus
// at the moment the page is navigated away from. Repeat the navigation a few times
// to give the focus race a chance to surface.
for (int i = 0; i < 10; i++)
{
App.WaitForElement("NavigateButton");
App.Tap("NavigateButton");

App.WaitForElement("NavigateBackButton");
App.Tap("NavigateBackButton");
}

// If we got here without an unhandled InvalidOperationException, the regression is fixed.
App.WaitForElement("NavigateButton");
}
}
10 changes: 10 additions & 0 deletions src/Core/src/Core/Extensions/InternalElementExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,15 @@ internal static bool IsConnectingHandler(this IElement element) =>
/// <returns></returns>
internal static bool IsReconnectingHandler(this IElement element) =>
(element.Handler as IElementHandlerStateExhibitor)?.State.HasFlag(ElementHandlerState.Reconnecting) ?? false;

/// <summary>
/// Indicates whether the element's handler is currently being disconnected.
/// While disconnecting, the platform view has already been released so property mappers
/// must not be invoked.
/// </summary>
/// <param name="element"></param>
/// <returns></returns>
internal static bool IsDisconnectingHandler(this IElement element) =>
(element.Handler as IElementHandlerStateExhibitor)?.State.HasFlag(ElementHandlerState.Disconnecting) ?? false;
}
}
26 changes: 23 additions & 3 deletions src/Core/src/Handlers/Element/ElementHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,14 @@ public virtual void UpdateValue(string property)
if (VirtualView == null)
return;

// While the handler is being disconnected the platform view has already been released
// (see IElementHandler.DisconnectHandler below). Property changes that fan out from the
// virtual view during teardown (e.g. IsFocused -> ChangeVisualState -> VSM Setters)
// must not invoke mappers that touch the released platform view, otherwise strongly-typed
// PlatformView accessors throw "PlatformView cannot be null here".
if (this.IsDisconnectingHandler())
return;

_mapper?.UpdateProperty(this, VirtualView, property);
}

Expand Down Expand Up @@ -146,10 +154,22 @@ void IElementHandler.DisconnectHandler()
// DisconnectHandler
var oldPlatformView = PlatformView;
PlatformView = null;
DisconnectHandler(oldPlatformView);
try
{
// Mark the handler as disconnecting so property changes that fan out from the
// virtual view during platform teardown skip the mapper (see UpdateValue above).
_handlerState = ElementHandlerState.Disconnecting;
DisconnectHandler(oldPlatformView);
}
finally
{
_handlerState = ElementHandlerState.Disconnected;
}
}
else
{
_handlerState = ElementHandlerState.Disconnected;
}

_handlerState = ElementHandlerState.Disconnected;
}
}
}
11 changes: 10 additions & 1 deletion src/Core/src/Handlers/ElementHandlerState.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,15 @@ internal enum ElementHandlerState : byte
/// <summary>
/// The handler is connected to an element.
/// </summary>
Connected = 0x8
Connected = 0x8,
/// <summary>
/// The handler is in the process of disconnecting from its element.
/// While in this state the platform view has already been released
/// (and is no longer accessible via <see cref="IElementHandler.PlatformView"/>),
/// but the handler is still wired to the virtual view to allow platform
/// teardown to complete. Property mappers are short-circuited during this
/// window to avoid touching the released platform view.
/// </summary>
Disconnecting = 0x10
}
}
10 changes: 10 additions & 0 deletions src/Core/src/Handlers/InternalElementHandlerExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,15 @@ internal static bool IsConnectingHandler(this IElementHandler handler) =>
/// <returns></returns>
internal static bool IsReconnectingHandler(this IElementHandler handler) =>
(handler as IElementHandlerStateExhibitor)?.State.HasFlag(ElementHandlerState.Reconnecting) ?? false;

/// <summary>
/// Indicates whether the handler is currently being disconnected from its element.
/// While disconnecting, the platform view has already been released so property mappers
/// must not be invoked.
/// </summary>
/// <param name="handler"></param>
/// <returns></returns>
internal static bool IsDisconnectingHandler(this IElementHandler handler) =>
(handler as IElementHandlerStateExhibitor)?.State.HasFlag(ElementHandlerState.Disconnecting) ?? false;
}
}
33 changes: 33 additions & 0 deletions src/Core/tests/UnitTests/AbstractViewHandlerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,39 @@ public void ConnectAndDisconnectFireAppropriateNumberOfTimes()
Assert.Equal(2, handlerStub.DisconnectHandlerCount);
}

[Fact]
public void UpdateValueIsSkippedWhileHandlerIsDisconnecting()
{
// Regression test for https://github.com/dotnet/maui/issues/27101
// While the handler is being disconnected the platform view has already been released.
// Property changes that fan out from the virtual view during teardown (e.g. IsFocused
// -> ChangeVisualState -> VSM Setters) must not invoke mappers, otherwise the strongly
// typed PlatformView accessor throws "PlatformView cannot be null here".

DisconnectingTrackingHandlerStub handlerStub = null;
handlerStub = new DisconnectingTrackingHandlerStub(() =>
{
// Simulate a property change that fans into the mapper while the platform handler
// is still tearing down (this mirrors what UpdateIsFocused(false) triggers on Windows).
handlerStub.UpdateValue(nameof(IView.Background));
});

handlerStub.SetVirtualView(new Maui.Controls.Button());

// Sanity check: outside of disconnect, UpdateValue routes through the mapper.
handlerStub.MapBackgroundCallCount = 0;
handlerStub.UpdateValue(nameof(IView.Background));
Assert.Equal(1, handlerStub.MapBackgroundCallCount);

// Reset so we can detect any mapper invocations that happen during DisconnectHandler.
handlerStub.MapBackgroundCallCount = 0;

(handlerStub as IViewHandler).DisconnectHandler();

// The mapper-routed UpdateValue inside DisconnectHandler must have been a no-op.
Assert.Equal(0, handlerStub.MapBackgroundCallCount);
}

[Fact]
public void GetRequiredServiceThrowsOnNoContext()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
using System;
using Microsoft.Maui.Handlers;

namespace Microsoft.Maui.UnitTests
{
// Helper used by the regression test for https://github.com/dotnet/maui/issues/27101.
// During DisconnectHandler we trigger an UpdateValue call to mimic the cascade caused on
// Windows by UpdateIsFocused(false) -> ChangeVisualState -> VSM Setters -> mapper. We then
// assert the mapper was NOT invoked while the handler was tearing down.
public class DisconnectingTrackingHandlerStub : ViewHandler<Maui.Controls.Button, object>
{
readonly Action _onDisconnect;

public int MapBackgroundCallCount { get; set; }

public DisconnectingTrackingHandlerStub(Action onDisconnect)
: base(BuildMapper())
{
_onDisconnect = onDisconnect;
}

static IPropertyMapper<IView, IViewHandler> BuildMapper()
{
var mapper = new PropertyMapper<IView, IViewHandler>();
mapper.Add(nameof(IView.Background), (handler, view) =>
{
if (handler is DisconnectingTrackingHandlerStub stub)
{
stub.MapBackgroundCallCount++;
}
});
return mapper;
}

protected override object CreatePlatformView() => new object();

protected override void DisconnectHandler(object platformView)
{
base.DisconnectHandler(platformView);
_onDisconnect?.Invoke();
}
}
}
Loading