Bug 76113 – Reduce throttling on video-buffering-repaints-controls test to prevent timeout.

• Component: New Bugs
• Status: RESOLVED WONTFIX

[Dale Curtis]

Reduce throttling on video-buffering-repaints-controls test to prevent timeout.

[Dale Curtis]

Created attachment 122115 [details]
Patch

[Dale Curtis]

apavlov: Can you review/commit?

I'll redo baselines once this lands. https://bugs.webkit.org/show_bug.cgi?id=75570

[Dale Curtis]

Created attachment 122154 [details]
Patch

[Alexander Pavlov (apavlov)]

(In reply to comment #2)
> apavlov: Can you review/commit?
> 
> I'll redo baselines once this lands. https://bugs.webkit.org/show_bug.cgi?id=75570

I'm not a reviewer, but Dmitry is (cc'ed). Hope he can review.

[Dmitry Titov]

I don't feel like I have enough context to review this patch. I don't know what throttle in this case is and why it is preventing timeout... Perhaps someone more familiar with video controls could take a look?

[Dale Curtis]

+dglazkov

The test uses a throttled http server to slow down loading of a local video. With the previous 10kb/s throttling the test was too slow and was hitting the global 6 sec test timeout on occasion. I've bumped this throttling from 10kb/s to 80kb/s.

I used a 10kb/s previously because it was the only way I had found to reduce flakiness. Too fast and the progress bar moves an indeterminate number of steps ahead before pixel snapshot is saved.

I also noticed some flaky failures during debugging, so I switched the test to check for repaints between the last progress event and the suspend event. Since this requires loading ~all of the file, the rate needed to be higher to prevent test timeouts.

The rate can't be made too fast though or the progress bar loads ~instantly and shows the correct status even on the old pre-buffering fix code. 80kb/s seems to be roughly the sweet spot.

I hope this explains the patch well enough for review. Thanks in advance!

[Dale Curtis]

Created attachment 123033 [details]
Patch

[Dale Curtis]

+jamesr who offered to look at this on IRC.

[James Robinson]

Comment on attachment 123033 [details]
Patch

OK, let's see how this goes. R=me

[WebKit Review Bot]

Comment on attachment 123033 [details]
Patch

Rejecting attachment 123033 [details] from commit-queue.

New failing tests:
http/tests/media/video-buffering-repaints-controls.html
Full output: http://queues.webkit.org/results/11132022

[Dale Curtis]

Created attachment 123162 [details]
Patch

[Dale Curtis]

jamesr: PTAL. Added an IMAGE failure to test_expectations for chromium. Other platforms already have this.

[WebKit Review Bot]

Comment on attachment 123162 [details]
Patch

Clearing flags on attachment: 123162

Committed r105651: <http://trac.webkit.org/changeset/105651>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Dale Curtis]

Reopening to attach new patch.

[Dale Curtis]

Created attachment 124023 [details]
Patch

[Dale Curtis]

Partial revert of the last patch set. The last patch caused timeouts on GTK/Mac due to suspend event not firing. Results were also flaky on Chrome platforms, more so than before.

Now I've decreased throttling significantly (to 200kb/s) to try and ensure the progress bar has buffered to 100% when the pixel dump is taken. 100+ local A/B testing show 100 passes w/ fix and 100 failures w/o.

Not sure what else to do if this doesn't work. Repaint testing is a pain :-/

[Martin Robinson]

*** Bug 77019 has been marked as a duplicate of this bug. ***

[Dale Curtis]

Created attachment 128266 [details]
Patch

[Dale Curtis]

abarth: You were kind enough to land this change for me last time, can you take a look at another attempt to de-flake. Thanks!

[Dale Curtis]

Closing, test removed in https://bugs.webkit.org/show_bug.cgi?id=83097

[Eric Seidel (no email)]

Comment on attachment 128266 [details]
Patch

Cleared review? from attachment 128266 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).