Bug 76113 - Reduce throttling on video-buffering-repaints-controls test to prevent timeout.
Summary: Reduce throttling on video-buffering-repaints-controls test to prevent timeout.
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 77019 (view as bug list)
Depends on:
Blocks: 75570
  Show dependency treegraph
 
Reported: 2012-01-11 16:01 PST by Dale Curtis
Modified: 2012-04-19 14:18 PDT (History)
7 users (show)

See Also:


Attachments
Patch (1.55 KB, patch)
2012-01-11 16:02 PST, Dale Curtis
no flags Details | Formatted Diff | Diff
Patch (46.24 KB, patch)
2012-01-11 19:04 PST, Dale Curtis
no flags Details | Formatted Diff | Diff
Patch (51.97 KB, patch)
2012-01-18 16:28 PST, Dale Curtis
no flags Details | Formatted Diff | Diff
Patch (52.87 KB, patch)
2012-01-19 11:57 PST, Dale Curtis
no flags Details | Formatted Diff | Diff
Patch (3.16 KB, patch)
2012-01-25 15:19 PST, Dale Curtis
no flags Details | Formatted Diff | Diff
Patch (3.16 KB, patch)
2012-02-22 12:33 PST, Dale Curtis
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dale Curtis 2012-01-11 16:01:58 PST
Reduce throttling on video-buffering-repaints-controls test to prevent timeout.
Comment 1 Dale Curtis 2012-01-11 16:02:59 PST
Created attachment 122115 [details]
Patch
Comment 2 Dale Curtis 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
Comment 3 Dale Curtis 2012-01-11 19:04:20 PST
Created attachment 122154 [details]
Patch
Comment 4 Alexander Pavlov (apavlov) 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.
Comment 5 Dmitry Titov 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?
Comment 6 Dale Curtis 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!
Comment 7 Dale Curtis 2012-01-18 16:28:29 PST
Created attachment 123033 [details]
Patch
Comment 8 Dale Curtis 2012-01-18 16:40:47 PST
+jamesr who offered to look at this on IRC.
Comment 9 James Robinson 2012-01-18 16:48:22 PST
Comment on attachment 123033 [details]
Patch

OK, let's see how this goes. R=me
Comment 10 WebKit Review Bot 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
Comment 11 Dale Curtis 2012-01-19 11:57:52 PST
Created attachment 123162 [details]
Patch
Comment 12 Dale Curtis 2012-01-19 12:03:40 PST
jamesr: PTAL. Added an IMAGE failure to test_expectations for chromium. Other platforms already have this.
Comment 13 WebKit Review Bot 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>
Comment 14 WebKit Review Bot 2012-01-23 16:01:14 PST
All reviewed patches have been landed.  Closing bug.
Comment 15 Dale Curtis 2012-01-25 15:19:38 PST
Reopening to attach new patch.
Comment 16 Dale Curtis 2012-01-25 15:19:41 PST
Created attachment 124023 [details]
Patch
Comment 17 Dale Curtis 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 :-/
Comment 18 Martin Robinson 2012-01-25 15:35:03 PST
*** Bug 77019 has been marked as a duplicate of this bug. ***
Comment 19 Dale Curtis 2012-02-22 12:33:03 PST
Created attachment 128266 [details]
Patch
Comment 20 Dale Curtis 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!
Comment 21 Dale Curtis 2012-04-09 16:07:38 PDT
Closing, test removed in https://bugs.webkit.org/show_bug.cgi?id=83097
Comment 22 Eric Seidel (no email) 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).