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.

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

Pushed http://hg.mozilla.org/mozilla-central/rev/42b8e2c66c5b. The test should be checked in as a crashtest, but I forgot to do that.

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