Bug 72903 - [chromium] Layout Test compositing/geometry/clipped-video-controller.html is flaky on Win/Linux
Summary: [chromium] Layout Test compositing/geometry/clipped-video-controller.html is ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adam Klein
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-21 11:36 PST by Adam Klein
Modified: 2011-11-21 15:31 PST (History)
3 users (show)

See Also:


Attachments
Patch (1.67 KB, patch)
2011-11-21 14:17 PST, Adam Klein
no flags Details | Formatted Diff | Diff
Remove expectation (2.40 KB, patch)
2011-11-21 14:18 PST, Adam Klein
no flags Details | Formatted Diff | Diff
Use dumpAsText (7.03 KB, patch)
2011-11-21 14:37 PST, Adam Klein
no flags Details | Formatted Diff | Diff
dumpAsText(true) (7.04 KB, patch)
2011-11-21 14:51 PST, Adam Klein
jamesr: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Klein 2011-11-21 11:36:08 PST
The following layout test is flaky on Chromium Linux and Windows.

compositing/geometry/clipped-video-controller.html

Flakiness dashboard:

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=compositing%2Fgeometry%2Fclipped-video-controller.html

The diff is a single pixel of scrollheight:

-layer at (20,50) size 70x70 clip at (30,60) size 50x50 scrollWidth 352 scrollHeight 293
+layer at (20,50) size 70x70 clip at (30,60) size 50x50 scrollWidth 352 scrollHeight 292

There doesn't seem to be any culprit on the WebKit side. In Chromium, the timing lined up with http://src.chromium.org/viewvc/chrome?view=rev&revision=110733 (and thus I've CCed vrk@chromium.org), but the test doesn't seem to have anything in particular to do with the canplaythrough event.  James mentioned on IRC that this failure might be due to timing, so I've CCed him as well in case he has further thoughts.
Comment 1 Adam Klein 2011-11-21 11:39:39 PST
Marked as PASS TEXT in http://trac.webkit.org/changeset/100941
Comment 2 James Robinson 2011-11-21 12:34:44 PST
The test uses the canplaythrough event to finish the test. I'm not sure why the render tree is different when this event is fired.
Comment 3 Adam Klein 2011-11-21 13:48:25 PST
(In reply to comment #2)
> The test uses the canplaythrough event to finish the test. I'm not sure why the render tree is different when this event is fired.

Ah, I got my tests mixed up (was looking at a different one that didn't mention canplaythrough).  I'm tempted to change this to use the 'canplay' event instead.
Comment 4 Adam Klein 2011-11-21 14:17:10 PST
Created attachment 116134 [details]
Patch
Comment 5 Adam Klein 2011-11-21 14:18:34 PST
Created attachment 116136 [details]
Remove expectation
Comment 6 James Robinson 2011-11-21 14:19:36 PST
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? I'd be happy to review officially if there's a valid-sounding story of how this could fix the test.
Comment 7 Adam Klein 2011-11-21 14:37:47 PST
Created attachment 116138 [details]
Use dumpAsText
Comment 8 James Robinson 2011-11-21 14:45:11 PST
Comment on attachment 116138 [details]
Use dumpAsText

View in context: https://bugs.webkit.org/attachment.cgi?id=116138&action=review

> LayoutTests/compositing/geometry/clipped-video-controller.html:23
> +      layoutTestController.dumpAsText();

this needs to be dumpAsText(true) or it won't produce any pixel output
Comment 9 Adam Klein 2011-11-21 14:51:36 PST
Created attachment 116140 [details]
dumpAsText(true)
Comment 10 Adam Klein 2011-11-21 14:51:54 PST
Comment on attachment 116138 [details]
Use dumpAsText

View in context: https://bugs.webkit.org/attachment.cgi?id=116138&action=review

>> LayoutTests/compositing/geometry/clipped-video-controller.html:23
>> +      layoutTestController.dumpAsText();
> 
> this needs to be dumpAsText(true) or it won't produce any pixel output

Done. Learn something new every day...
Comment 11 Victoria Kirst 2011-11-21 14:52:05 PST
(In reply to comment #6)
> 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? I'd be happy to review officially if there's a valid-sounding story of how this could fix the test.

I don't know how this could cause a 1px rendering difference, but I can chime in on the timing: Chrome fires the canplay event fires at the beginning of playback, and as of last Friday Chrome fires the canplaythrough event when it believes it can play to the end of the video without stalling to buffer.

Previously, Chrome always fired canplay and canplaythrough at approx the same time, which goes against the html5 spec (crbug.com/73609). last Friday I checked in some code to fire canplaythrough at the media player's best guess.

So, I'm not sure why the rendering difference. It's true that canplay should be firing at the same time as always, and canplaythrough should be fired a bit later than before, but since this test uses a local video file, Chrome figures out very quickly that it can play through to the end without buffering and the canplaythrough event should be fired very close to the same time as it did in the past.
Comment 12 Adam Klein 2011-11-21 15:31:58 PST
Committed r100967: <http://trac.webkit.org/changeset/100967>