[FIX] Windows CTD on Application Close #2067
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
bijington
previously approved these changes
Aug 7, 2024
src/CommunityToolkit.Maui.MediaElement/Views/MediaManager.windows.cs
Outdated
Show resolved
Hide resolved
…ows.cs Co-authored-by: Shaun Lawrence <shaunrlawrence@gmail.com>
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`.
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.
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.
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.
Simplified GetCurrentPage call in PageExtensions.cs. Changed ParentWindow struct visibility to internal. Refactored ParentWindowTests.cs for better readability. Added MockWindowHandler class for IElementHandler mocking.
pictos
approved these changes
Aug 8, 2024
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Change
Add null checks for MediaElement in update methods. This should resolve the issue with CTD in Windows when closing app.
Linked Issues
PR Checklist
approved(bug) orChampioned(feature/proposal)mainat time of PRAdditional information
2 1/2 min video in Windows where MediaElement is playing video and then app is closed randomly to see if it crashes. It does not. The startup issue display at some points in video is unrelated and has been present since Maui went GA. It is not addressed and not related to toolkit.
2024-07-28.21-42-27.mp4