784061 - Assertion failure: "Non-display SVG do not maintain visual overflow rects"

Status: RESOLVED WORKSFORME

(Core :: SVG, defect)

Description

[Jesse Ruderman]

Attachment: testcase

Assertion failure: !(mState & (nsFrameState(1) << (43))) || !(mState & (nsFrameState(1) << (22))) (Non-display SVG do not maintain visual overflow rects), at ../../../layout/generic/nsFrame.cpp:5088

This assertion was added recently:

changeset:   ad77846165e3
user:        Jonathan Watt
date:        Tue Aug 14 10:04:24 2012 +0100
summary:     Bug 780963 - Make UpdateOverflow() on filter primitive frames a no-op. r=roc.

Comment 1

[Daniel Veditz [:dveditz]]

Is this an assertion benign or something we need to worry about? The assertion is new in Firefox 17, although the issue might be old.

Comment 2

[Jonathan Watt [:jwatt]]

Doesn't seem to assert for me.

Comment 3

[Daniel Holbert [:dholbert]]

Does assert for me. (Linux x86_64 up-to-date debug build)

Comment 4

[Jonathan Watt [:jwatt]]

Could you attach a stack trace, Daniel?

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: backtrace.txt

Sure, here's the backtrace.

The frame in question (which doesn't satisfy the assertion) is a nsSVGPathGeometryFrame, with frame-tree ancestry as follows:

(gdb) p this
$4 = (nsSVGPathGeometryFrame * const) 0x7f7910740600
(gdb) p mParent
$5 = (nsSVGContainerFrame *) 0x7f7910740208
(gdb) p mParent->mParent
$6 = (nsSVGOuterSVGAnonChildFrame *) 0x7f791073fef0
(gdb) p mParent->mParent->mParent
$7 = (nsSVGOuterSVGFrame *) 0x7f791073fd80

Comment 6

[Daniel Holbert [:dholbert]]

This also isn't any sort of unlucky startup race-condition.  I tried performing the testcase's style-tweak manually (in the web developer console), and it triggers the same assertion-failure.

Comment 7

[Jonathan Watt [:jwatt]]

(In reply to Daniel Holbert [:dholbert] from comment #5)
> Sure, here's the backtrace.

Thanks!

(In reply to Daniel Veditz [:dveditz] from comment #1)
> Is this an assertion benign

It's not a security issue, but it is a perf issue to a certain extent. We end up calling UpdateOverflow() up the parent chain which is a complete waste of cycles for non-display SVG.

Comment 8

[Jonathan Watt [:jwatt]]

We should avoid scheduling nsChangeHint_UpdateOverflow restyles for such frames, or, if that's messy, return if we encounter such frames in nsCSSFrameConstructor::ProcessRestyledFrames around these two locations:

http://hg.mozilla.org/mozilla-central/annotate/e327e66a027d/layout/base/nsCSSFrameConstructor.cpp#l8126
http://hg.mozilla.org/mozilla-central/annotate/e327e66a027d/layout/base/nsCSSFrameConstructor.cpp#l8157

Comment 9

[Alex Keybl [:akeybl]]

Not a security issue, user impact hasn't yet been demonstrated. Please re-nominate if that changes in the future.

Comment 10

[Robert Longson [:longsonr]]

Jonathan, are you working on this. If not, I can do something along the lines of comment 8.

Comment 11

[Robert Longson [:longsonr]]

Taking per irc.

Comment 12

[Robert Longson [:longsonr]]

Doesn't assert any more.

Comment 13

[Daniel Holbert [:dholbert]]

We should probably check in a crashtest, to be sure it's not just fixed on your configuration and that we don't later regress this.

Comment 14

[Daniel Holbert [:dholbert]]

Attachment: crashtest patch

This patch adds the testcase (w/ reftest-wait) as a crashtest.

Comment 15

[Daniel Holbert [:dholbert]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6d1337d2db17

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6d1337d2db17

Comment 17

[Jesse Ruderman]

This assertion was also mentioned in bug 795592, so maybe this was a dup.