Closed Bug 117984 Opened 18 years ago Closed 14 years ago
[PFM]ASSERTION: frame was not removed from primary frame map before destruction
When loading the above page, I hit the assertion in FrameManager::NotifyDestroyingFrame() about the frame not being removed propertly from the primary frame map. Seems bad...
There is another bug on seeing that with object frames that get replaced.
probably a dupe of bug 101216
No, I was thinking of bug 117703.
The assertions are kind of annoying. Is there a simple fix for whatever bug this is?
The assertions are a sign that we're very likely to crash if we do anything else with the page, and they should not be removed. What we could do is change the code so that it does the removal there -- I've been tempted to do that, but I don't want to have the removal in both places, and I'm not sure what would happen if we removed it from the earlier site.
Yes, I don't want the assertion code gone, just a fix so that the assertion isn't triggered. Why isn't the frame remove the same way for objects as for everything else? Plugin difficulties?
Another page where the assertion is a always firing: http://news.bbc.co.uk/sport3/worldcup2002/hi/history/newsid_1966000/1966379.stm
Summary: ASSERTION: frame was not removed from primary frame map before destruction... → [PFM]ASSERTION: frame was not removed from primary frame map before destruction...
Assignee: dbaron → mats.palmgren
OS: Windows 2000 → All
Call 'RemoveMappingsForFrameSubtree()' before destroying 'mPopupFrames'.
Comment on attachment 194823 [details] [diff] [review] Patch rev. 1 r+sr=bzbarsky, but perhaps nsFrameList should just handle this? Or would this a very special case?
I did a quick investigation into why this is needed. The assertion is rooted in call to WipeContainingBlock(): http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/base/nsCSSFrameConstructor.cpp&rev=1.1126&root=/cvsroot&mark=13176#13135 it seems that WipeContainingBlock() takes care of abs/fixed/float through the state and that CleanupFrameReferences() is designed to only take care of the principal child list. http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/base/nsCSSFrameConstructor.cpp&rev=1.1126&root=/cvsroot&mark=682#652 This means that any frame that has a "special" child list (or other private frames not in the principal child list) not known to the frame constructor needs to deal with this explicitly. I found that 'nsMenuFrame' that also have a 'popupList' child list have the same code that I suggested: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/xul/base/src/nsMenuFrame.cpp&rev=1.298&root=/cvsroot&mark=322-325#315 Given what WipeContainingBlock() and other code currently does, I would say that adding RemoveMappingsForFrameSubtree() to nsFrameList::DestroyFrames() is probably not the right design (likely a perf problem if we do). I think we should just add calls to RemoveMappingsForFrameSubtree() in ::Destroy() where needed... What do you think?
Correction, the first link in the last comment should be: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/base/nsCSSFrameConstructor.cpp&rev=1.1126&root=/cvsroot&mark=13173#13135
Ah, I see. Child lists that are being used for stuff that has no placeholders.. The patch looks good, then, but maybe we should have a way to enumerate such child lists, in general?
Took the liberty of removing an unused variable as well
Checked in to trunk at 2005-09-20 13:28 PDT -> FIXED
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.