Bug 72758 – Use canplay instead of canplaythrough in http/tests/media to prevent timeout

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Victoria Kirst]

Use canplay instead of canplaythrough in http/tests/media to prevent timeout

[Victoria Kirst]

Created attachment 115870 [details]
Patch

[Eric Carlson]

Why does 'canplaythough' cause these tests to time out?

[Victoria Kirst]

(In reply to comment #2)
> Why does 'canplaythough' cause these tests to time out?

In my 1st stab implementation of canplaythrough for chromium (that landed today and so far hasn't been reverted! http://codereview.chromium.org/8399023/), here is my main criteria for firing canplaythrough:

 a) Sample incoming downloading for 5 seconds and calculate download rate per second. If download rate >= media bitrate fire canplaythrough.

 b) But if we download 4 seconds of data in less 2 seconds (i.e. downloading least 2x faster than consuming), fire canplaythrough without waiting for the 5s window

 c) If we've buffered to the end of the file and still haven't fired canplaythrough, fire canplaythrough. This also means fully loaded source (e.g. file:// protocol) fires canplaythrough immediately.

Running the tests from my local machine, the http test serves video data fast enough to trigger scenario (b). But it looks like on our windows bots, scenario (a) or (c) is happening, which is causing the test to time out.

It doesn't appear checking for canplaythrough is needed for these tests, so I changed the event to canplay, an event that should trigger much earlier and more predictably. (Of course we shouldn't change the test to potentially cover up other bugs, though!)

[Adam Klein]

Note that at least one other tests saw some behavior differences due to this change in canplaythrough:

compositor/geometry/clipped-video-controller.html

In that case, it wasn't a timeout, but a difference in layout (scrollHeight 293 vs 292)

[Eric Carlson]

(In reply to comment #4)
> Note that at least one other tests saw some behavior differences due to this change in canplaythrough:
> 
> compositor/geometry/clipped-video-controller.html
> 
> In that case, it wasn't a timeout, but a difference in layout (scrollHeight 293 vs 292)

I can't imagine why the intrinsic size of the element would change by one pixel. This sounds like a bug, has anyone checked to see what is happening?

[Eric Carlson]

(In reply to comment #3)
> Running the tests from my local machine, the http test serves video data fast enough to trigger scenario (b). But it looks like on our windows bots, scenario (a) or (c) is happening, which is causing the test to time out.
> 
> It doesn't appear checking for canplaythrough is needed for these tests, so I changed the event to canplay, an event that should trigger much earlier and more predictably. (Of course we shouldn't change the test to potentially cover up other bugs, though!)

I agree that having these tests trigger on 'canplay' instead of 'canplaythough' won't change them, but I worry that you are papering over a problem in your new implementation because I don't see how a file loaded from a same-machine web server could load slowly enough to not trigger your logic.

[Adam Klein]

(In reply to comment #5)
> (In reply to comment #4)
> > Note that at least one other tests saw some behavior differences due to this change in canplaythrough:
> > 
> > compositor/geometry/clipped-video-controller.html
> > 
> > In that case, it wasn't a timeout, but a difference in layout (scrollHeight 293 vs 292)
> 
> I can't imagine why the intrinsic size of the element would change by one pixel. This sounds like a bug, has anyone checked to see what is happening?

James was the one who suggested this on IRC, I'll leave the explanation to him.

[WebKit Review Bot]

Comment on attachment 115870 [details]
Patch

Clearing flags on attachment: 115870

Committed r100936: <http://trac.webkit.org/changeset/100936>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Eric Carlson]

(In reply to comment #7)
> (In reply to comment #5)
> > 
> > I can't imagine why the intrinsic size of the element would change by one pixel. This sounds like a bug, has anyone checked to see what is happening?
> 
> James was the one who suggested this on IRC, I'll leave the explanation to him.

https://bugs.webkit.org/show_bug.cgi?id=72903 is also about a 1px change when a test is triggered by 'canplaythrough' instead of 'canplay'. In that bug James says:

    I don't feel qualified enough on these events to know what the 
    change does.  Can someone familiar with when these events fire 
    and what might change the render tree please chime in with a 
    theory of how this could help? 

and Victoria says:

    I don't know how this could cause a 1px rendering difference

but the patch was landed anyway - as this one was. 

Does anyone know what caused the rendering change?

[Adam Klein]

(In reply to comment #10)
> (In reply to comment #7)
> > (In reply to comment #5)
> > > 
> > > I can't imagine why the intrinsic size of the element would change by one pixel. This sounds like a bug, has anyone checked to see what is happening?
> > 
> > James was the one who suggested this on IRC, I'll leave the explanation to him.
> 
> https://bugs.webkit.org/show_bug.cgi?id=72903 is also about a 1px change when a test is triggered by 'canplaythrough' instead of 'canplay'. In that bug James says:
> 
>     I don't feel qualified enough on these events to know what the 
>     change does.  Can someone familiar with when these events fire 
>     and what might change the render tree please chime in with a 
>     theory of how this could help? 
> 
> and Victoria says:
> 
>     I don't know how this could cause a 1px rendering difference
> 
> but the patch was landed anyway - as this one was. 

Note that the patch on 72903 ended up being a better-understood workaround: the pixel results weren't flaky, only the text layout results, and so we suppressed the latter (as James pointed out, we only cared about the pixel output for that test anyway).

> Does anyone know what caused the rendering change?

But your point here still stands, the root cause of the difference in rendering has yet to be determined.