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




10 years ago
6 years ago


(Reporter: Martijn Wargers (dead), Assigned: smaug)


(Blocks: 1 bug, 4 keywords)

crash, fixed1.8.0.15, testcase, verified1.8.1.8
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)



10 years ago
Created attachment 268038 [details]

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]


10 years ago
OS: Windows XP → All

Comment 1

10 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

10 years ago
This is worksforme now that the patch for bug 279703 went in. (tested with a tinderbox build)
Last Resolved: 10 years ago
Resolution: --- → FIXED
Created attachment 272571 [details]
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: 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

10 years ago
Created attachment 280653 [details] [diff] [review]
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

10 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 8

10 years ago
Created attachment 280657 [details] [diff] [review]
do not notify when unsetting the attribute

Comment 9

10 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

10 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)
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

10 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
Keywords: fixed1.8.1.8 → verified1.8.1.8
Group: security
Flags: in-testsuite?
Depends on: 400976
Depends on: 400744

Comment 17

10 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

8 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.