Closed Bug 571036 Opened 10 years ago Closed 10 years ago

Intermittent failure in layout/reftests/svg/dynamic-use-nested-01.svg

Categories

(Core :: SVG, defect)

x86
All
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: ehsan, Unassigned)

References

()

Details

(Keywords: intermittent-failure)

Attachments

(1 obsolete file)

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1276102740.1276105887.26795.gz
Linux mozilla-central debug test reftest

This started since peterv's checkin: <http://hg.mozilla.org/mozilla-central/rev/593d13102b68>
OS: Mac OS X → All
That checkin doesn't change any code. It makes the configuration file for qsgen.py easier to read but the generated code is identical.
(In reply to comment #2)
> That checkin doesn't change any code.

This also could be a regression from bug 552938, a few pushes before peterv's.  That checkin seems to have caused regressions in another SVG-related test (bug 571016).
Worth a shot Daniel?
Attachment #450199 - Flags: review?(dholbert)
Comment on attachment 450199 [details] [diff] [review]
patch? (add reftest-wait to test)

Indeed -- I was actually just posting a comment saying "looks like this test needs reftest-wait", and got an in-flight collision with your patch posting. :)
Attachment #450199 - Flags: review?(dholbert) → review+
Attachment #450199 - Attachment description: patch? → patch? (add reftest-wait to test)
It's getting late for me. Would you mind checking it in?
Keywords: checkin-needed
Sure, I'll land it in a few minutes.  I can watch the tree for the other stuff you landed recently, too.
Thanks Daniel.
No problem - thanks for the quick fix!

Pushed longsonr's patch (with a few added whitespace-at-end-of-line fixes to the test file, as long as we're already modifying it):
  http://hg.mozilla.org/mozilla-central/rev/5e417ae6c103

I'm leaving this bug open until we're sure it's fixed, so that people can still easily find it to report any more oranges of this type.
Keywords: checkin-needed
(In reply to comment #12)
> No problem - thanks for the quick fix!
> 
> Pushed longsonr's patch (with a few added whitespace-at-end-of-line fixes to
> the test file, as long as we're already modifying it):
>   http://hg.mozilla.org/mozilla-central/rev/5e417ae6c103
> 
> I'm leaving this bug open until we're sure it's fixed, so that people can still
> easily find it to report any more oranges of this type.

FWIW, you could always mark a bug as FIXED, and it will show up crossed-out in tbpl, so people can actually note that and reopen if needed (although tbplbot doesn't reopen automatically.)
Why does this test need reftest-wait?  Is it because <svg load> is treated differently from <body load> and window.addEventListener('load')?
bug 552938 makes it occur async I think and therefore potentially later.
I don't think the reftest-wait helped.
Yeah, I was afraid it might not help -- I just figured it might, since the reftest screenshot looks like it could fall into the category of "oops we took the snapshot before the scripted update finished".

> I don't think the reftest-wait helped.

Indeed, the reftest failed on the push that included this change.  I'll undo the reftest-wait addition.
(In reply to comment #19)
> reftest screenshot looks like it could fall into the category of "oops we took
> the snapshot before the scripted update finished".

To be clear, that's not the issue -- I actually see this failure when loading the reftest directly in today's nightly, though it's infrequent (maybe 1 in every 10-15 reloads has the problem).  (The problem, when it happens, is that the red stripe is only 20% of the height of the page, instead of 100%)
Comment on attachment 450199 [details] [diff] [review]
patch? (add reftest-wait to test)

Removed reftest-wait, and removed an unnecessary full-page white background-rect in the test.
http://hg.mozilla.org/mozilla-central/rev/8f2aa769d221
Attachment #450199 - Attachment is obsolete: true
So this was the testcase from bug 467498, which was never fixed but rather became WORKSFORME, and has had its testcase in reftests for two weeks now. And then magically this morning it started sporadically failing again, with the second-level-of-<use> not getting updated sometimes.

Looking at the recent pushes before this started failing, the only ones that even slightly stand out as potential causes are:
 - bug 570837 - If we use something in nsIFrame::mState to know knowing that the red frame needs to be updated (not sure if we do), then maybe this checkin caused or exposed a bug with keeping track of a particular flag in mState?
 - bug 552908, only because it's SVG related (but doesn't really have to do with "liveness" of the document -- just the firing of SVGLoad)
Depends on: 467498
Ok, I figured this out:
 * As longsonr said in comment 17, bug 552938 makes our onload handler get fired asynchronously.
 * This gives us a chance to reflow the page entirely before the onload handler fires (I think)
 * In that case -- where the onload handler fires after we've reflowed the page -- we still fail on bug 467498 (the bug this reftest is testing).

I just described this in bug Bug 467498 comment 9 & 10, too.  I'm going to mark the reftest as random for now.
actually, scratch that -- I'm going to back out bug 552908 instead, since its async-onload-firing seems to have caused both this regression and bug 571016.

Assuming that re-lands with the SVGLoad event being fired synchronously, this testcase shouldn't need to be marked random, I think.
(In reply to comment #28)
> actually, scratch that -- I'm going to back out bug 552908 instead

er, sorry, I meant s/bug 552908/bug 552938/ there (and in comment 22)

(bug 552908 has nothing to do with this bug)
Just backed out bug 552938 (per bug 552938 comment 20), so this should be fixed in tinderbox cycles after the backout.
No instances of this randomorange during the 9 tinderbox-cycles that have happened since the backout.  Calling this fixed.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.