Bug 78060 – [chromium] Add a layout test for losing the compositor context with a video playing

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

[James Robinson]

[chromium] Add a layout test for losing the compositor context with a video playing

[James Robinson]

Created attachment 125960 [details]
Patch

[Ami Fischman]

Comment on attachment 125960 [details]
Patch

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

lgtm

> LayoutTests/platform/chromium/compositing/lost-compositor-context-with-video.html:23
> +function canplaythrough() {

Drop?

> LayoutTests/platform/chromium/compositing/lost-compositor-context-with-video.html:27
> +<video oncanplaythrough="test();"></video>

Heads-up that chromium has a bug where it fires canplaythrough immediately; since you only seek to 0 that should be fine.

[James Robinson]

does "immediately" mean that I can definitely show the first frame?

[Ami Fischman]

FTR the fire-canplaythrough-immediately crbug is http://code.google.com/p/chromium/issues/detail?id=73609

Since you're loading from file:// (as opposed to http://) I think you should be fine.

[Eric Carlson]

Comment on attachment 125960 [details]
Patch

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

> LayoutTests/platform/chromium/compositing/lost-compositor-context-with-video.html:7
> +    layoutTestController.dumpAsText(true);  // This is only useful as a pixel test.

Why call dumpAsText() at all - there is no text output? I would either remove this call completely so it doesn't generate an "expected" text file, or include some text in the test so the text file say something about why it is empty.

>> LayoutTests/platform/chromium/compositing/lost-compositor-context-with-video.html:27
>> +<video oncanplaythrough="test();"></video>
> 
> Heads-up that chromium has a bug where it fires canplaythrough immediately; since you only seek to 0 that should be fine.

Can 'canplaythrough' fire when readyState is HAVE_NOTHING? If so, this will fail because setting currentTime will throw an exception.

[Ami Fischman]

(In reply to comment #5)
> >> LayoutTests/platform/chromium/compositing/lost-compositor-context-with-video.html:27
> >> +<video oncanplaythrough="test();"></video>
> > 
> > Heads-up that chromium has a bug where it fires canplaythrough immediately; since you only seek to 0 that should be fine.
> 
> Can 'canplaythrough' fire when readyState is HAVE_NOTHING? If so, this will fail because setting currentTime will throw an exception.

My understanding is that 'canplaythrough' is fired by webkit when chromium's webmediaplayer moves readyState to HAVE_ENOUGH_DATA, so this isn't an issue:
http://code.google.com/codesearch#OAMlx_jo-ck/src/webkit/media/webmediaplayer_impl.cc&exact_package=chromium&q=havenothing%20-file:third_party%20file:%5C.cc&type=cs&l=700

[James Robinson]

(In reply to comment #5)
> (From update of attachment 125960 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=125960&action=review
> 
> > LayoutTests/platform/chromium/compositing/lost-compositor-context-with-video.html:7
> > +    layoutTestController.dumpAsText(true);  // This is only useful as a pixel test.
> 
> Why call dumpAsText() at all - there is no text output? I would either remove this call completely so it doesn't generate an "expected" text file, or include some text in the test so the text file say something about why it is empty.
> 

Without the dumpAsText() DRT will produce a render tree dump, which isn't helpful for this test.

[James Robinson]

Committed r107132: <http://trac.webkit.org/changeset/107132>