Skip to content

Commit

Permalink
Fix crash when HTMLMediaElement::exitFullscreen is called on a video
Browse files Browse the repository at this point in the history
element which is not currently full screen
https://bugs.webkit.org/show_bug.cgi?id=255970
rdar://108489504

Reviewed by Jer Noble.

This change fixes an issue where exitFullScreen is called on video, but
the current full screen element is div, due to which we end up
scheduling the webkitendfullscreenEvent event for video, which trips
over an assertion.

* LayoutTests/fullscreen/exit-full-screen-video-crash-expected.txt: Added.
* LayoutTests/fullscreen/exit-full-screen-video-crash.html: Added.
* Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.mm:
(WebKit::VideoFullscreenManager::exitVideoFullscreenForVideoElement):
(WebKit::VideoFullscreenManager::exitVideoFullscreenToModeWithoutAnimation):

Originally-landed-as: 259548.703@safari-7615-branch (0ffc79d). rdar://113167859
Canonical link: https://commits.webkit.org/266584@main
  • Loading branch information
chirags27 authored and JonWBedard committed Aug 4, 2023
1 parent e4c0a68 commit 549d44e
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 0 deletions.
2 changes: 2 additions & 0 deletions LayoutTests/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -6552,3 +6552,5 @@ webkit.org/b/257904 http/tests/site-isolation/basic-iframe.html [ Skip ]

# Only some OSes have support for auto word breaking.
imported/w3c/web-platform-tests/css/css-text/word-break/auto [ Pass Failure ImageOnlyFailure ]

webkit.org/b/255970 [ Debug ] fullscreen/exit-full-screen-video-crash.html [ Crash ]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
PASS if no crash
17 changes: 17 additions & 0 deletions LayoutTests/fullscreen/exit-full-screen-video-crash.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<script>
onload = async () => {
if (window.testRunner)
testRunner.dumpAsText();
internals.withUserGesture(() => {});
let video0 = document.createElement('video');
video0.src = 'data:video/mp4;';
document.body.append(video0);
$vm.print(await video0.requestFullscreen());
navigator.mediaSession.callActionHandler({action: 'play'});
let div0 = document.createElement('div');
video0.append(div0);
$vm.print(await div0.requestFullscreen());
video0.webkitSetPresentationMode('inline');
document.body.innerHTML = 'PASS if no crash';
};
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
CONSOLE MESSAGE: Unhandled Promise Rejection: TypeError: video0.webkitSetPresentationMode is not a function. (In 'video0.webkitSetPresentationMode('inline')', 'video0.webkitSetPresentationMode' is undefined)

10 changes: 10 additions & 0 deletions Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.mm
Original file line number Diff line number Diff line change
Expand Up @@ -409,9 +409,16 @@ static FloatRect inlineVideoFrame(HTMLVideoElement& element)
void VideoFullscreenManager::exitVideoFullscreenForVideoElement(HTMLVideoElement& videoElement, CompletionHandler<void(bool)>&& completionHandler)
{
INFO_LOG(LOGIDENTIFIER);
LOG(Fullscreen, "VideoFullscreenManager::exitVideoFullscreenForVideoElement(%p)", this);

ASSERT(m_page);
ASSERT(m_videoElements.contains(videoElement));

if (!m_videoElements.contains(videoElement)) {
completionHandler(false);
return;
}

auto contextId = m_videoElements.get(videoElement);
auto& interface = ensureInterface(contextId);
if (interface.animationState() != VideoFullscreenInterfaceContext::AnimationType::None) {
Expand Down Expand Up @@ -442,6 +449,9 @@ static FloatRect inlineVideoFrame(HTMLVideoElement& element)
ASSERT(m_page);
ASSERT(m_videoElements.contains(videoElement));

if (!m_videoElements.contains(videoElement))
return;

if (m_videoElementInPictureInPicture == &videoElement)
m_videoElementInPictureInPicture = nullptr;

Expand Down

0 comments on commit 549d44e

Please sign in to comment.