786331 - WebGL tests that use video can loop indefinitely

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Paul Adenot (:padenot)]

In the webgl conformance tests that use video elements (example: [1]), the tests are ran on "playing", and the video has a loop attribute. In that case, "playing" is fired every time we loop. Also, when the video is paused because it is buffering (which happens a lot even in local on try), it also fires a "playing" event.

That means the tests can be run multiple time (with multiple value being an high enough number to cause a timeout in the tests harness), and run an infinite number of time when ran outside the harness (making debugging rather painful).

An easy fix would be to set the |video.src| to null when we passed the tests, to ensure that "playing" will not fire.

This has been triggered by upcoming changes to the media element, and causes intermittent failures.

[1]: http://mxr.mozilla.org/mozilla-central/source/content/canvas/test/webgl/conformance/textures/tex-image-and-sub-image-2d-with-video.html?force=1

Comment 1

[Benoit Jacob [:bjacob] (mostly away)]

Thanks a lot! We should fix the tests and upstream the fix. Are you writing the patch for our copy of the tests? I have no experience whatsoever coding for the video element.

Comment 2

[Paul Adenot (:padenot)]

I have a patch sitting somewhere. I'll make sure it works properly (and fixes all the occurrences of this pattern) and attach it here.

Comment 3

[Paul Adenot (:padenot)]

Attachment: WebGL tests that use video can loop indefinitely. r=

This patch fixes the two occurences I found.

It's green on try: https://tbpl.mozilla.org/?tree=Try&rev=5d9493c46563.

Comment 4

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 656293 [details] [diff] [review]
WebGL tests that use video can loop indefinitely. r=

Review of attachment 656293 [details] [diff] [review]:
-----------------------------------------------------------------

Wouldn't it make more sense to remove the event listener?

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

Btw, sorry for being so nitpicky. The reason why I am is we're going to try to upstream it.

Comment 6

[Paul Adenot (:padenot)]

Attachment: WebGL tests that use video can loop indefinitely. r=

This seems indeed cleaner.

Comment 7

[Paul Adenot (:padenot)]

Attachment: WebGL tests that use video can loop indefinitely. r=

Even better with a semicolon at the end.

Comment 8

[Paul Adenot (:padenot)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b70f1224a9e8

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/b70f1224a9e8

Comment 10

[Benoit Jacob [:bjacob] (mostly away)]

Merged upstream (a slightly different patch as it had changed):
https://github.com/KhronosGroup/WebGL/pull/98