Closed
Bug 571036
Opened 15 years ago
Closed 15 years ago
Intermittent failure in layout/reftests/svg/dynamic-use-nested-01.svg
Categories
(Core :: SVG, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: ehsan.akhgari, 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>
Reporter | ||
Comment 1•15 years ago
|
||
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1276101378.1276104428.19833.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1276100814.1276102536.10393.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1276099569.1276101960.7521.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1276100445.1276103502.15532.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1276098694.1276101917.7268.gz
Reporter | ||
Updated•15 years ago
|
OS: Mac OS X → All
Comment 2•15 years ago
|
||
That checkin doesn't change any code. It makes the configuration file for qsgen.py easier to read but the generated code is identical.
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 5•15 years ago
|
||
(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).
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 8•15 years ago
|
||
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+
Updated•15 years ago
|
Attachment #450199 -
Attachment description: patch? → patch? (add reftest-wait to test)
Comment 9•15 years ago
|
||
It's getting late for me. Would you mind checking it in?
Keywords: checkin-needed
Comment 10•15 years ago
|
||
Sure, I'll land it in a few minutes. I can watch the tree for the other stuff you landed recently, too.
Comment 11•15 years ago
|
||
Thanks Daniel.
Comment 12•15 years ago
|
||
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
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Reporter | ||
Comment 15•15 years ago
|
||
(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.)
Comment 16•15 years ago
|
||
Why does this test need reftest-wait? Is it because <svg load> is treated differently from <body load> and window.addEventListener('load')?
Comment 17•15 years ago
|
||
bug 552938 makes it occur async I think and therefore potentially later.
Comment 18•15 years ago
|
||
I don't think the reftest-wait helped.
Comment 19•15 years ago
|
||
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.
Comment 20•15 years ago
|
||
(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 21•15 years ago
|
||
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
Comment 22•15 years ago
|
||
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
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 27•15 years ago
|
||
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.
Comment 28•15 years ago
|
||
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.
Comment 29•15 years ago
|
||
(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)
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 32•15 years ago
|
||
Just backed out bug 552938 (per bug 552938 comment 20), so this should be fixed in tinderbox cycles after the backout.
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 34•15 years ago
|
||
No instances of this randomorange during the 9 tinderbox-cycles that have happened since the backout. Calling this fixed.
Updated•15 years ago
|
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•12 years ago
|
Keywords: intermittent-failure
Assignee | ||
Updated•12 years ago
|
Whiteboard: [orange]
You need to log in
before you can comment on or make changes to this bug.
Description
•