Closed Bug 375775 Opened 14 years ago Closed 13 years ago

Filter crash [@ nsSVGUtils::PaintChildWithEffects] with display:none

Categories

(Core :: SVG, defect)

x86
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: jruderman, Assigned: tor)

References

(Blocks 1 open bug)

Details

(Keywords: crash, testcase, Whiteboard: [sg:critical?] post-1.8-branch)

Crash Data

Attachments

(3 files)

Crashes [@ nsSVGUtils::PaintChildWithEffects] dereferencing 0xdddddde9.
Whiteboard: [sg:critical?]
Whiteboard: [sg:critical?] → [sg:critical?] post-1.8-branch
What appears to be happening is that display:none is causing the filter frame to go away, not something that we watch for.  One way of handling this would be the nsSVGFilterFrame supporting weak references and then using a weakptr to track the frame.  Are there more elegant ways of doing what we need in gecko?

Likely there are similar problems for other internally referenced objects (paint servers, gradients, markers, clipPaths).
You can use an nsWeakFrame if you really need to, but it's expensive-ish.

That said, why are you holding pointers to frames at all?  Who's holding the pointer?  Why aren't they getting the frame as needed?  In general, frames can die at the drop of a hat, so holding pointers to them is very dangerous.
We tend to store frame pointers in the properties of the frames they're referenced from, to avoid the lookup hit when painting.
Is the hit actually noticeable?  GetPrimaryFrameFor is just a hashtable lookup after the first time...
Flags: wanted1.8.1.x-
Flags: wanted1.8.0.x-
We need to consider what the appropriate behavior should be when a filter has display:none.

Option 1: Shouldn't display the filtered geometry - current behavior of the patch.

Option 2: Should display filtered geometry unfiltered.  This introduces a complication that we need to invalidate the union of the filter invalidation area and the geometry area when the filter frame comes/goes, as the filter region could be a subset of the geometry covered by the filter.  How do we find out when this happens?
Can you raise it with the SVG WG? It's definitely a spec issue.
Or:

Option 3: we should display the filtered geometry since the filter element isn't shown directly, so display:none on it shouldn't matter.
(In reply to comment #8)
> Option 3: we should display the filtered geometry since the filter element
> isn't shown directly, so display:none on it shouldn't matter.

That one is tricky since we need the frames for style information on a few filter elements, and display:none stops the frame generation.

Oh sure, #3 would involved the most work by far since it doesn't fit into how things work now in Mozilla, but from a spec point of view it's probably how things should work I'd think.
Actually it looks like #3 is what the spec requires: "'filter' elements are never rendered directly; their only usage is as something that can be referenced using the 'filter' property. The 'display' property does not apply to the 'filter' element; thus, 'filter' elements are not directly rendered even if the 'display' property is set to a value other than none, and 'filter' elements are available for referencing even when the 'display' property on the 'filter' element or any of its ancestors is set to none.".

Similar language is in the other items which have references to them (gradients, patterns, clipPaths, and masks).
The elements in question could probably store references to their style contexts...  The problems come with knowing when to restyle them, since we don't restyle display:none subtrees, since CSS has this little invariant that display:none stuff never affects the rendering of the document...
Wouldn't this involve modifying all SVG elements to store their style information, since any content can be the child of <marker>, <pattern>, <clipPath>, and <mask>?
Oh, you need info about the kids, not just the node itself?  Yeah, indeed.
You can get the style for an element the way nsComputedDOMStyle does, in the case where there's no frame. It'd be slower but this is a corner case.

http://lxr.mozilla.org/seamonkey/source/layout/style/nsComputedDOMStyle.cpp#314
Moved discussion of general display:none handling in SVG to bug 376027.
Attachment #260070 - Flags: review?(roc)
Summary: Filter crash [@ nsSVGUtils::PaintChildWithEffects] → Filter crash [@ nsSVGUtils::PaintChildWithEffects] with display:none
The patch here no longer applies :(
Assignee: general → tor
Status: NEW → ASSIGNED
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Group: security
Crashtest checked in.
Flags: in-testsuite+
Crash Signature: [@ nsSVGUtils::PaintChildWithEffects]
You need to log in before you can comment on or make changes to this bug.