See testcase, which crashes current trunk build on load. This regressed between 2008-10-10 and 2008-10-11: http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2008-10-10+04%3A00%3A00&enddate=2008-10-11+05%3A00%3A00 I think a regression from bug 309220. http://crash-stats.mozilla.com/report/index/e82a59d9-a628-4b55-b19d-6e1b02090105?p=1 0 mozcrt19.dll _ctrandisp2 1 xul.dll nsSVGLineElement::GetMarkPoints content/svg/content/src/nsSVGLineElement.cpp:180 2 xul.dll nsSVGPathGeometryFrame::GetCoveredRegion layout/svg/base/src/nsSVGPathGeometryFrame.cpp:213 3 xul.dll nsSVGPathGeometryFrame::UpdateCoveredRegion layout/svg/base/src/nsSVGPathGeometryFrame.cpp:275 4 xul.dll nsSVGOuterSVGFrame::UpdateAndInvalidateCoveredRegion layout/svg/base/src/nsSVGOuterSVGFrame.cpp:658 5 xul.dll nsSVGMarkerProperty::DoUpdate layout/svg/base/src/nsSVGEffects.cpp:229 6 xul.dll nsSVGRenderingObserver::InvalidateViaReferencedFrame layout/svg/base/src/nsSVGEffects.cpp:140 7 xul.dll nsSVGRenderingObserverList::InvalidateAll layout/svg/base/src/nsSVGEffects.cpp:407 8 xul.dll nsSVGEffects::InvalidateRenderingObservers layout/svg/base/src/nsSVGEffects.cpp:468 9 xul.dll nsSVGRenderingObserver::DoUpdate layout/svg/base/src/nsSVGEffects.cpp:129 10 xul.dll nsSVGMarkerProperty::DoUpdate layout/svg/base/src/nsSVGEffects.cpp:220 11 xul.dll nsSVGRenderingObserver::InvalidateViaReferencedFrame layout/svg/base/src/nsSVGEffects.cpp:140 12 xul.dll nsSVGRenderingObserverList::InvalidateAll layout/svg/base/src/nsSVGEffects.cpp:407 13 xul.dll nsSVGEffects::InvalidateRenderingObservers layout/svg/base/src/nsSVGEffects.cpp:468 14 xul.dll nsSVGRenderingObserver::DoUpdate layout/svg/base/src/nsSVGEffects.cpp:129 15 xul.dll nsSVGMarkerProperty::DoUpdate layout/svg/base/src/nsSVGEffects.cpp:220 16 xul.dll nsSVGRenderingObserver::InvalidateViaReferencedFrame layout/svg/base/src/nsSVGEffects.cpp:140 17 xul.dll nsSVGRenderingObserverList::InvalidateAll layout/svg/base/src/nsSVGEffects.cpp:407 18 xul.dll nsSVGEffects::InvalidateRenderingObservers layout/svg/base/src/nsSVGEffects.cpp:468 19 xul.dll nsSVGRenderingObserver::DoUpdate layout/svg/base/src/nsSVGEffects.cpp:129 20 xul.dll nsSVGMarkerProperty::DoUpdate layout/svg/base/src/nsSVGEffects.cpp:220 etc..

I don't think this is the correct fix. I think it's vulnerable to loops created with extra levels of indirection. What's *supposed* to prevent this loop is that nsSVGRenderingObserver::InvalidateViaReferencedFrame is supposed to clear mReferencedFrame, which should be preventing another call to InvalidateViaReferencedFrame on this observer from doing anything. I'm not sure why that's not working here.

This is a similar testcase, but crashes in a different way: http://crash-stats.mozilla.com/report/index/736bf030-e298-4dc4-998c-779192090111?p=1 0 xul.dll _cairo_spline_error_squared gfx/cairo/cairo/src/cairo-spline.c:198 1 xul.dll _cairo_spline_decompose_into gfx/cairo/cairo/src/cairo-spline.c:252 2 xul.dll _cairo_spline_decompose_into gfx/cairo/cairo/src/cairo-spline.c:257 3 xul.dll _cairo_spline_decompose_into gfx/cairo/cairo/src/cairo-spline.c:257 4 xul.dll _cairo_spline_decompose_into gfx/cairo/cairo/src/cairo-spline.c:257 5 xul.dll _cairo_spline_decompose gfx/cairo/cairo/src/cairo-spline.c:278 6 xul.dll _cpf_curve_to gfx/cairo/cairo/src/cairo-path-fixed.c:739 7 xul.dll _cairo_path_fixed_interpret gfx/cairo/cairo/src/cairo-path-fixed.c:524 8 xul.dll _cairo_path_fixed_interpret_flat gfx/cairo/cairo/src/cairo-path-fixed.c:788 9 xul.dll _cairo_path_fixed_bounds gfx/cairo/cairo/src/cairo-path-bounds.c:157 10 xul.dll _cairo_gstate_path_extents gfx/cairo/cairo/src/cairo-gstate.c:789 11 xul.dll _moz_cairo_path_extents gfx/cairo/cairo/src/cairo.c:1917 12 xul.dll gfxContext::GetUserPathExtent gfx/thebes/src/gfxContext.cpp:767 13 xul.dll nsSVGPathGeometryFrame::UpdateCoveredRegion layout/svg/base/src/nsSVGPathGeometryFrame.cpp:260 14 xul.dll nsSVGOuterSVGFrame::UpdateAndInvalidateCoveredRegion layout/svg/base/src/nsSVGOuterSVGFrame.cpp:658 15 xul.dll nsSVGMarkerProperty::DoUpdate layout/svg/base/src/nsSVGEffects.cpp:229 etc.. This looks a bit like the crashes in bug 435756. Is this perhaps a bug in cairo?

I don't think it's a cairo bug its gecko not detecting the recursion. The marked element is an ancestor of the marker.

(In reply to comment #2) > > What's *supposed* to prevent this loop is that > nsSVGRenderingObserver::InvalidateViaReferencedFrame is supposed to clear > mReferencedFrame, which should be preventing another call to > InvalidateViaReferencedFrame on this observer from doing anything. I'm not sure > why that's not working here. nsSVGRenderingObserver::InvalidateViaReferencedFrame calls DoUpdate which ends up in nsSVGMarkerProperty::DoUpdate. This calls UpdateAndInvalidateCoveredRegion which calls UpdateCoveredRegion and this puts back the referenced frame pointer in order to determine the covered region as that is affected by the presence of markers. The new patch doesn't crash but it does suffer from an asynchronous repaint loop so the CPU runs at 100%. I don't know how to fix that, suggestions appreciated.

I guess we're calling nsSVGMarkerProperty::DoUpdate during painting somehow? You'd need to find the call stack for that and figure out how to avoid calling it if things are already up to date.

Pushed http://hg.mozilla.org/mozilla-central/rev/9246fefef1cc Need a crashtest here

Backed out. http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1232184917.1232189138.19404.gz#err1 had two failures. One is an unexpected-pass on dynamic-filter-contents-01.svg, which you predicted, and is fine, although I don't quite understand why this patch fixes that. But dynamic-marker-01.svg failed, which seems like a real problem. (It didn't fail on the Windows test though, so it might be random.)

(In reply to comment #9) > Backed out. > http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1232184917.1232189138.19404.gz#err1 > had two failures. One is an unexpected-pass on dynamic-filter-contents-01.svg, > which you predicted, and is fine, although I don't quite understand why this > patch fixes that. filters already use the async method to refresh which needs the change in nsCSSFrameConstructor to work properly. But dynamic-marker-01.svg failed, which seems like a real > problem. (It didn't fail on the Windows test though, so it might be random.) Not sure I can do anything about that till my Mac comes. It works for me but then I only have Windows at the moment.

(In reply to comment #10) > filters already use the async method to refresh which needs the change in > nsCSSFrameConstructor to work properly. Ah, of course. > > But dynamic-marker-01.svg failed, which seems like a real > > problem. (It didn't fail on the Windows test though, so it might be random.) > > Not sure I can do anything about that till my Mac comes. It works for me but > then I only have Windows at the moment. If I get a chance I'll try to reproduce it in my Windows VM. It might just be random though, try varying the timeouts. You could also try converting the test to use the MozReftestInvalidate event instead of setTimeout.

(In reply to comment #9) > patch fixes that. But dynamic-marker-01.svg failed, which seems like a real > problem. (It didn't fail on the Windows test though, so it might be random.) You confused me there. The Windows tinderbox was the only one to fail, Mac and Linux passed. This didn't fail for me on Windows although I only ran the reftests once with the patch in. I'll try running the test locally some more times to see if I can get it to fail.

reftests adjusted for asynchronicity of update

As I mentioned in the other bug, I'd really like the "not Gecko" timeout to be much larger, say 5000ms, both to ensure the timeout does not fire before MozReftestInvalidate (on a slow machine, like a slow test VM) and to ensure that the test is reliable on non-Gecko.

Hmm, and actually, the way these tests are doing it is wrong. You should be running doTest in response to MozReftestInvalidate and removing reftest-wait inside MozReftestInvalidate should work fine. The patch you had in your other bug might have the same problem.

Pushed http://hg.mozilla.org/mozilla-central/rev/7ab2054c7680 Hmm, I guess we should add a crashtest for this bug.

Verified fixed, using: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090126 Minefield/3.2a1pre

Pushed to 1.9.1 as http://hg.mozilla.org/releases/mozilla-1.9.1/rev/c126be164faa Note: These reftest.list changes weren't applicable to the 1.9.1 changeset... -fails == dynamic-filter-contents-01.svg dynamic-filter-contents-01-ref.svg # bug 471631 +== dynamic-filter-contents-01.svg dynamic-filter-contents-01-ref.svg ...because that test was _already_ marked as passing on 1.9.1. (The "fails" annotation had been added to mozilla-central as part of Bug 471365's patch, but that patch doesn't seem to be intended for 1.9.1) Aside from that, the 1.9.1 changeset is the same as the one that landed on mozilla-central.

Verified fixed on the 1.9.1 branch using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b4pre) Gecko/20090409 Shiretoko/3.5b4pre and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090409 Shiretoko/3.5b4pre. Adding keyword.