Bug 229453 – [Monterey] LayoutTest media/element-containing-pip-video-going-into-fullscreen.html is flaky timeout/crash

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Peng Liu]

ASSERTION FAILED: standby || mode != HTMLMediaElementEnums::VideoFullscreenModeNone
/Volumes/Data/worker/trunk-star-debug-archive/build/OpenSource/Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.mm(252) : void WebKit::VideoFullscreenManager::enterVideoFullscreenForVideoElement(WebCore::HTMLVideoElement &, HTMLMediaElementEnums::VideoFullscreenMode, bool)
1   0x40f3b5e79 WTFCrash
2   0x3ee73aa9b WTFCrashWithInfo(int, char const*, char const*, int)
3   0x3f01e4224 WebKit::VideoFullscreenManager::enterVideoFullscreenForVideoElement(WebCore::HTMLVideoElement&, unsigned int, bool)
4   0x3f0228e69 WebKit::VideoFullscreenManager::didCleanupFullscreen(WTF::ObjectIdentifier<WebKit::PlaybackSessionContextIdentifierType>)::$_18::operator()() const
5   0x3f0228d29 WTF::Detail::CallableWrapper<WebKit::VideoFullscreenManager::didCleanupFullscreen(WTF::ObjectIdentifier<WebKit::PlaybackSessionContextIdentifierType>)::$_18, void>::call()
6   0x40f3ded42 WTF::Function<void ()>::operator()() const
7   0x40f45a81e WTF::RunLoop::performWork()
8   0x40f45e0fe WTF::RunLoop::performWork(void*)
9   0x7ff8011f3d4a __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__
10  0x7ff8011f3cb2 __CFRunLoopDoSource0
11  0x7ff8011f3a27 __CFRunLoopDoSources0
12  0x7ff8011f2447 __CFRunLoopRun
13  0x7ff8011f1a09 CFRunLoopRunSpecific
14  0x7ff80204d437 -[NSRunLoop(NSRunLoop) runMode:beforeDate:]
15  0x7ff8020d7b13 -[NSRunLoop(NSRunLoop) run]
16  0x7ff800e84377 _xpc_objc_main
17  0x7ff800e83cfa xpc_main
18  0x3ef54e56f WebKit::XPCServiceMain(int, char const**)
19  0x3f0be007b WKXPCServiceMain
20  0x10190cea2 main
21  0x10a4b44d5
com.apple.WebKit.WebContent.Development terminated (pid 46200) because the process crashed

[Peng Liu]

<rdar://80346428>

[Peng Liu]

Created attachment 436306 [details]
Patch

[Eric Carlson]

Comment on attachment 436306 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=436306&action=review

> Source/WebKit/ChangeLog:16
> +        This change fixes an assertion failure in `enterVideoFullscreenForVideoElement()`.

This requires the reader to look at the code before and after the change. Instead I think it would be better to say something like "Return immediately if the element is not in fullscreen to avoid an assertion later in `enterVideoFullscreenForVideoElement`".

[Peng Liu]

Created attachment 436312 [details]
Revise the patch based on Eric's comment

[Peng Liu]

Comment on attachment 436306 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=436306&action=review

>> Source/WebKit/ChangeLog:16
>> +        This change fixes an assertion failure in `enterVideoFullscreenForVideoElement()`.
> 
> This requires the reader to look at the code before and after the change. Instead I think it would be better to say something like "Return immediately if the element is not in fullscreen to avoid an assertion later in `enterVideoFullscreenForVideoElement`".

Agree! Fixed.

[Peng Liu]

The layout test failure on mac-debug-wk1 bot is not relevant to this patch.

[EWS]

Committed r281557 (240924@main): <https://commits.webkit.org/240924@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 436312 [details].

[Alexey Proskuryakov]

Comment on attachment 436312 [details]
Revise the patch based on Eric's comment

View in context: https://bugs.webkit.org/attachment.cgi?id=436312&action=review

> LayoutTests/platform/mac-wk2/TestExpectations:691
> +[ Catalina BigSur Monterey ] media/element-containing-pip-video-going-into-fullscreen.html [ Pass ]

"Catalina BigSur Monterey" is all of them, so we don't need the OS version qualifiers. In fact, they are harmful, as they result in the test being skipped on all future versions.