Intermittent failure in reftests/svg/as-image/img-anim-1.html

RESOLVED FIXED in mozilla6

Status

()

Core
SVG
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: philor, Assigned: dholbert)

Tracking

({intermittent-failure})

Trunk
mozilla6
intermittent-failure
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

7 years ago
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1289800534.1289801192.813.gzRev3 Fedora 12x64 mozilla-central opt test reftest on 2010/11/14 21:55:34
s: talos-r3-fed64-005

REFTEST TEST-UNEXPECTED-FAIL | file:///home/cltbld/talos-slave/mozilla-central_fedora64_test-reftest/build/reftest/tests/layout/reftests/svg/as-image/img-anim-1.html |
(Reporter)

Comment 1

7 years ago
Created attachment 490518 [details]
reftest log
(Assignee)

Comment 2

7 years ago
So in the reftest log, the testcase is completely blank (and the reference is a green square).

FWIW, the testcase source is:
http://mxr.mozilla.org/mozilla-central/source/layout/reftests/svg/as-image/img-anim-1.html?force=1

It tries to check if animations are running in our animated SVG image like so:
 - catch the first paint (which might be before animations are applied)
 - explicitly request an animation sample (mozRequestAnimationFrame)
 - let the paint after that be the reftest-snapshot

I'm not immediately sure which part is failing here.
Comment hidden (Treeherder Robot)
(Assignee)

Comment 4

7 years ago
FWIW, I tweaked the SVG image used in this test, in a changeset landed earlier today, to have a red rect that (when animations run correctly) gets covered up by a green rect.

Because of that change, the sporadic-failure here (based on comment 3's log) now shows a red rect, instead of just blankness.

That indicates that the image is loading & painting correctly -- we're just not getting an animated sample painted for some reason.
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 31

7 years ago
(In reply to comment #2)
> It tries to check if animations are running in our animated SVG image like so:
>  - catch the first paint (which might be before animations are applied)
>  - explicitly request an animation sample (mozRequestAnimationFrame)
>  - let the paint after that be the reftest-snapshot

Ah -- ok, I just realized what's going wrong in the steps above. The animated image in the testcase has its *own* nsRefreshDriver -- so, while the testcase's call to mozRequestAnimationFrame() will trigger an animation sample in the *host* document, we still won't necessarily end up getting an animation sample in the *guest* document. (inside the image)

I guess this means we need to just fall back on a setTimeout, to give the image time to load & paint its first animation frame.  We have to rely on that for a few other SMIL tests -- I was hoping this testcase's strategy was a new way around that, but I guess not (at least, not when there's an animated image in the test).
(Assignee)

Updated

7 years ago
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
OS: Linux → All
Hardware: x86_64 → All
(Assignee)

Comment 32

7 years ago
Seems odd that the failures here have all been on Linux -- I don't know why that would be.
OS: All → Linux
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 52

7 years ago
Disabled the test on Linux:
  http://hg.mozilla.org/mozilla-central/rev/f5cf523d8bd2

I still don't know why this specifically would fail on Linux & not other platforms (see comment 31 & comment 32)... until it's noticed failing elsewhere, I tend to want to leave the test itself as-is, since I don't know what the fedora-specific aspect of the failure is.
Whiteboard: [orange] → [orange][test disabled on linux]
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 93

6 years ago
I think the right fix for this might be to convert this test into a chrome test and use an animation listener, as bz suggested (for a different animated-image testcase) in bug 641198 comment 24.
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(In reply to comment #31)
> Ah -- ok, I just realized what's going wrong in the steps above. The animated
> image in the testcase has its *own* nsRefreshDriver -- so, while the testcase's
> call to mozRequestAnimationFrame() will trigger an animation sample in the
> *host* document, we still won't necessarily end up getting an animation sample
> in the *guest* document. (inside the image)
> 
> I guess this means we need to just fall back on a setTimeout, to give the image
> time to load & paint its first animation frame.  We have to rely on that for a
> few other SMIL tests -- I was hoping this testcase's strategy was a new way
> around that, but I guess not (at least, not when there's an animated image in
> the test).

We shouldn't use setTimeout here. I think we should probably rewrite the test to be a (chrome) mochitest using windowSnapshot.js and poll until the testcase matches the reference we want. Make sense?
(Assignee)

Comment 110

6 years ago
Yup, sounds like a good idea.  (That's roughly what I was hinting at in comment 93, though I was much more hand-wavy due to unfamiliarity with chrome mochitests. :))
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Duplicate of this bug: 651085
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Created attachment 528145 [details] [diff] [review]
Set the testcase to random
Attachment #528145 - Flags: review?(dholbert)
This has been intermittent for a while now and shows no sign of being fixed. We should just set it to random.
(Assignee)

Comment 129

6 years ago
Comment on attachment 528145 [details] [diff] [review]
Set the testcase to random

Coincidentally, I actually *just* finished rewriting this test, and I'm going to be landing the fixed version soon as part of bug 649440.  (The correct rewrite of this test depends on that bug, and it also acts as a method of testing that bug.)  The rewrite is along the lines of comment 109 / comment 110.

Sorry for the randomorange noise and for the delayed fix here -- I intend to have this really-truly-fixed today, so r-minusing this random marking, since I'd just be reverting it soon anyway.  (or rather, deleting the line entirely, since the new test will be a chrome mochitest rather than a reftest)
Attachment #528145 - Flags: review?(dholbert) → review-
Sounds great to me.
(Assignee)

Updated

6 years ago
Depends on: 649440
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 133

6 years ago
Woot, this is now FIXED by bug 649440, which took this test and reshaped it into a chrome mochitest ( modules/libpr0n/test/mochitest/test_animSVGImage.html ), as described in comment 109 / comment 110.

So, there will definitely no longer be any randomorange in img-anim-1.html because there is no longer any test by that name on mozilla-central. :)  (In the unlikely event that the test's new incarnation starts failing, we can file a new randomorange for that.)
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
OS: Linux → All
Resolution: --- → FIXED
Whiteboard: [orange][test disabled on linux] → [orange]
Target Milestone: --- → mozilla6
It might be worth asking dbaron about adding SpecialPowers to reftest if you are going to have a whole class of tests that need things like this, instead of having to make them all chrome tests.
(In reply to comment #134)
> It might be worth asking dbaron about adding SpecialPowers to reftest if you
> are going to have a whole class of tests that need things like this, instead
> of having to make them all chrome tests.

Filed bug 655622.
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Keywords: intermittent-failure
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.