User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.2) Gecko/20070219 Firefox/2.0.0.2 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.2) Gecko/20070219 Firefox/2.0.0.2 In my extension i decided to put the textbox inside a toolbarbutton to save some space on my toolbar. The toolbarbutton is of type menu. In that menu i have the textbox and some menuitems. I have onpopupshown="this.enableKeyboardNavigator(false);" on the menupopup in order to allow typing inside the textbox. Everything is ok i can open the menu, type inside the textbox and everything works (the actions associated with the textbox are all ok). The crash appears when closing the tab, the window or firefox. Reproducible: Always Steps to Reproduce: 1. Load the test XUL file in extension developer's XUL editor 2. press the button to open the menu of the toolbarbutton 3. close XUL editor Actual Results: Firefox crashes. If you don't open the menu it wouldn't crash Expected Results: Firefox should not have crashed Used Firefox 2.0.0.2 on WindowsXP

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a3pre) Gecko/20070302 Minefield/3.0a3pre ID:2007030204 [cairo] I believe this caused me to crash too. (after closing the testcase and trying to navigate away from bugzilla). TB29906682Z Incident ID: 29906682 Stack Signature DoDeletingFrameSubtree 40d2dd49 Product ID FirefoxTrunk Build ID 2007030204 Trigger Time 2007-03-04 05:36:51.0 Platform Win32 Operating System Windows NT 5.0 build 2195 Module firefox.exe + (0023dce2) URL visited bug 372576 User Comments loaded bug 372576 testcase, clicked button, entered some text in text area, closed page, navigated away from bugzilla, crashed Since Last Crash 58086 sec Total Uptime 58086 sec Trigger Reason Access violation Source File, Line No. e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\layout\base\nscssframeconstructor.cpp, line 9247 Stack Trace DoDeletingFrameSubtree [mozilla/layout/base/nscssframeconstructor.cpp, line 9247] DoDeletingFrameSubtree [mozilla/layout/base/nscssframeconstructor.cpp, line 9253] DoDeletingFrameSubtree [mozilla/layout/base/nscssframeconstructor.cpp, line 9253] DoDeletingFrameSubtree [mozilla/layout/base/nscssframeconstructor.cpp, line 9253] DoDeletingFrameSubtree [mozilla/layout/base/nscssframeconstructor.cpp, line 9253] DoDeletingFrameSubtree [mozilla/layout/base/nscssframeconstructor.cpp, line 9253] DoDeletingFrameSubtree [mozilla/layout/base/nscssframeconstructor.cpp, line 9253] DoDeletingFrameSubtree [mozilla/layout/base/nscssframeconstructor.cpp, line 9253] DeletingFrameSubtree [mozilla/layout/base/nscssframeconstructor.cpp, line 9307] nsCSSFrameConstructor::RemoveMappingsForFrameSubtree [mozilla/layout/base/nscssframeconstructor.cpp, line 9331] nsFrameList::DestroyFrames [mozilla/layout/generic/nsframelist.cpp, line 60] nsFrameList::DestroyFrames [mozilla/layout/generic/nsframelist.cpp, line 60] nsFrameList::DestroyFrames [mozilla/layout/generic/nsframelist.cpp, line 60] nsFrameList::DestroyFrames [mozilla/layout/generic/nsframelist.cpp, line 60] nsFrameList::DestroyFrames [mozilla/layout/generic/nsframelist.cpp, line 60] nsFrameManager::Destroy [mozilla/layout/base/nsframemanager.cpp, line 301] DocumentViewerImpl::Destroy [mozilla/layout/base/nsdocumentviewer.cpp, line 1629] nsSHEntry::~nsSHEntry [mozilla/docshell/shistory/src/nsshentry.cpp, line 122] nsSHEntry::Release [mozilla/docshell/shistory/src/nsshentry.cpp, line 129] nsDocShell::AddToSessionHistory [mozilla/docshell/base/nsdocshell.cpp, line 7766] nsDocShell::OnNewURI [mozilla/docshell/base/nsdocshell.cpp, line 7536] nsDocShell::OnLoadingSite [mozilla/docshell/base/nsdocshell.cpp, line 7581] nsDocShell::CreateContentViewer [mozilla/docshell/base/nsdocshell.cpp, line 5757] nsDSURIContentListener::DoContent [mozilla/docshell/base/nsdsuricontentlistener.cpp, line 139] nsDocumentOpenInfo::TryContentListener [mozilla/uriloader/base/nsuriloader.cpp, line 791] nsDocumentOpenInfo::DispatchContent [mozilla/uriloader/base/nsuriloader.cpp, line 488] table 0x7302f883 Dupe of bug 360339 and/or bug 364034?

It also crashes by: 1. load the attached testcase 2. press the button to open the menu of the toolbarbutton 3. reload the page

This fixes the crash for the testcase (and most likely bug 364034). I think this is worth it, at least until we can figure out how to make menu frame construction/destruction sane. Also for branches.

I think the frame tree is correct so this is something we need to deal with.

Deal with null out-of-flow, assert if not a popup.

So why is this needed in this testcase? Which subtree in that frametree do we enter twice and why?

We only enter DoDeletingFrameSubtree once. The popup frame is null because the are only created while the are displayed (I think). I guess this is a case DoDeletingFrameSubtree never really handled correctly; a menuitem with a descendent popup (the textbox context menu in this case I think).

Um... if the frame is not created yet, why is the placeholder created? I can't think of a way that should be able to happen. In particular, the only caller of nsPlaceholderFrame() is NS_NewPlaceholderFrame(), the only caller of that is nsCSSFrameConstructor::CreatePlaceholderFrameFor, and that code would crash if aFrame were null. So something is nulling out the out-of-flow on the placeholder without removing the placeholder from the tree. Or we're crashing before it gets a chance to remove the placeholder from the tree... Is that frame dump from a steady state? or sometime in the middle of teardown?

Right, there's something missing in my story, let me have a look again...

(In reply to comment #10) > So something is nulling out the out-of-flow on the placeholder without removing > the placeholder from the tree. PresShell::Destroy nsFrameManager::Destroy nsFrameManager::ClearPlaceholderFrameMap then while destroying the frame tree: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/xul/base/src/nsMenuFrame.cpp&rev=1.344&root=/cvsroot&mark=336#328 which is how we reach DoDeletingFrameSubtree() even though PresShell.mIsDestroying=1 I see that PresShell::Destroy() calls mFrameConstructor->WillDestroyFrameTree() before it starts destroying stuff, maybe we could use that to make an early return in RemoveMappingsForFrameSubtree() ? Or expose mIsDestroying? http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/base/nsPresShell.cpp&rev=3.971&root=/cvsroot&mark=1662#1618

I'd prefer it if the frame constructor ignored calls to RemoveMappingsForFrameSubtree while we're destroying. Either by asking the presshell, or by the presshell notifying the frame constructor when it starts destroying.

Comment on attachment 257266 [details] [diff] [review] Patch rev. 2 Looks good. Add a comment saying that the tree might not be in a consistent state to start with if we're destroying the presshell or something?

Added a comment and checked in to trunk at 2007-03-04 14:39 PST. Will ask for branch approval in a couple of days. -> FIXED

You can't change nsIPresShell on branch.

Would this be an acceptable change for the branches?

Er, I somehow missed part of comment 12. That patch is fine for branches, and in fact is what we should do on trunk, imo.

not in scope for 1.8.1.3, moving nom to 1.8.1.4

Note: this is a 1.8 branch diff. For trunk I'll back out rev. 2 in addition to this. (I changed the comment slightly compared with the last "Alternative patch" - no code changes.)

Comment on attachment 257501 [details] [diff] [review] Alternative patch, for trunk and branches Looks great!

Comment on attachment 257501 [details] [diff] [review] Alternative patch, for trunk and branches Landed on trunk (+ backing out rev. 2) at 2007-03-06 13:07 PST.

Comment on attachment 257501 [details] [diff] [review] Alternative patch, for trunk and branches Low-risk crash fix

Comment on attachment 257501 [details] [diff] [review] Alternative patch, for trunk and branches approved for 1.8/1.8.0 branches, a=dveditz for drivers

Checked in to MOZILLA_1_8_BRANCH at 2007-03-23 00:25 PST Checked in to MOZILLA_1_8_0_BRANCH at 2007-03-23 00:38 PST

Verified fixed on branch. The latest branch builds don't crash anymore on the testcase and the steps to reproduce.

Crash test: https://hg.mozilla.org/integration/mozilla-inbound/rev/499f187c9b7a