[webkit-reviews] review granted: [Bug 210349] [ Mac wk2 Debug ] media/modern-media-controls/media-controller/media-controller-auto-hide.html is flaky timing out. : [Attachment 396652] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Antoine Quint
<graouts at apple.com>'s request for review:
Bug 210349: [ Mac wk2 Debug ]
media/modern-media-controls/media-controller/media-controller-auto-hide.html is
flaky timing out.
https://bugs.webkit.org/show_bug.cgi?id=210349

Attachment 396652: Patch

https://bugs.webkit.org/attachment.cgi?id=396652&action=review




--- Comment #5 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 396652
  --> https://bugs.webkit.org/attachment.cgi?id=396652
Patch

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

> LayoutTests/ChangeLog:19
> +	   This test had several things wrong about it. First, it manually
created a MediaController in the test page and set a custom auto-hide delay on
it.
> +	   However, it would test the proper behavior of the auto-hide behavior
on the MediaController created by the media element, querying the state of
> +	   the media controls markup within that media element's shadow root.
In that world, the auto-hide delay was the default delay, and not the custom
> +	   one set on the page-created MediaController.
> +
> +	   The cause of the timeout was that the media clip used in this test
is barely longer than the auto-hide delay, and it could happen that under slow
> +	   conditions, the media would end and the auto-hide timer would never
have a chance to fire since it would be canceled due to auto-hide being
disabled
> +	   when the media is not playing, such as when it ended.
> +
> +	   We make this test saner and more realistic by not customizing the
auto-hide delay, removing the page MediaController and simply setting "loop" on
the
> +	   media element such that it never stops playing, givin the auto-hide
timer additional time to fire with no risk of it being canceled by the media
ending.

A+ for the ChangeLog comments!