Closed
Bug 395331
Opened 17 years ago
Closed 17 years ago
Crash [@ nsTextFrame::Destroy] with <xul:arrowscrollbox>
Categories
(Core :: Layout: Text and Fonts, defect)
Tracking
()
VERIFIED
FIXED
People
(Reporter: jruderman, Assigned: roc)
References
Details
(Keywords: assertion, crash, testcase, Whiteboard: [sg:critical?])
Crash Data
Attachments
(2 files)
333 bytes,
application/xhtml+xml
|
Details | |
6.40 KB,
patch
|
bzbarsky
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
Loading the testcase (Mac trunk debug Firefox) triggers an assertion: ###!!! ASSERTION: Gap or overlap in textframes mapping content?!: 'endOfLastContent == contentStart', file /Users/jruderman/trunk/mozilla/layout/generic/nsTextFrameThebes.cpp, line 1590 Leaving the testcase triggers a crash. The stack trace depends on how I leave: 1) Quit the browser (Cmd+Q): Thread 0 Crashed: 0 libthebes.dylib 0x059f052f nsAutoRefCnt::operator unsigned() const + 9 (nsISupportsImpl.h:214) 1 libthebes.dylib 0x059f2976 gfxTextRunFactory::Release() + 26 (gfxFont.h:419) ... 2) Reload after a while: ###!!! ASSERTION: Object is lying about its index: 'generation.Length() > index && generation[index] == aObj', file ../../dist/include/xpcom/nsExpirationTracker.h, line 153 Thread 0 Crashed: 0 libgklayout.dylib 0x17b17117 nsTextFrame::ClearTextRun() + 95 (nsTextFrameThebes.cpp:3561) 1 libgklayout.dylib 0x17b1781f nsTextFrame::Destroy() + 17 (nsTextFrameThebes.cpp:3249) ... 3) Reload immediately: Thread 0 Crashed: 0 <<00000000>> 0x00000000 0 + 0 1 libgklayout.dylib 0x17b1781f nsTextFrame::Destroy() + 17 (nsTextFrameThebes.cpp:3249) 2 libgklayout.dylib 0x17abe0f8 nsFrameList::DestroyFrames() + 48 (nsFrameList.cpp:68) 3 libgklayout.dylib 0x17aa7649 nsContainerFrame::Destroy() + 73 (nsContainerFrame.cpp:255) ...
Flags: blocking1.9?
Reporter | ||
Comment 1•17 years ago
|
||
Assignee | ||
Updated•17 years ago
|
Assignee: nobody → roc
Reporter | ||
Updated•17 years ago
|
Whiteboard: [sg:critical?]
Assignee | ||
Comment 2•17 years ago
|
||
The problem is that when we remove the text node from the DOM, the frame isn't removed because we can't find primary frame for the node. Here's the relevant fragment of the frame tree: Box(arrowscrollbox)(1)@0x24cae38 next=0x24cb44c {0,-60,0,0} [state=80c40000] [content=0x3dfe44c0] [sc=0x246a15c]< ButtonBoxFrame(autorepeatbutton)(-1)@0x24ab724 next=0x24cb114 {120,60,0,0} [state=80400000] [content=0x3dfe45d0] [sc=0x246a3e8]< ImageBox(image)(-1)@0x24cafc0 {0,0,0,0} [content=0x3e41bcf0] > XULScroll(scrollbox)(-1)@0x24cb114 [view=0x3e407b70] next=0x24cb540 {0,0,0,0} [state=80c42000] [content=0x3dfe4740] [sc=0x24cb050]< Box(scrollbox)(-1)@0x24cb0ac [view=0x3e407c10] {0,0,0,0} [state=80c02000] [content=0x3dfe4740] [sc=0x24cb208] pst=:-moz-scrolled-content< Box(box)(-1)@0x24cb350 {0,0,0,0} [state=80c00000] [content=0x3e42de00] [sc=0x24cb2f4]< Text(-1)@0x246a300[0,1,T] {0,0,0,0} [content=0x3dfe4510] sc=0x24cb3fc pst=:-moz-non-element< "\n" > > > > ButtonBoxFrame(autorepeatbutton)(-1)@0x24cb540 {120,60,0,0} [state=80400000] [content=0x3dfe4770] [sc=0x24cb4dc]< ImageBox(image)(-1)@0x24cb650 {0,0,0,0} [content=0x3e41bed0] > > Relevant part of the call stack: #0 nsCSSFrameConstructor::FindPrimaryFrameFor (this=0x3e424ca0, aFrameManager=0x239c41c, aContent=0x3dfe4510, aFrame=0xbfffa938, aHint=0x0) at /Users/roc/mozilla-checkin/mozilla/layout/base/nsCSSFrameConstructor.cpp:10796 But the relevant locals are: (gdb) p parentFrame $37 = (nsIFrame *) 0x24cae38 (gdb) p aContent $38 = (nsIContent *) 0x3dfe4510 (gdb) p aContent->GetParent() $39 = (nsIContent *) 0x3dfe44c0 So the text node's parent is the arrowscrollbox. But the FindPrimaryFrameFor code is expecting the parent to be the inner box (0x3e42de00). I can't remember whether nsIContent::GetParent() should return the parent in the real DOM or the shadow DOM... if the real DOM, then how is FindPrimaryFrameFor supposed to work for children of XBL insertion points?
Comment 3•17 years ago
|
||
For non-anonymous nodes, nsIContent::GetParent() returns the real DOM parent. > how is FindPrimaryFrameFor supposed to work parentContent ends up being the arrowscrollbox, so parentFrame is its primary frame. Then we call FindFrameWithContent, and that steps down through kids that have parentContent as the GetBindingParent(). See patch for bug 54524. So I'd expect this to work, yes...
Assignee | ||
Comment 4•17 years ago
|
||
OK, FindFrameWithContent is not finding the text node because it refuses to descend from the scrollbox into the box. The reason is this check: if (aParentContent == kidContent || (aParentContent && (aParentContent == kidContent->GetBindingParent()))) Here, when kidContent is the scrollbox's child (0x24cb350), its binding parent is the scrollbox because of the nested binding. But aParentContent is the arrowscrollbox, so this check fails and we don't descend. I think we could probably check whether aParentContent is a binding *ancestor* of kidContent. I'll make a patch to that effect.
Assignee | ||
Comment 5•17 years ago
|
||
This fixes it and I think it makes sense.
Attachment #281293 -
Flags: superreview?(bzbarsky)
Attachment #281293 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•17 years ago
|
Attachment #281293 -
Flags: approval1.9?
Comment 6•17 years ago
|
||
Comment on attachment 281293 [details] [diff] [review] fix >Index: layout/base/nsCSSFrameConstructor.cpp >+IsBindingAncestor(nsIContent* aContent, nsIContent* aBindingRoot) >+ nsIContent* bindingParent = aContent->GetBindingParent(); >+ if (!bindingParent) >+ return PR_FALSE; >+ if (bindingParent == aBindingRoot) >+ return PR_TRUE; >+ aContent = bindingParent; I think you want another clause there after the == check: if (bindingParent == aContent) return PR_FALSE; Otherwise passing in a native-anonymous aContent will send you into an infinite loop, no? r+sr=bzbarsky with that change.
Attachment #281293 -
Flags: superreview?(bzbarsky)
Attachment #281293 -
Flags: superreview+
Attachment #281293 -
Flags: review?(bzbarsky)
Attachment #281293 -
Flags: review+
Updated•17 years ago
|
Assignee: roc → nobody
Component: GFX: Thebes → Layout: Fonts and Text
QA Contact: thebes → layout.fonts-and-text
Updated•17 years ago
|
Assignee: nobody → roc
Assignee | ||
Updated•17 years ago
|
Flags: blocking1.9? → blocking1.9+
Assignee | ||
Comment 7•17 years ago
|
||
checked in with that change.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•17 years ago
|
Attachment #281293 -
Flags: approval1.9?
Assignee | ||
Updated•17 years ago
|
Flags: in-testsuite+
Comment 8•16 years ago
|
||
verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2008011009 Firefox/3.0b3pre ID:2008011009 while using the testcase. No Assertion or crash when loading the testcase -> Verified fixed
Status: RESOLVED → VERIFIED
Reporter | ||
Comment 9•16 years ago
|
||
I don't see any assertions/crashes on branch with this testcase.
Group: security
Flags: wanted1.8.1.x-
Updated•13 years ago
|
Crash Signature: [@ nsTextFrame::Destroy]
You need to log in
before you can comment on or make changes to this bug.
Description
•