Loading the testcase triggers the assertion added in bug 458636: ###!!! ASSERTION: GetPrimaryFrameFor() called while frames are being destroyed!: '!mIsDestroyingFrames', file /Users/jruderman/central/layout/base/nsFrameManager.cpp, line 328 Is this the same issue as bug 458493?

Bug 458453 fixes this. It's the issue I pointed out in bug 458453 comment 2.

Bug 458453 did not fix this. Now I get three assertions: ###!!! ASSERTION: GetPrimaryFrameFor() called while frames are being destroyed!: '!mIsDestroyingFrames', file /Users/jruderman/central/layout/base/nsFrameManager.cpp, line 336 ###!!! ASSERTION: Cached frame is incorrect!: 'mElement.get() && static_cast<nsGenericElement*>(mElement.get())->GetPrimaryFrame() == mReferencedFrame', file /Users/jruderman/central/layout/svg/base/src/nsSVGEffects.cpp, line 81 ###!!! ASSERTION: PostRestyleEvent after the shell is destroyed (bug 279505): 'Not Reached', file /Users/jruderman/central/layout/base/nsCSSFrameConstructor.cpp, line 13470

As per comment https://bugzilla.mozilla.org/show_bug.cgi?id=279923#c13 in October 2008, this is the only assertion left on the Firefox leak boxes. Requesting blocking-1.9.1? because this prevents turning assertions fatal on the boxes.

(In reply to comment #4) > As per comment https://bugzilla.mozilla.org/show_bug.cgi?id=279923#c13 in > October 2008, this is the only assertion left on the Firefox leak boxes. > Requesting blocking-1.9.1? because this prevents turning assertions fatal on > the boxes. Assertions should now be fatal on the Firefox leak boxes since bug 463681 landed.

The main fix here is to avoid posting a style-change to the frame constructor if the frame constructor thinks it's tearing down the frame tree. Instead of asking the presshell if it's being destroyed, we should ask the frame constructor specifically if it's torn down the frame tree, as that catches the document-reconstruction case too. The other fix here is just to avoid callling GetPrimaryFrameFor in the NS_ASSERTION that checks the cached frame value, if it's unsafe to do so.

Pushed http://hg.mozilla.org/mozilla-central/rev/4167419a5f4a including a crashtest.

This caused 478128 so not landing on 1.9.1 until that bug is fixed.

Backed out: http://hg.mozilla.org/mozilla-central/rev/989bd3bc6cdf

There are two fixes here. One is like the last patch, just guard the assertion containing GetPrimaryFrameFor so that we don't test it when it's a bad time to call GetPrimaryFrameFor. The other fix is to have PostRestyleEvent check mPresShell->IsDestroying() instead of mIsDestroyingFrameTree. The latter is also set when we're reconstructing the frame tree, and in that case I think it's fine, in fact good, to proceed with PostRestyleEvent. Also, if we do bail out, I think the bail-out should be silent; swallowing restyles that are sure to be irrelevant is perfectly safe. Code like nsSVGFilter/MaskProperty::DoUpdate that calls PostRestyleEvent to ensure that stuff gets repainted should be able to do that at any time without triggering errors and without having to explicitly guard the call with !presshell->IsDestroying(). I also removed the parameter from WillDestroyFrameTree because it can just call presshell->IsDestroying() to know what's going on.

Comment on attachment 370169 [details] [diff] [review] fix #2 I should really get rid of ReconstructDocElementHierarchy....

Sorry no, that was actually http://hg.mozilla.org/mozilla-central/rev/7157cc2440bb

verified FIXED (no assertions) using the attached testcase on debug builds: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090608 Shiretoko/3.5pre ID:20090608122057 and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090608 Minefield/3.6a1pre ID:20090608122028