Closed
Bug 132334
Opened 22 years ago
Closed 22 years ago
[PATCH][PFM]crash if repeat changing display of absolutely positioned form element [@ nsIFrame::GetStyleData]
Categories
(Core :: Layout: Form Controls, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla1.0
People
(Reporter: mail, Assigned: karnaze)
References
()
Details
(Keywords: crash, testcase, Whiteboard: [adt2])
Crash Data
Attachments
(2 files, 1 obsolete file)
815 bytes,
text/html
|
Details | |
8.43 KB,
patch
|
karnaze
:
review+
karnaze
:
superreview+
asa
:
approval+
|
Details | Diff | Splinter Review |
From Bugzilla Helper: User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.9) Gecko/20020311 BuildID: 2002031104 In the testcase to follow, changing the display back and forth from block to none of an absolutely positioned div inside a form causes a crash. Removing the positioning, or removing the form element prevents the crash. Reproducible: Always Steps to Reproduce: 1. Run testcase 2. click several times on hide/show button 3. Actual Results: browser crashes Expected Results: should hide and show div
Reporter | ||
Comment 1•22 years ago
|
||
Updated•22 years ago
|
Comment 2•22 years ago
|
||
###!!! ASSERTION: frame was not removed from primary frame map before destruction or was readded to map after being removed: '!PL_DHASH_ENTRY_IS_BUSY(entry) || entry->frame != aFrame', file nsFrameManager.cpp, line 1009 ###!!! Break: at file nsFrameManager.cpp, line 1009 ###!!! ASSERTION: frame was not removed from primary frame map before destruction or was readded to map after being removed: '!PL_DHASH_ENTRY_IS_BUSY(entry) || entry->frame != aFrame', file nsFrameManager.cpp, line 1009 ###!!! Break: at file nsFrameManager.cpp, line 1009 ###!!! ASSERTION: frame was not removed from primary frame map before destruction or was readded to map after being removed: '!PL_DHASH_ENTRY_IS_BUSY(entry) || entry->frame != aFrame', file nsFrameManager.cpp, line 1009 ###!!! Break: at file nsFrameManager.cpp, line 1009 ###!!! ASSERTION: No style context found!: 'mStyleContext', file ../../../dist/include/layout/nsIFrame.h, line 556 ###!!! Break: at file ../../../dist/include/layout/nsIFrame.h, line 556 Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 1024 (runnable)] 0x412d66eb in nsIFrame::GetStyleData (this=0x8791770, aSID=eStyleStruct_Display, aStyleStruct=@0xbfffccac) at ../../../dist/include/layout/nsIFrame.h:557 557 aStyleStruct = mStyleContext->GetStyleData(aSID); (gdb) (gdb) p mStyleContext $1 = (class nsIStyleContext *) 0x0 #0 0x412d66eb in nsIFrame::GetStyleData (this=0x8791770, aSID=eStyleStruct_Display, aStyleStruct=@0xbfffccac) at ../../../dist/include/layout/nsIFrame.h:557 #1 0x410e53a9 in nsComputedDOMStyle::GetStyleData (this=0x86a2968, aID=eStyleStruct_Display, aStyleStruct=@0xbfffccac, aFrame=0x8791770) at nsComputedDOMStyle.cpp:3623 #2 0x410e1a8e in nsComputedDOMStyle::GetDisplay (this=0x86a2968, aFrame=0x8791770, aValue=@0xbfffce28) at nsComputedDOMStyle.cpp:2322 #3 0x410dbfad in nsComputedDOMStyle::GetPropertyCSSValue (this=0x86a2968, aPropertyName=@0x8437490, aReturn=0xbfffce78) at nsComputedDOMStyle.cpp:515 #4 0x410dba33 in nsComputedDOMStyle::GetPropertyValue (this=0x86a2968, aPropertyName=@0x8437490, aReturn=@0xbfffd024) at nsComputedDOMStyle.cpp:480 #5 0x40268a65 in XPTC_InvokeByIndex (that=0x86a2968, methodIndex=5, paramCount=2, params=0xbfffd0b8) at xptcinvoke_unixish_x86.cpp:153 #6 0x40aef7e6 in XPCWrappedNative::CallMethod (ccx=@0xbfffd160, mode=CALL_METHOD) at xpcwrappednative.cpp:2025 #7 0x40af8227 in XPC_WN_CallMethod (cx=0x8637300, obj=0x86cbe08, argc=1, argv=0x8793bd8, vp=0xbfffd284) at xpcwrappednativejsops.cpp:1266 #8 0x400d5902 in js_Invoke (cx=0x8637300, argc=1, flags=0) at jsinterp.c:788 #9 0x400ed677 in js_Interpret (cx=0x8637300, result=0xbfffdc34) at jsinterp.c:2745 #10 0x400d597d in js_Invoke (cx=0x8637300, argc=1, flags=2) at jsinterp.c:805 #11 0x400d5cdc in js_InternalInvoke (cx=0x8637300, obj=0x84fd7c0, fval=139450336, flags=0, argc=1, argv=0xbfffe0bc, rval=0xbfffddb8) at jsinterp.c:880 looks like GetPrimaryFrameFor gives us a bogus frame and then we crash when we try to GetStyleData() on it.
Summary: crash if repeat changing display of absolutely positioned form element → crash if repeat changing display of absolutely positioned form element [@ nsIFrame::GetStyleData]
OS: All, Crashing on Linux 2002031721. TB4276685X.
OS: Windows 2000 → All
Marking bugs dealing with re-addition to the primary frame map with [PFM].
Summary: crash if repeat changing display of absolutely positioned form element [@ nsIFrame::GetStyleData] → [PFM]crash if repeat changing display of absolutely positioned form element [@ nsIFrame::GetStyleData]
Assignee | ||
Comment 5•22 years ago
|
||
Taking the bug.
Assignee: dbaron → karnaze
Priority: -- → P1
Target Milestone: --- → mozilla1.0
Assignee | ||
Updated•22 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•22 years ago
|
||
Nominating for nsbeta1, since the problem is due to gfx text controls accessing the editor during frame destruction which seems like a serious problem in general.
Keywords: nsbeta1
Comment 7•22 years ago
|
||
nsbeta1+. adt2
Assignee | ||
Comment 8•22 years ago
|
||
The patch adds nsIFrame::RemovedAsPrimaryFrame which is called in nsCSSFrameConstructor::DoDeletingFrameSubtree and DoCleanupFrameReferences. It allows nsGfxTextControlFrame to call its new PreDestroy method which does what was previously being done in Destroy. So, it accesses mEditor before mEditor's anonymous div gets removed from the map.
Assignee | ||
Updated•22 years ago
|
Component: Style System → HTML Form Controls
Assignee | ||
Updated•22 years ago
|
Whiteboard: [adt2] → [adt2]PATCH
Comment 9•22 years ago
|
||
Comment on attachment 77156 [details] [diff] [review] patch to fix the bug sr=attinasi - though I must say I really hate inline virtual methods (yes, I see there are already some in there...)
Attachment #77156 -
Flags: superreview+
Comment 10•22 years ago
|
||
Comment on attachment 77156 [details] [diff] [review] patch to fix the bug Seems good to me, since we are currently guaranteed that a frame is about to be destroyed when it is removed from the primary frame map, and that happens before the sub-frames get messed with. About that mBits array; if you're going to define a bitfield why not put the other bools into it? Also it's a bitfield, not a bool; it should be PRUint8 or whatever else you want to declare it as. In the case of other bools that need to be set to false, you can use macros (it's easier); I made some in nsHTMLInputElement.cpp that should apply here just fine. with one of those, r=jkeiser
Attachment #77156 -
Flags: review+
Updated•22 years ago
|
Summary: [PFM]crash if repeat changing display of absolutely positioned form element [@ nsIFrame::GetStyleData] → [PATCH][PFM]crash if repeat changing display of absolutely positioned form element [@ nsIFrame::GetStyleData]
Whiteboard: [adt2]PATCH → [adt2]
Assignee | ||
Comment 11•22 years ago
|
||
I'll make the minimal change of turning it into a PRUint8 and adding a comment, because I want to reduce the risk. And besides, what you are suggestion should have already been done before my change. The best long term solution is to use the extra bits from the frame state (there are 12 or so high order bits available) and let the existing 3 packed bools and this new use that. This would save 4 bytes from the text control frame. However, I was not 100% sure which if any of those bits were already being used by ancestors of the text control frame.
Assignee | ||
Comment 12•22 years ago
|
||
Attachment #77156 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Attachment #77786 -
Flags: superreview+
Attachment #77786 -
Flags: review+
Comment 13•22 years ago
|
||
Comment on attachment 77786 [details] [diff] [review] revised patch with reviewer's suggestions a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #77786 -
Flags: approval+
Comment 14•22 years ago
|
||
adt1.0.0+ (on ADT's behalf) approval for checkin to 1.0.
Assignee | ||
Comment 15•22 years ago
|
||
The patch is in.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Updated•22 years ago
|
Keywords: fixed1.0.0
Updated•13 years ago
|
Crash Signature: [@ nsIFrame::GetStyleData]
You need to log in
before you can comment on or make changes to this bug.
Description
•