RESOLVED WONTFIX 76113
Reduce throttling on video-buffering-repaints-controls test to prevent timeout.
https://bugs.webkit.org/show_bug.cgi?id=76113
Summary Reduce throttling on video-buffering-repaints-controls test to prevent timeout.
Dale Curtis
Reported 2012-01-11 16:01:58 PST
Reduce throttling on video-buffering-repaints-controls test to prevent timeout.
Attachments
Patch (1.55 KB, patch)
2012-01-11 16:02 PST, Dale Curtis
no flags
Patch (46.24 KB, patch)
2012-01-11 19:04 PST, Dale Curtis
no flags
Patch (51.97 KB, patch)
2012-01-18 16:28 PST, Dale Curtis
no flags
Patch (52.87 KB, patch)
2012-01-19 11:57 PST, Dale Curtis
no flags
Patch (3.16 KB, patch)
2012-01-25 15:19 PST, Dale Curtis
no flags
Patch (3.16 KB, patch)
2012-02-22 12:33 PST, Dale Curtis
no flags
Dale Curtis
Comment 1 2012-01-11 16:02:59 PST
Dale Curtis
Comment 2 2012-01-11 16:05:01 PST
apavlov: Can you review/commit? I'll redo baselines once this lands. https://bugs.webkit.org/show_bug.cgi?id=75570
Dale Curtis
Comment 3 2012-01-11 19:04:20 PST
Alexander Pavlov (apavlov)
Comment 4 2012-01-11 23:05:10 PST
(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
Comment 5 2012-01-11 23:17:04 PST
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
Comment 6 2012-01-12 10:24:35 PST
+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
Comment 7 2012-01-18 16:28:29 PST
Dale Curtis
Comment 8 2012-01-18 16:40:47 PST
+jamesr who offered to look at this on IRC.
James Robinson
Comment 9 2012-01-18 16:48:22 PST
Comment on attachment 123033 [details] Patch OK, let's see how this goes. R=me
WebKit Review Bot
Comment 10 2012-01-18 22:38:07 PST
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
Comment 11 2012-01-19 11:57:52 PST
Dale Curtis
Comment 12 2012-01-19 12:03:40 PST
jamesr: PTAL. Added an IMAGE failure to test_expectations for chromium. Other platforms already have this.
WebKit Review Bot
Comment 13 2012-01-23 16:01:09 PST
Comment on attachment 123162 [details] Patch Clearing flags on attachment: 123162 Committed r105651: <http://trac.webkit.org/changeset/105651>
WebKit Review Bot
Comment 14 2012-01-23 16:01:14 PST
All reviewed patches have been landed. Closing bug.
Dale Curtis
Comment 15 2012-01-25 15:19:38 PST
Reopening to attach new patch.
Dale Curtis
Comment 16 2012-01-25 15:19:41 PST
Dale Curtis
Comment 17 2012-01-25 15:23:33 PST
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
Comment 18 2012-01-25 15:35:03 PST
*** Bug 77019 has been marked as a duplicate of this bug. ***
Dale Curtis
Comment 19 2012-02-22 12:33:03 PST
Dale Curtis
Comment 20 2012-02-22 13:24:02 PST
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
Comment 21 2012-04-09 16:07:38 PDT
Eric Seidel (no email)
Comment 22 2012-04-19 14:18:51 PDT
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).
Note You need to log in before you can comment on or make changes to this bug.