Closed Bug 472782 Opened 11 years ago Closed 11 years ago
Crash due to too much recursion involving <svg:text
Loading the testcase makes Firefox crash due to too much recursion: ... 412 nsSVGTextPathProperty::DoUpdate() + 198 (nsSVGEffects.cpp:250) 413 nsSVGRenderingObserver::InvalidateViaReferencedFrame() + 44 (nsSVGEffects.cpp:141) 414 nsSVGRenderingObserverList::InvalidateAll() + 109 (nsSVGEffects.cpp:406) 415 nsSVGEffects::InvalidateRenderingObservers(nsIFrame*) + 166 (nsSVGEffects.cpp:469) 416 nsSVGUtils::UpdateGraphic(nsISVGChildFrame*) + 42 (nsSVGUtils.cpp:606) 417 nsSVGGlyphFrame::SetGlyphPosition(float, float) + 56 (nsSVGGlyphFrame.cpp:828) 418 nsSVGTextFrame::UpdateGlyphPositioning(int) + 1296 (nsSVGTextFrame.cpp:450) 419 nsSVGTextFrame::NotifyGlyphMetricsChange() + 32 (nsSVGTextFrame.cpp:302) 420 nsSVGTextContainerFrame::NotifyGlyphMetricsChange() + 37 (nsSVGTextContainerFrame.cpp:60) 421 nsSVGTextPathProperty::DoUpdate() + 198 (nsSVGEffects.cpp:250) ...
FWIW the patch in bug 472135 would fix this too, although I suppose this patch is neater and more efficient.
I think we should just rely on 472135 to fix this. I don't think we should add code to speed up incorrect markup.
Depends on: 472135
I tried to rewrite this along the marker/filter approach and it just doesn't hang together. Unlike markers and filters, text locations are cached and we update them only when necessary via NotifyGlyphMetrics change. If I move to async painting then there is nothing to trigger UpdateEffects in the first place as the property is not referenced during painting, only when the text metrics i.e. the glyph locations change. So after much thought I still think the original patch is the right thing to do. You can't get reference loops in text unless you fail to check the referenced frame type as referenced frame must point to a path and a path can't contain text (or anything else for that matter).
Comment on attachment 356602 [details] [diff] [review] patch OK
Whiteboard: [needs landing]
Priority: -- → P3
Pushed http://hg.mozilla.org/mozilla-central/rev/42b8e2c66c5b. The test should be checked in as a crashtest, but I forgot to do that.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing] → [needs 191 landing]
verified FIXED on builds: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090415 Minefield/3.6a1pre ID:20090415030845 and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090416 Shiretoko/3.5b4pre ID:20090416030924
You need to log in before you can comment on or make changes to this bug.