Bug 212762

Summary: REGRESSION (r262456?): [macOS] media/picture-in-picture/picture-in-picture-api tests are flaky timeouts
Product: WebKit Reporter: Ryan Haddad <ryanhaddad>
Component: MediaAssignee: Peng Liu <peng.liu6>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, changseok, darin, eric.carlson, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, jer.noble, peng.liu6, philipj, sergio, webkit-bot-watchers-bugzilla, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=202425
Attachments:
Description Flags
Patch
none
Revise the patch to fix test failures
none
Fix the API test failure
eric.carlson: review+
Rebase the patch none

Description Ryan Haddad 2020-06-04 10:51:17 PDT 
The following two tests recently became flaky timeouts on Catalina and Mojave

media/picture-in-picture/picture-in-picture-api-enter-pip-4.html
media/picture-in-picture/picture-in-picture-api-exit-pip-1.html

https://results.webkit.org/?suite=layout-tests&suite=layout-tests&test=media%2Fpicture-in-picture%2Fpicture-in-picture-api-enter-pip-4.html&test=media%2Fpicture-in-picture%2Fpicture-in-picture-api-exit-pip-1.html
Comment 1 Radar WebKit Bug Importer 2020-06-04 10:51:43 PDT 
<rdar://problem/63985839>
Comment 2 Ryan Haddad 2020-06-04 11:04:48 PDT 
This started shortly after the following change landed:
Stressing webkitSetPresentationMode leads to wrong inline video dimensions
​https://bugs.webkit.org/show_bug.cgi?id=202425
Comment 3 Ryan Haddad 2020-06-04 12:40:41 PDT 
Same with media/picture-in-picture/picture-in-picture-api-pip-window.html
https://results.webkit.org/?suite=layout-tests&test=media%2Fpicture-in-picture%2Fpicture-in-picture-api-pip-window.html
Comment 4 Peng Liu 2020-06-04 14:06:18 PDT 
Created attachment 401077 [details]
Patch
Comment 5 Darin Adler 2020-06-04 14:25:01 PDT 
Comment on attachment 401077 [details]
Patch

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

> Source/WebKit/ChangeLog:10
> +        The Picture-in-Picture implementation expects the two callbacks HTMLVideoElement::didBecomeFullscreenElement()
> +        and HTMLVideoElement::setVideoFullscreenFrame() to come in order. So we need to remove the dispatch_async() to
> +        call HTMLVideoElement::didBecomeFullscreenElement().

That’s the benefit of removing the dispatch_async.

But why is it safe to remove dispatch_async?
Comment 6 Peng Liu 2020-06-04 14:49:07 PDT 
Comment on attachment 401077 [details]
Patch

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

>> Source/WebKit/ChangeLog:10
>> +        call HTMLVideoElement::didBecomeFullscreenElement().
> 
> That’s the benefit of removing the dispatch_async.
> 
> But why is it safe to remove dispatch_async?

The dispatch_async() was added in r229714 because of some issue on Watch, and I have discussed it with Eric. Removing it seems fine for now.
Comment 7 Peng Liu 2020-06-04 16:22:39 PDT 
Created attachment 401090 [details]
Revise the patch to fix test failures
Comment 8 Peng Liu 2020-06-05 00:55:12 PDT 
Created attachment 401129 [details]
Fix the API test failure
Comment 9 Peng Liu 2020-06-05 09:48:22 PDT 
Created attachment 401165 [details]
Rebase the patch
Comment 10 EWS 2020-06-05 11:02:45 PDT 
Committed r262634: <https://trac.webkit.org/changeset/262634>

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