400185 - Crash [@ nsFrameManager::ReResolveStyleContext] with popupgroup, treecols, nativescrollbar and (re)moving things

Status: VERIFIED FIXED

(Core :: XUL, defect, P3)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, which crashes current trunk build on load.

This regressed between 2007-07-04 and 2007-07-05, so I think a regression of bug 279703.

Maybe the patch for bug 399013 or bug 386642 can fix this?

http://crash-stats.mozilla.com/report/index/d6542138-7ce2-11dc-9848-001a4bd43e5c
0  	nsFrameManager::ReResolveStyleContext(nsPresContext*, nsIFrame*, nsIContent*, nsStyleChangeList*, nsChangeHint)  	 mozilla/layout/base/nsFrameManager.cpp:1075
1 	nsFrameManager::ReResolveStyleContext(nsPresContext*, nsIFrame*, nsIContent*, nsStyleChangeList*, nsChangeHint) 	mozilla/layout/base/nsFrameManager.cpp:1355
2 	nsFrameManager::ReResolveStyleContext(nsPresContext*, nsIFrame*, nsIContent*, nsStyleChangeList*, nsChangeHint) 	mozilla/layout/base/nsFrameManager.cpp:1365
3 	nsFrameManager::ReResolveStyleContext(nsPresContext*, nsIFrame*, nsIContent*, nsStyleChangeList*, nsChangeHint) 	mozilla/layout/base/nsFrameManager.cpp:1365
4 	nsFrameManager::ReResolveStyleContext(nsPresContext*, nsIFrame*, nsIContent*, nsStyleChangeList*, nsChangeHint) 	mozilla/layout/base/nsFrameManager.cpp:1365
5 	nsFrameManager::ReResolveStyleContext(nsPresContext*, nsIFrame*, nsIContent*, nsStyleChangeList*, nsChangeHint) 	mozilla/layout/base/nsFrameManager.cpp:1365
6 	nsFrameManager::ComputeStyleChangeFor(nsIFrame*, nsStyleChangeList*, nsChangeHint) 	mozilla/layout/base/nsFrameManager.cpp:1427
7 	nsCSSFrameConstructor::RestyleElement(nsIContent*, nsIFrame*, nsChangeHint) 	mozilla/layout/base/nsCSSFrameConstructor.cpp:10091
8 	nsCSSFrameConstructor::ProcessOneRestyle(nsIContent*, nsReStyleHint, nsChangeHint) 	mozilla/layout/base/nsCSSFrameConstructor.cpp:13135
9 	nsCSSFrameConstructor::ProcessPendingRestyles() 	mozilla/layout/base/nsCSSFrameConstructor.cpp:13188
10 	PresShell::DoFlushPendingNotifications(mozFlushType, int) 	mozilla/layout/base/nsPresShell.cpp:4443
11 	PresShell::FlushPendingNotifications(mozFlushType) 	mozilla/layout/base/nsPresShell.cpp:4407
12 	nsCSSFrameConstructor::RestyleEvent::Run() 	mozilla/layout/base/nsCSSFrameConstructor.cpp:13244
13 	nsThread::ProcessNextEvent(int, int*) 	mozilla/xpcom/threads/nsThread.cpp:490
14 	NS_ProcessNextEvent_P(nsIThread*, int) 	nsThreadUtils.cpp:227
15 	nsBaseAppShell::Run() 	mozilla/widget/src/xpwidgets/nsBaseAppShell.cpp:154
16 	nsAppStartup::Run() 	mozilla/toolkit/components/startup/src/nsAppStartup.cpp:170
17 	XRE_main 	mozilla/toolkit/xre/nsAppRunner.cpp:3142
18 	main 	mozilla/browser/app/nsBrowserApp.cpp:153
19 	WinMain 	mozilla/browser/app/nsBrowserApp.cpp:166
20 	__tmainCRTStartup 	crtexe.c:589
21 	BaseProcessStart

Comment 1

[Neil Deakin]

Neither the patch in bug 399013 or bug 386642 fix this crash.

I get an assertion also:

###!!! ASSERTION: Deleting out of flow without tearing down placeholder relationship; see comments in nsFrame.h: '!(mState & NS_FRAME_OUT_OF_FLOW) || !shell->FrameManager()->GetPlaceholderFrameFor(this)', file /builds/trunk-project/mozilla/layout/generic/nsFrame.cpp, line 487

Comment 2

[Martijn Wargers (dead)]

Maybe the patch in bug 398982 fixes this?

Comment 3

[Neil Deakin]

Nope, it still crashes.

Comment 4

[Neil Deakin]

Attachment: The easiest fix is to just ignore any attempts to use popupgroup directly

Comment 5

[Boris Zbarsky [:bzbarsky]]

So what is the effect of this patch?  Where do the native anonymous popupgroups come from?  What will we do with the non-anonymous ones?

Comment 6

[Neil Deakin]

Each document should have one native anonymous popupgroup created by the document frame (nsDocElementBoxFrame). The popupgroup is used as the content node for an nsPopupSetFrame which holds all of the non-menu popups (tooltips, context menus, etc).

There isn't any reason to have more than one popupgroup element. Creating a second one currently causes assertions.

Ideally, we could just get rid of popupgroup and create an anonymous frame directly in some way.

Comment 7

[Boris Zbarsky [:bzbarsky]]

That doesn't answer the last question in comment 5.  What sort of frame do we end up creating for the popupgroup with this patch?

> Ideally, we could just get rid of popupgroup and create an anonymous frame
> directly in some way.

That's doable, sure.

Comment 8

[Neil Deakin]

(In reply to comment #7)
> That doesn't answer the last question in comment 5.  What sort of frame do we
> end up creating for the popupgroup with this patch?
> 

It should just fall through and create a boxframe as with any xul tag that isn't recognized, (and that hasn't changed the display property).

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 293863 [details] [diff] [review]
The easiest fix is to just ignore any attempts to use popupgroup directly

Looks ok, then, assuming we never look for ::popupgroup tags anywhere.

Comment 10

[Carsten Book [:Tomcat]]

verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9b3pre) Gecko/2008010104 Minefield/3.0b3pre ID:2008010104 and the testcase. No Crash on testcase - changing bug to verified fixed

Comment 11

[Jesse Ruderman]

Crashtest checked in.