Bug 212762 – REGRESSION (r262456?): [macOS] media/picture-in-picture/picture-in-picture-api tests are flaky timeouts

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

[Ryan Haddad]

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

[Radar WebKit Bug Importer]

<rdar://problem/63985839>

[Ryan Haddad]

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

[Ryan Haddad]

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

[Peng Liu]

Created attachment 401077 [details]
Patch

[Darin Adler]

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?

[Peng Liu]

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.

[Peng Liu]

Created attachment 401090 [details]
Revise the patch to fix test failures

[Peng Liu]

Created attachment 401129 [details]
Fix the API test failure

[Peng Liu]

Created attachment 401165 [details]
Rebase the patch

[EWS]

Committed r262634: <https://trac.webkit.org/changeset/262634>

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