524006 - toolkit/content/tests/widgets/test_videocontrols_audio_direction.html fails on Windows debug builds

Status: RESOLVED FIXED

(Toolkit :: Video/Audio Controls, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

If you look at the Windows debug unittest 5/5 on http://tinderbox.mozilla.org/showbuilds.cgi?tree=Firefox-Unittest&noignore=1 it appears that this failure:

14549 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_videocontrols_audio_direction.html | Rendering of reftest videocontrols_direction-2a.html should not be different to the reference

happens every time.  It seems ok on other platforms, and on non-debug.

In particular, look for the box "WINNT 5.2 mozilla-central test debug mochitests-5/5".

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I can't reproduce this failure locally (running just toolkit/content/tests/widgets/).

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The difference in the snapshots (from the logs) looks like the part of the UI that shows how much of the audio has loaded.  (At least I think that's what that UI is showing.)

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: snapshots of failure

Comment 4

[Jeff Walden [:Waldo]]

So, looking at the test, it's snapshotting when it gets a load event for an iframe.  Video elements don't hold up load any more, so this looks entirely random based on how much of the video has been buffered by the time the load event fires.  It looks to me like toolkit/content/tests/widgets/videocontrols_direction_test.js needs some rejiggering to cooperatively delay for embedded video element buffering.

Comment 5

[Matthew Gregan [:kinetik]]

They do hold up the document load until we reach the first decoded frame, but they no longer fire the load event on the media element.  I don't think the test relies on that, so that's not the problem.

None of the media elements used in the test are autobuffer, so the load is going to be suspended by the media cache sometime after the first frame is decoded.  Because the test files are so small, we'll usually have loaded the entire file before this happens, but it's not guaranteed.

Comment 6

[Matthew Gregan [:kinetik]]

You're right that it needs to wait for more than the document load event, though, because even with autobuffer the file might not finish loading until after we've unblocked the document load event.

Comment 7

[Justin Dolske [:Dolske]]

Ah, indeed, that would cause the problem.

Since this is effectively a reftest, it needs to wait until the load actually finishes so that the progress bars look the same. I think that will now require listening to progress events, and waiting until .loaded == .total (it's a small clip, so that should be fine w.r.t. the media cache). And they'd also need the autobuffer attribute set.

Comment 8

[cajbir (:cajbir)]

I believe the progress events are no longer actual progress events in the spec, so .loaded and .total will not exist. 

We haven't made this change yet but it would pay not to have tests rely on something that is going away.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

When they go away, controls will have to be modified to use 'buffered' instead, so I think it's reasonable to expect to modify these tests then too, and use .loaded and .total for now.

Comment 10

[Justin Dolske [:Dolske]]

AIUI, this is one of the tests that's holding up switching over to the split mochitests, so we should fix this ASAP. Ehsan, can you pick this up?

Comment 11

[(not currently active) Ted Mielczarek]

It's just holding up unhiding that part of the Windows debug Mochitests.

Comment 12

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

(In reply to comment #11)
> It's just holding up unhiding that part of the Windows debug Mochitests.

Any ETA on this?

We're having to run unittests both ways until this is fixed, and the extra load is hurting wait times.

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Seems to have gone away on its own, at least for now, but it would still be really good to make this test wait for what it ought to be waiting for.

Comment 14

[(no longer active)]

Attachment: Patch (v1) [Checkin: Comment 16 & 17]

This should take care of the failure.

I've also submitted a try server build...

Comment 15

[Justin Dolske [:Dolske]]

Comment on attachment 411298 [details] [diff] [review]
Patch (v1)
[Checkin: Comment 16 & 17]

r=me, land away assuming it doesn't explode on try (doesn't need further review).

Comment 16

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/25138a08de15

I'll land it on branch after a green cycle on trunk.

Comment 17

[(no longer active)]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/ae99530573a9

Comment 18

[(no longer active)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/645622c570fb

Comment 19

[Jim Mathies [:jimm]]

(In reply to comment #18)
> http://hg.mozilla.org/releases/mozilla-1.9.1/rev/645622c570fb


Firefox3.5-Unittest went orange once this landed -

85931 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_videocontrols_audio_direction.html | Test timed out.
85934 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_videocontrols_video_direction.html | Test timed out.

Comment 20

[Serge Gautherie (:sgautherie)]

(In reply to comment #18)
> http://hg.mozilla.org/releases/mozilla-1.9.1/rev/645622c570fb

Broke SeaMonkey too:
{
28659 ERROR TEST-UNEXPECTED-FAIL | /tests/content/xml/document/test/test_bug293347.html | doc still intact - got 0, expected 2
28660 ERROR TEST-UNEXPECTED-FAIL | /tests/content/xml/document/test/test_bug293347.html | doc body still intact - got null, expected "http://www.w3.org/1999/xhtml"
buildbot.slave.commands.TimeoutError: command timed out: 300 seconds without output, killing pid 17359
}

Please, back out.!.

Comment 21

[(no longer active)]

Backed out: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d89ca352b319.

Comment 22

[(not currently active) Ted Mielczarek]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1258020412.1258021384.16867.gz
WINNT 5.2 mozilla-central debug test mochitests-5/5 on 2009/11/12 02:06:52  
"s: moz2-win32-slave22"

Comment 23

[(no longer active)]

Attachment: Patch (v2) [Checkin: Comment 27 & 28]

Hmm, I'm really out of ideas here.  The buffer bar value is set by the progress handler, and after the final progress event, a suspend event is fired <http://mxr.mozilla.org/mozilla-central/source/content/media/ogg/nsOggDecoder.cpp#2225>.  The only other case in which a suspend event is fired is when nsHTMLMediaElement::SuspendDownload is called.  So, this check should handle that case as well.  I've ran this test quite a few times locally and it passes every time.  I think this patch is also worth a shot.

Comment 24

[Phil Ringnalda (:philor)]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1258097623.1258098384.17798.gz#err0
WINNT 5.2 mozilla-central debug test mochitests-5/5 on 2009/11/12 23:33:43

Comment 25

[(not currently active) Ted Mielczarek]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1258127273.1258128330.10805.gz
WINNT 5.2 mozilla-central debug test mochitests-5/5 on 2009/11/13 07:47:53  
"s: moz2-win32-slave14"

Comment 26

[(not currently active) Ted Mielczarek]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1258130599.1258131421.13534.gz
WINNT 5.2 mozilla-central debug test mochitests-5/5 on 2009/11/13 08:43:19  
"s: moz2-win32-slave27"

Comment 27

[(no longer active)]

Comment on attachment 411978 [details] [diff] [review]
Patch (v2)
[Checkin: Comment 27 & 28]

Pushed: http://hg.mozilla.org/mozilla-central/rev/3dcf63a4435c

Comment 28

[(no longer active)]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/6c6f2621732b

Comment 29

[(no longer active)]

I think this can be marked as FIXED now.  The only remaining known issue with this test is bug 528001.