|Summary:||LayoutTest media/modern-media-controls/media-controller/media-controller-auto-hide.html is flaky|
|Product:||WebKit||Reporter:||Ryan Haddad <ryanhaddad>|
|Component:||New Bugs||Assignee:||Antoine Quint <graouts>|
|Severity:||Normal||CC:||commit-queue, graouts, webkit-bug-importer|
Description Ryan Haddad 2017-01-20 15:54:02 PST
LayoutTest media/modern-media-controls/media-controller/media-controller-auto-hide.html is flaky https://build.webkit.org/results/Apple%20El%20Capitan%20Debug%20WK1%20(Tests)/r210986%20(10966)/results.html https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=media%2Fmodern-media-controls%2Fmedia-controller%2Fmedia-controller-auto-hide.html --- /Volumes/Data/slave/elcapitan-debug-tests-wk1/build/layout-test-results/media/modern-media-controls/media-controller/media-controller-auto-hide-expected.txt +++ /Volumes/Data/slave/elcapitan-debug-tests-wk1/build/layout-test-results/media/modern-media-controls/media-controller/media-controller-auto-hide-actual.txt @@ -7,7 +7,7 @@ PASS controlsBar.classList.contains('faded') is false Auto-hide timer has been going for 200ms, controls bar should still be visible. -PASS controlsBar.classList.contains('faded') is false +FAIL controlsBar.classList.contains('faded') should be false. Was true. Auto-hide timer has elapsed, controls bar should be faded. PASS controlsBar.classList.contains('faded') is true
Comment 1 Ryan Haddad 2017-01-20 16:02:37 PST
Marked as flaky in https://trac.webkit.org/changeset/210994
Comment 2 Antoine Quint 2017-01-25 05:38:25 PST
This is tricky because this test, and a host of others, is timing-sensitive by design since it's testing the auto-hide timer. The test checks as close as possible to the auto-hide delay that the controls are still visible, and then again as soon as possible after the auto-hide delay has elapsed to check that the controls are no longer visible. So in certain cases, probably where the system is under pressure and timers run late, we get in cases where the check prior to the timer firing fails because we've actually run past auto-hide delay. We're going to have to find a different way to test this.
Comment 3 Antoine Quint 2017-02-03 06:08:36 PST
OK, I think I understand what makes this test flaky. First, the test makes the assumption that the auto-hide timer can only be started as a result of the "play" event firing… but it turns out that when "loadedmetadata" fires, media.paused can be set to "false", and ControlsVisibilitySupport which governs when the controls bar is set to have fadesWhileIdle = true, uses both "loadedmetadata" and "play" as triggers provided media.paused = "false". So that's the first issue, since we could potentially wait enough time until we'd assume the auto-hide timer started to make our 200ms and 300ms timers off-base enough. Second, while the system is under stress (in my case I have a full WK build and running the test with GuardMalloc), there might be a significant delta between the moment the same event is handled within the media controls code and when it's handled in the test. Locally, I can see something around 85ms as the delay. This would cause another significant delay to make the 200ms and 300ms timers off-base. So this test is definitely flaky by nature, and all of the other auto-hide tests, many of which are showing flakiness, suffer from the same design flaw. I'll need to think of a better, more robust way to write those tests.
Comment 6 Antoine Quint 2017-03-02 17:50:26 PST
Created attachment 303275 [details] Patch for landing
Comment 7 WebKit Commit Bot 2017-03-02 18:04:27 PST
Comment on attachment 303275 [details] Patch for landing Clearing flags on attachment: 303275 Committed r213337: <http://trac.webkit.org/changeset/213337>
Comment 8 WebKit Commit Bot 2017-03-02 18:04:31 PST
All reviewed patches have been landed. Closing bug.