Skip to content

Commit

Permalink
[FIX] Windows CTD on Application Close (#2067)
Browse files Browse the repository at this point in the history
* Add null checks for MediaElement in update methods

Improved robustness by adding null checks for `MediaElement` in
`PlatformUpdatePosition` and `PlatformUpdateShouldKeepScreenOn`
methods to prevent potential null reference exceptions.
Removed redundant null checks in `PlatformUpdateShouldKeepScreenOn`
to streamline the logic and enhance code readability.

* Update src/CommunityToolkit.Maui.MediaElement/Views/MediaManager.windows.cs

Co-authored-by: Shaun Lawrence <shaunrlawrence@gmail.com>

* Add ParentWindow struct and refactor null checks

Introduced a new `ParentWindow` record struct in the `MediaManager` class to handle null checks for the parent window and its handler. Added a static property `Exists` within `ParentWindow` to perform these checks and log errors if necessary. Updated the `PlatformUpdatePosition` method to return early if the parent window does not exist, preventing position updates when the window is closed. Simplified the `PlatformUpdateShouldKeepScreenOn` method by removing a redundant check for `MediaElement.ShouldKeepScreenOn`.

* Fix accidental change

* Add MediaManagerWindowsTests and ParentWindow struct

Introduce MediaManagerWindowsTests in MediaManager.windows.Tests.cs.
Add ParentWindow record struct with static properties CurrentPage
and Exists. Implement four Xunit tests to verify ParentWindow.Exists
behavior under various conditions.

* Refactor ParentWindow tests into separate file

Moved ParentWindow tests from MediaManager.windows.Tests.cs to a new file, ParentWindowTests.cs. Updated and reorganized test cases for ParentWindow.Exists, including scenarios for null parent window, null handler, and null platform view. Introduced MockWindowHandler class for simulating handler behavior. Adjusted using directives and namespace declarations to reflect the new file structure.

* Refactor ParentWindow struct into PageExtensions class

Refactored the `ParentWindow` record struct to consolidate its definition within the `PageExtensions` class. This change involved:

- Adding a new `ParentWindow` record struct in `PageExtensions.cs` with static properties `CurrentPage` and `Exists`.
- Removing the `ParentWindow` record struct from `MediaManager.windows.cs` and adding a `using` directive to import it from `CommunityToolkit.Maui.Extensions.PageExtensions`.
- Removing the `ParentWindow` record struct from `ParentWindowTests.cs` and adding a `using` directive to import it from `CommunityToolkit.Maui.Extensions.PageExtensions`.
- Ensuring the `ParentWindowTests` class retains its test method `Exists_WhenParentWindowIsNull_ReturnsFalse` to verify the `Exists` property behavior.

* Refactor tests and add MockWindowHandler for unit testing

Simplified GetCurrentPage call in PageExtensions.cs.
Changed ParentWindow struct visibility to internal.
Refactored ParentWindowTests.cs for better readability.
Added MockWindowHandler class for IElementHandler mocking.

---------

Co-authored-by: Shaun Lawrence <shaunrlawrence@gmail.com>
  • Loading branch information
ne0rrmatrix and bijington authored Aug 8, 2024
1 parent 43b09f5 commit 397d091
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,31 @@ internal static Page GetCurrentPage(this Page currentPage)

return currentPage;
}

internal record struct ParentWindow
{
static Page CurrentPage => GetCurrentPage(Application.Current?.MainPage ?? throw new InvalidOperationException($"{nameof(Application.Current.MainPage)} cannot be null."));
/// <summary>
/// Checks if the parent window is null.
/// </summary>
public static bool Exists
{
get
{
if (CurrentPage.GetParentWindow() is null)
{
return false;
}
if (CurrentPage.GetParentWindow().Handler is null)
{
return false;
}
if (CurrentPage.GetParentWindow().Handler.PlatformView is null)
{
return false;
}
return true;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
using Windows.Media.Playback;
using Windows.Storage;
using Windows.System.Display;
using Page = Microsoft.Maui.Controls.Page;
using WindowsMediaElement = Windows.Media.Playback.MediaPlayer;
using WinMediaSource = Windows.Media.Core.MediaSource;
using ParentWindow = CommunityToolkit.Maui.Extensions.PageExtensions.ParentWindow;

namespace CommunityToolkit.Maui.Core.Views;

Expand Down Expand Up @@ -181,6 +183,13 @@ protected virtual partial void PlatformUpdateShouldShowPlaybackControls()

protected virtual partial void PlatformUpdatePosition()
{
if (!ParentWindow.Exists)
{
// Parent window is null, so we can't update the position
// This is a workaround for a bug where the timer keeps running after the window is closed
return;
}

if (Player is not null
&& allowUpdatePositionStates.Contains(MediaElement.CurrentState))
{
Expand Down Expand Up @@ -217,8 +226,7 @@ protected virtual partial void PlatformUpdateShouldKeepScreenOn()
{
if (MediaElement.ShouldKeepScreenOn)
{
if (MediaElement != null
&& allowUpdatePositionStates.Contains(MediaElement.CurrentState)
if (allowUpdatePositionStates.Contains(MediaElement.CurrentState)
&& !displayActiveRequested)
{
DisplayRequest.RequestActive();
Expand Down
24 changes: 24 additions & 0 deletions src/CommunityToolkit.Maui.UnitTests/Mocks/MockWindowHandler.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
namespace CommunityToolkit.Maui.UnitTests.Mocks;

class MockWindowHandler : IElementHandler
{
public object? PlatformView { get; set; }
public IElement? VirtualView { get; set; }

public IMauiContext? MauiContext => new object() as IMauiContext;

public void DisconnectHandler() { }

public void Invoke(string command, object? args = null)
{
// No-op
}

public void SetMauiContext(IMauiContext mauiContext)
{
// No-op
}

public void SetVirtualView(IElement view) { }
public void UpdateValue(string property) { }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
using CommunityToolkit.Maui.UnitTests.Mocks;
using Xunit;
using ParentWindow = CommunityToolkit.Maui.Extensions.PageExtensions.ParentWindow;

namespace CommunityToolkit.Maui.UnitTests.Views;

public class ParentWindowTests
{
[Fact]
public void Exists_WhenParentWindowIsNull_ReturnsFalse()
{
Application.Current = new Application();
Application.Current.MainPage = new ContentPage();

Assert.False(ParentWindow.Exists);
}

[Fact]
public void Exists_WhenParentWindowHandlerIsNull_ReturnsFalse()
{
var mockWindow = new Window();
var mockPage = new ContentPage();
mockWindow.Page = mockPage;
Application.Current = new Application();
Application.Current.MainPage = mockPage;

Assert.False(ParentWindow.Exists);
}

[Fact]
public void Exists_WhenParentWindowHandlerPlatformViewIsNull_ReturnsFalse()
{
var mockWindow = new Window();
var mockPage = new ContentPage();
mockWindow.Page = mockPage;
Application.Current = new Application();
Application.Current.MainPage = mockPage;

// Simulate a scenario where the handler is set but the platform view is null
mockWindow.Handler = new MockWindowHandler();

Assert.False(ParentWindow.Exists);
}

[Fact]
public void Exists_WhenAllConditionsAreMet_ReturnsTrue()
{
var mockWindow = new Window();
var mockPage = new ContentPage();
mockWindow.Page = mockPage;
Application.Current = new Application();
Application.Current.MainPage = mockPage;

// Simulate a scenario where all conditions are met
mockWindow.Handler = new MockWindowHandler { PlatformView = new object() };

Assert.True(ParentWindow.Exists);
}
}

0 comments on commit 397d091

Please sign in to comment.