Bug 223762 – [ macOS wk2 ] media/pip-video-going-into-fullscreen.html is a flakey timeout

• Component: Media
• Status: RESOLVED FIXED

[Robert Jenner]

media/pip-video-going-into-fullscreen.html

is a flakey timeout in macOS release and debug for Catalina and BigSur in wk2. 

HISTORY URL:
https://results.webkit.org/?suite=layout-tests&test=media%2Fpip-video-going-into-fullscreen.html

DIFF TEXT:
-Tests pip video going into fullscreen should exit pip
+#PID UNRESPONSIVE - WebKitTestRunner (pid 90418)
+FAIL: Timed out waiting for notifyDone to be called
 
-Going into Picture-in-Picture
-EXPECTED (video.webkitPresentationMode == 'picture-in-picture') OK
-Going into Full Screen from Picture-in-Picture
-EXPECTED (document.webkitCurrentFullScreenElement == '[object HTMLVideoElement]') OK
-EXPECTED (video.webkitPresentationMode == 'fullscreen') OK
-END OF TEST
-
+#EOF
+#EOF

[Robert Jenner]

I was able to reproduce the timeout by generating a test list and running it as the following test:

run-webkit-test --test-list <path to test list> --child-process=1

I have attached the test list I used for reproduction to this bug.

[Robert Jenner]

Created attachment 424283 [details]
Test list used to reproduce crash.

Attaching test list used to reproduce crash. Currently working on narrowing down root cause.

[Radar WebKit Bug Importer]

<rdar://problem/75854906>

[Robert Jenner]

(In reply to Robert Jenner from comment #2)
> Created attachment 424283 [details]
> Test list used to reproduce crash.
> 
> Attaching test list used to reproduce crash. Currently working on narrowing
> down root cause.


And by Crash, I mean Timeout. I was able to reproduce the timeout.

[Robert Jenner]

I was able to bisect it down to a range of revisions, as that was all that was available. 

Running the test on r274796 did not produce the timeout. Running at r274822 did produce the timeout. 

It does look like changes at r274810 do deal with media controls and PIP tests. So that may hav caused the timeouts. 
 
https://trac.webkit.org/changeset/274810/webkit

[Robert Jenner]

Updated test expectations to Pass Timeout while test is being reviewed:

https://trac.webkit.org/changeset/275167/webkit

[Peng Liu]

Created attachment 424802 [details]
Patch

[Peng Liu]

Created attachment 424805 [details]
Rebase the patch

[Eric Carlson]

Comment on attachment 424805 [details]
Rebase the patch

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

> LayoutTests/media/pip-video-going-into-fullscreen.html:44
> +            video.addEventListener('webkitfullscreenchange', onbeginfullscreen);

Nit: you use `video.addEventListener('webkitfullscreenchange', onbeginfullscreen, {once: true})` you won't have to remove the listener below.

[Peng Liu]

Comment on attachment 424805 [details]
Rebase the patch

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

>> LayoutTests/media/pip-video-going-into-fullscreen.html:44
>> +            video.addEventListener('webkitfullscreenchange', onbeginfullscreen);
> 
> Nit: you use `video.addEventListener('webkitfullscreenchange', onbeginfullscreen, {once: true})` you won't have to remove the listener below.

Good idea! Will fix it. Thanks!

[Peng Liu]

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

[EWS]

Committed r275328: <https://commits.webkit.org/r275328>

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