315752 - random crashes on unb forums [@ DoDeletingFrameSubtree 4684cd55]

Status: RESOLVED FIXED

(Core :: Layout, defect, P1)

Description

[Robert Förster]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051109 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051109 Firefox/1.6a1

i can't post reproducable things here, it is crashing randomly on this website.
it also happens in the forum software developed there.

Reproducible: Sometimes




Talkback: TB11642998W and TB11643456M

Comment 1

[Peter van der Woude [:Peter6]]

Incident ID: 11642998
Stack Signature	DoDeletingFrameSubtree 49187fb5
Product ID	FirefoxTrunk
Build ID	2005110905
Trigger Time	2005-11-09 10:37:44.0
Platform	Win32
Operating System	Windows NT 5.1 build 2600
Module	FIREFOX.EXE + (00184f29)
URL visited	http://newsboard.unclassified.de/forum/
User Comments	register there and go to settings. then click on "Appereance" change some settings there and save, then the browser seems to hang; closed it --> crash
Since Last Crash	7753 sec
Total Uptime	7753 sec
Trigger Reason	Access violation
Source File, Line No.	c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 9619
Stack Trace 	
DoDeletingFrameSubtree  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 9619]
DoDeletingFrameSubtree  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 9642]
DoDeletingFrameSubtree  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 9642]
DeletingFrameSubtree  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 9688]
nsCSSFrameConstructor::RemoveMappingsForFrameSubtree  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 9738]
nsLineBox::DeleteLineList  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsLineBox.cpp, line 325]
nsLineBox::DeleteLineList  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsLineBox.cpp, line 325]
nsLineBox::DeleteLineList  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsLineBox.cpp, line 325]
nsLineBox::DeleteLineList  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsLineBox.cpp, line 325]
nsLineBox::DeleteLineList  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsLineBox.cpp, line 325]
nsLineBox::DeleteLineList  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsLineBox.cpp, line 325]
nsLineBox::DeleteLineList  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsLineBox.cpp, line 325]
nsFrameList::DestroyFrames  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsFrameList.cpp, line 138]
nsFrameList::DestroyFrames  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsFrameList.cpp, line 138]
nsFrameList::DestroyFrames  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsFrameList.cpp, line 138]
nsFrameList::DestroyFrames  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsFrameList.cpp, line 138]
nsTableFrame::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/tables/nsTableFrame.cpp, line 312]
nsTableOuterFrame::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/tables/nsTableOuterFrame.cpp, line 81]
nsLineBox::DeleteLineList  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsLineBox.cpp, line 325]
nsFrameList::DestroyFrames  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsFrameList.cpp, line 138]
CanvasFrame::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsHTMLFrame.cpp, line 220]
nsFrameList::DestroyFrames  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsFrameList.cpp, line 138]
nsHTMLScrollFrame::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsGfxScrollFrame.cpp, line 165]
nsPositionedInlineFrame::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsInlineFrame.cpp, line 1052]
DocumentViewerImpl::Hide  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsDocumentViewer.cpp, line 2024]
nsSubDocumentFrame::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsFrameFrame.cpp, line 553]
nsFrameList::DestroyFrames  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsFrameList.cpp, line 138]
nsBoxFrame::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp, line 1105]
nsBoxFrame::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp, line 1105]
nsBoxFrame::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp, line 1105]
nsBoxFrame::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp, line 1105]
nsBoxFrame::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp, line 1105]
nsBoxFrame::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp, line 1105]
nsBoxFrame::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp, line 1105]
nsBoxFrame::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp, line 1105]
nsPositionedInlineFrame::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsInlineFrame.cpp, line 1052]
DocumentViewerImpl::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsDocumentViewer.cpp, line 1556]
nsDocShell::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/docshell/base/nsDocShell.cpp, line 3532]
nsXULWindow::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/xpfe/appshell/src/nsXULWindow.cpp, line 511]
nsWebShellWindow::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/xpfe/appshell/src/nsWebShellWindow.cpp, line 850]
nsWebShellWindow::HandleEvent  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/xpfe/appshell/src/nsWebShellWindow.cpp, line 408]
nsWindow::DispatchEvent  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp, line 1140]
nsWindow::DispatchStandardEvent  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp, line 1180]
nsWindow::ProcessMessage  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp, line 4188]
nsWindow::WindowProc  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp, line 1329]
USER32.dll + 0x8734 (0x77d18734)
USER32.dll + 0x8816 (0x77d18816)
USER32.dll + 0xb4c0 (0x77d1b4c0)
USER32.dll + 0xb50c (0x77d1b50c)
ntdll.dll + 0xeae3 (0x7c91eae3)
USER32.dll + 0xb3f9 (0x77d1b3f9)
uxtheme.dll + 0x3c20 (0x5b0f3c20)
uxtheme.dll + 0x1e300 (0x5b10e300)
uxtheme.dll + 0x1ac7 (0x5b0f1ac7)
uxtheme.dll + 0x1b3d (0x5b0f1b3d)
USER32.dll + 0xbb15 (0x77d1bb15)
nsWindow::DefaultWindowProc  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp, line 1355]
USER32.dll + 0x8734 (0x77d18734)
USER32.dll + 0x8816 (0x77d18816)
USER32.dll + 0xc63f (0x77d1c63f)
USER32.dll + 0xc665 (0x77d1c665)
nsWindow::WindowProc  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp, line 1336]
USER32.dll + 0x8734 (0x77d18734)
USER32.dll + 0x8816 (0x77d18816)

Comment 2

[Martijn Wargers (dead)]

Attachment: testcase

They use floating images inside a select drop down list.
This could very well be related to bug 310505.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix, I think

So the problem is that comboboxes try to do this extra destruction stuff (added for bug 117984).  If we're destroying the whole document, though, placeholders will already be nulled out (bug 297850), so we'll crash as in this bug.

This patch basically backs out the one from bug 117984 and refixes that bug by making DoCleanupFrameReferences properly walk all child lists; troy's comment about how only the principal list needs to be walked seems bogus to me.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 205341 [details] [diff] [review]
Fix, I think

Mats, could you also take a look at this?  It's good to hear from you again, btw!

Comment 5

[Daniel Veditz [:dveditz]]

Sounds like we'd want this in the next stability release, it fixes several related crashers.

Comment 6

[Daniel Veditz [:dveditz]]

(In reply to comment #5)
> Sounds like we'd want this in the next stability release

scratch that, this is fallout from bug 117984 which was after the 1.8 branch

Comment 7

[Mats Palmgren (inactive)]

Comment on attachment 205341 [details] [diff] [review]
Fix, I think

Yes, this is the right fix.
The reason RemoveMappingsForFrameSubtree() is bad is that the
frame constructor actually knows about the combobox popup:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/forms/nsComboboxControlFrame.cpp&rev=1.346&root=/cvsroot&mark=2116-2118#2104

nsMenuFrame hides it and that's why it needs to have
an explicit RemoveMappingsForFrameSubtree():
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/xul/base/src/nsMenuFrame.cpp&rev=1.305&root=/cvsroot&mark=338,316,296-305#292

The reason for the assertion in bug 117984 was actually the
missing processing of additional lists in DoCleanupFrameReferences().
I verified that using this URLhttp://news.bbc.co.uk/sport3/worldcup2002/hi/history/newsid_1966000/1966379.stm

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: bz's fix, updated to tip

We're looking at the trio of bugs that this fixes for 1.8.0.1, so I'm taking the liberty of checking this in sooner, rather than later.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 206354 [details] [diff] [review]
bz's fix, updated to tip

Cancelling nomination, since this is a regression from a post-branch change.

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

I just checked this fix into the trunk on behalf of bz.

Comment 12

[Jesse Ruderman]

Crashtest checked in.