384105 - Crash [@ PresShell::AttributeChanged] with menuitem sizetopopup="always", position: absolute and tree stuff

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, which crashes Mozilla within 100ms.
It also crashes branch builds, so marking security sensitive for now.
I guess bug 279703 might fix things here.

Talkback ID: TB33045810E
0x01eb9089
PresShell::AttributeChanged  [mozilla/layout/base/nspresshell.cpp, line 4408]
nsNodeUtils::AttributeChanged  [mozilla/content/base/src/nsnodeutils.cpp, line 100]
nsXULElement::UnsetAttr  [mozilla/content/xul/content/src/nsxulelement.cpp, line 1316]
nsXULElement::GetAttrInfo  [mozilla/content/xul/content/src/nsxulelement.cpp, line 2108]
0x02dc3aa0
0x02c44be8
PresShell::AddRef  [mozilla/layout/base/nspresshell.cpp, line 1394]
0x08244c8b

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

taking for now. Need to think this a bit. Ungeneratemenu() shouldn't be called in ::Destroy().
Probably need to call that asynchronously, but only if menu doesn't 
have a new frame.

Comment 2

[Martijn Wargers (dead)]

This is worksforme now that the patch for bug 279703 went in. (tested with a tinderbox build)

Comment 3

[Samuel Sidler (old account; do not CC)]

Attachment: Crash log from branch build on Mac

This still affects a branch build. Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.5pre) Gecko/20070716 BonEcho/2.0.0.5pre

Requesting blocking (though; should probably be changed to wanted).

Comment 4

[Daniel Veditz [:dveditz]]

Crashes rv:1.8.0.12 as well (Firefox 1.5.0.12).

I can't see bug 279703 landing on the 1.8 branch. I also get a different stack on branch, "this" is a deleted object in nsCachedStyleData::GetStyleData(). Is there some way to wallpaper this for the branch? This looks familiar actually, maybe this testcase is triggering a different bug that's already fixed on trunk but not yet backported, and once we get that out of the way we'll see the nsPresShell::AttributeChanged issue on the branch.

>	nsCachedStyleData::GetStyleData() Line 210	C++
 	nsStyleContext::GetStyleData() Line 248	C++
 	nsIFrame::GetStyleData() Line 612	C++
 	nsIFrame::GetStyleDisplay() Line 90	C++
 	nsCSSFrameConstructor::AttributeChanged() Line 10752	C++
 	PresShell::AttributeChanged() Line 5499	C++
 	nsXULDocument::AttributeChanged() Line 1133	C++
 	nsXULElement::UnsetAttr() Line 1706	C++
 	nsMenuFrame::UngenerateMenu() Line 712	C++
 	nsMenuFrame::Destroy() Line 398	C++
 	nsLineBox::DeleteLineList() Line 325	C++
 	nsBlockFrame::Destroy() Line 303	C++
 	nsAreaFrame::Destroy() Line 155	C++
 	nsBoxFrame::RemoveFrame() Line 1178	C++
 	nsFrameManager::RemoveFrame() Line 717	C++
 	nsCSSFrameConstructor::ContentRemoved() Line 10141	C++
 	nsCSSFrameConstructor::RecreateFramesForContent() Line 12102	C++
 	nsCSSFrameConstructor::RestyleElement() Line 10600	C++
 	nsCSSFrameConstructor::ProcessOneRestyle() Line 14147	C++
 	nsCSSFrameConstructor::ProcessPendingRestyles() Line 14201	C++
 	nsCSSFrameConstructor::RestyleEvent::HandleEvent() Line 14265	C++
 	HandleRestyleEvent() Line 14274	C++
 	PL_HandleEvent() Line 688	C
 	PL_ProcessPendingEvents() Line 623	C
 	_md_EventReceiverProc() Line 1408	C
 	7e418724	
 	7e418806	
 	7e4189bd	
 	7e4193f2	
 	7e418a00	
 	nsAppShell::Run() Line 133	C++
 	nsAppStartup::Run() Line 151	C++
 	XRE_main() Line 2711	C++
 	main() Line 61	C++
 	mainCRTStartup() Line 398	C
 	7c816fd7	

Comment 5

[Daniel Veditz [:dveditz]]

This is a 1.8.1 branch blocker, but we can't take the 500K trunk patch ("redesign XUL popups"). Please see if there's a local fix for just this crash.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Attachment: WIP for 1.8

Fixes the crash, but I do see 
###!!! ASSERTION: frame was not removed from primary frame map before destruction or was readded to map after being removed: 'Not Reached', file /home/smaug/mozilla/mozilla_cvs/18_branch/mozilla/layout/base/nsFrameManager.cpp, line 734

That assertion is (sort of) from bug Bug 265404.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Other possibility is to not to notify when removing the attribute. Same assertions
happen then though. And not notifying means no mutation events when dom is mutated; that is hackish.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Attachment: do not notify when unsetting the attribute

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Are there any other possibilities?

Comment 10

[Daniel Veditz [:dveditz]]

Smaug: is this likely to get a branch patch, reviews and approvals by tomorrow night (1.8.1.8 code freeze)?

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 280653 [details] [diff] [review]
WIP for 1.8

Roc, what do you think about this. Not perfect, but possibly enough for branch. Not sure if some problem occurs when recreating a frame for menu...

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 280653 [details] [diff] [review]
WIP for 1.8

looks OK but Neil should take a look

Comment 13

[Daniel Veditz [:dveditz]]

Comment on attachment 280653 [details] [diff] [review]
WIP for 1.8

approved for 1.8.1.8, a=dveditz

Comment 14

[Daniel Veditz [:dveditz]]

Fix checked in for Olli

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

Thanks Daniel. Somehow I didn't notice this got reviews.

Comment 16

[Carsten Book [:Tomcat]]

verified fixed 1.8.1.8 using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.8) Gecko/2007100816 Firefox/2.0.0.8 and Mozilla/5.0 (Macintosh; U; Intel Mac OS X; ja-JP-mac; rv:1.8.1.8) Gecko/2007100816 Firefox/2.0.0.8 and the testcase from this bug.

-> no crash on testcase - adding verified keyword

Comment 17

[Alexander Sack]

Comment on attachment 280653 [details] [diff] [review]
WIP for 1.8

a=asac for 1.8.0.15

(same patch shipped by distros for some time now)

Comment 18

[Christopher Aillon (sabbatical, not receiving bugmail)]

checked in on 1.8.0 branch

Comment 19

[Bob Clary [:bc] (inactive)]

crash test landed
http://hg.mozilla.org/mozilla-central/rev/4457b7408447