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




12 years ago
8 years ago


(Reporter: martijn.martijn, Assigned: smaug)


(Blocks 1 bug, 4 keywords)

Dependency tree / graph
Bug Flags:
blocking1.8.1.8 +
wanted1.8.1.x +
wanted1.8.0.x +
in-testsuite +

Firefox Tracking Flags

(Not tracked)


(Whiteboard: [sg:critical?] fixed by 279703 on trunk, crash signature)


(4 attachments)



12 years ago
Posted file 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
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]
PresShell::AddRef  [mozilla/layout/base/nspresshell.cpp, line 1394]


12 years ago
OS: Windows XP → All

Comment 1

12 years ago
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.
Assignee: nobody → Olli.Pettay

Comment 2

12 years ago
This is worksforme now that the patch for bug 279703 went in. (tested with a tinderbox build)
Closed: 12 years ago
Resolution: --- → FIXED
This still affects a branch build. Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv: Gecko/20070716 BonEcho/

Requesting blocking (though; should probably be changed to wanted).
Flags: blocking1.8.1.6?
Crashes rv: as well (Firefox

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
 	nsAppShell::Run() Line 133	C++
 	nsAppStartup::Run() Line 151	C++
 	XRE_main() Line 2711	C++
 	main() Line 61	C++
 	mainCRTStartup() Line 398	C
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Whiteboard: [sg:critical?] fixed by 279703 on trunk
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.
Flags: blocking1.8.1.7? → blocking1.8.1.7+

Comment 6

12 years ago
Posted patch WIP for 1.8Splinter Review
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

12 years ago
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 9

12 years ago
Are there any other possibilities?
Smaug: is this likely to get a branch patch, reviews and approvals by tomorrow night ( code freeze)?

Comment 11

12 years ago
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...
Attachment #280653 - Flags: review?(roc)
Comment on attachment 280653 [details] [diff] [review]
WIP for 1.8

looks OK but Neil should take a look
Attachment #280653 - Flags: superreview+
Attachment #280653 - Flags: review?(roc)
Attachment #280653 - Flags: review?(enndeakin)


12 years ago
Attachment #280653 - Flags: review?(enndeakin) → review+
Comment on attachment 280653 [details] [diff] [review]
WIP for 1.8

approved for, a=dveditz
Attachment #280653 - Flags: approval1.8.1.8+
Whiteboard: [sg:critical?] fixed by 279703 on trunk → [sg:critical?] fixed by 279703 on trunk; need branch landing
Fix checked in for Olli
Keywords: fixed1.8.1.8
Whiteboard: [sg:critical?] fixed by 279703 on trunk; need branch landing → [sg:critical?] fixed by 279703 on trunk

Comment 15

12 years ago
Thanks Daniel. Somehow I didn't notice this got reviews.
verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/2007100816 Firefox/ and Mozilla/5.0 (Macintosh; U; Intel Mac OS X; ja-JP-mac; rv: Gecko/2007100816 Firefox/ and the testcase from this bug.

-> no crash on testcase - adding verified keyword
Group: security


12 years ago
Flags: in-testsuite?
Depends on: 400744

Comment 17

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

a=asac for

(same patch shipped by distros for some time now)
Attachment #280653 - Flags: approval1.8.0.15+
checked in on 1.8.0 branch
Keywords: fixed1.8.0.15

Comment 19

10 years ago
crash test landed
Flags: in-testsuite? → in-testsuite+
Crash Signature: [@ PresShell::AttributeChanged]
You need to log in before you can comment on or make changes to this bug.