Closed
Bug 344434
Opened 19 years ago
Closed 18 years ago
Crash [@ nsContentUtils::ComparePosition] involving <xul:toolbar> and <xul:toolbarpalette>
Categories
(Core :: XUL, defect)
Core
XUL
Tracking
()
RESOLVED
FIXED
People
(Reporter: jruderman, Unassigned)
References
Details
(Keywords: crash, testcase)
Crash Data
Attachments
(1 file, 1 obsolete file)
(deleted),
application/xhtml+xml
|
Details |
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060707 Minefield/3.0a1
The testcase causes:
###!!! ASSERTION: don't pass null: 'aNode1 && aNode2', file /Users/admin/trunk/mozilla/content/base/src/nsContentUtils.cpp, line 1230
followed by a null-deref crash [@ nsContentUtils::ComparePosition].
Reporter | ||
Comment 1•19 years ago
|
||
Comment 2•19 years ago
|
||
We hit this line, which calculates the insert index to be at the end:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/content/base/src/nsGenericElement.cpp&rev=3.489&root=/cvsroot&mark=2846,3047#2805
Then we end up here:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/content/base/src/nsContentList.cpp&rev=3.91&root=/cvsroot&mark=592#553
with aNewIndexInContainer == count.
Strange that we haven't seen this until now...
Assignee: nobody → mats.palmgren
OS: Mac OS X 10.4 → All
Hardware: Macintosh → All
Comment 3•19 years ago
|
||
Attachment #229127 -
Flags: superreview?(bugmail)
Attachment #229127 -
Flags: review?(bugmail)
Comment on attachment 229127 [details] [diff] [review]
Patch rev. 1
This doesn't seem right. For one you will still assert, right?
The index should be fine, by the time the notification goes out we should have already appended the new child so it should exist at the given index. See
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/content/base/src/nsGenericElement.cpp&rev=3.489&root=/cvsroot&mark=2390,2402#2372
Sorry, that link was incorrect, this one's better:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/content/base/src/nsGenericElement.cpp&rev=3.489&root=/cvsroot&mark=2390,2418#2387
Comment 6•19 years ago
|
||
(In reply to comment #4)
> (From update of attachment 229127 [details] [diff] [review] [edit])
> This doesn't seem right. For one you will still assert, right?
No assert since the test allowed index == length, but that's not important...
> The index should be fine, by the time the notification goes out we should
> have already appended the new child so it should exist at the given index.
Right, I see what you mean. There is something funky going on here...
The content tree actually changes while we notify the observers!
Here's a bit of ad hoc tracing:
=============================================================================================
nsGenericElement::doInsertChildAt: aDocument=0x848d978 aIndex=4 childCount=4
aParent= body@0x883c718 onload="setTimeout(foopy, 300);" intrinsicstate=[00000000] refcount=6<
Text@0x88e6700 refcount=2<\u000a >
div@0x883c878 intrinsicstate=[00000000] refcount=5<
Text@0x883c8a0 refcount=2<A>
div@0x837dbd0 intrinsicstate=[00000000] refcount=5<
>
>
Text@0x883c8f0 refcount=2<\u000a>
<XUL toolbarpalette@0x88a1480>
>
aKid=toolbar@0x8873128
aParent= body@0x883c718 onload="setTimeout(foopy, 300);" intrinsicstate=[00000000] refcount=6<
Text@0x88e6700 refcount=2<\u000a >
div@0x883c878 intrinsicstate=[00000000] refcount=5<
Text@0x883c8a0 refcount=2<A>
div@0x837dbd0 intrinsicstate=[00000000] refcount=5<
>
>
Text@0x883c8f0 refcount=2<\u000a>
<XUL toolbarpalette@0x88a1480>
<XUL toolbar@0x8873128>
>
aParent->GetChildCount() == 5
nsNodeUtils::ContentAppended: aContainer=0x883c718 childCount=5 aNewIndexInContainer=4
[Switching to Thread 16384 (LWP 6755)]
Breakpoint 1, nsNodeUtils::IMPL_MUTATION_NOTIFICATION_FW_A(nsINode*, nsIDocument*, nsIContent*, int) (content_=0x883c718, document=0x848d978,
aContainer=0x883c718, aNewIndexInContainer=4) at nsNodeUtils.cpp:88
88 printf("4 + 5\n");
(gdb) n
4 + 5
91 if (aContainer) printf("BEFORE: Observer->ContentAppended aContainer=%p aContainer->GetChildCount()=%d aNewIndexInContainer=%d\n", aContainer, aContainer->GetChildCount(),aNewIndexInContainer);
(gdb) br nsBindingManager::ContentAppended
Breakpoint 2 at 0x413c8705: file nsBindingManager.cpp, line 1148.
(gdb) br nsXMLPrettyPrinter::ContentAppended
Breakpoint 3 at 0x4135159b: file nsXMLPrettyPrinter.cpp, line 249.
(gdb) br nsXULDocument::ContentAppended
Breakpoint 4 at 0x413d4f5f: file nsXULDocument.cpp, line 978.
(gdb) br nsContentList::ContentAppended
Breakpoint 5 at 0x411fddcf: file nsContentList.cpp, line 558.
(gdb) br nsHTMLDocument::ContentAppended
Breakpoint 6 at 0x4132bdb1: file nsHTMLDocument.cpp, line 1204.
(gdb) br txMozillaXSLTProcessor::ContentAppended
Breakpoint 7 at 0x413a9191: file txMozillaXSLTProcessor.cpp, line 1187.
(gdb) br nsXPathResult::ContentAppended
Breakpoint 8 at 0x413694f9: file nsXPathResult.cpp, line 233.
(gdb) br nsXMLEventsManager::ContentAppended
Breakpoint 9 at 0x412b2ea1: file nsXMLEventsManager.cpp, line 403.
(gdb) br nsTreeContentView::ContentAppended
Breakpoint 10 at 0x4148b6ba: file nsTreeContentView.cpp, line 921.
(gdb) br PresShell::ContentAppended
Breakpoint 11 at 0x41048c92: file nsPresShell.cpp, line 5290.
(gdb) br nsImageMap::ContentAppended
Breakpoint 12 at 0x410ae9cf: file nsImageMap.cpp, line 971.
(gdb) br inDOMView::ContentAppended
Breakpoint 13 at 0x414d4b4e: file inDOMView.cpp, line 777.
(gdb) c
Continuing.
BEFORE: Observer->ContentAppended aContainer=0x883c718 aContainer->GetChildCount()=5 aNewIndexInContainer=4
Breakpoint 6, nsHTMLDocument::ContentAppended(nsIDocument*, nsIContent*, int) (this=0x848d978, aDocument=0x848d978, aContainer=0x883c718,
aNewIndexInContainer=4) at nsHTMLDocument.cpp:1204
1204 NS_ASSERTION(aDocument == this, "unexpected doc");
(gdb) c
Continuing.
AFTER: Observer->ContentAppended aContainer=0x883c718 aContainer->GetChildCount()=5 aNewIndexInContainer=4
BEFORE: Observer->ContentAppended aContainer=0x883c718 aContainer->GetChildCount()=5 aNewIndexInContainer=4
Breakpoint 2, nsBindingManager::ContentAppended(nsIDocument*, nsIContent*, int) (this=0x883f110, aDocument=0x848d978, aContainer=0x883c718,
aNewIndexInContainer=4) at nsBindingManager.cpp:1148
1148 if (aNewIndexInContainer == -1 || !mContentListTable.ops)
(gdb) c
Continuing.
AFTER: Observer->ContentAppended aContainer=0x883c718 aContainer->GetChildCount()=5 aNewIndexInContainer=4
BEFORE: Observer->ContentAppended aContainer=0x883c718 aContainer->GetChildCount()=5 aNewIndexInContainer=4
Breakpoint 11, PresShell::ContentAppended(nsIDocument*, nsIContent*, int) (this=0x8395970, aDocument=0x83959ec, aContainer=0x883c718, aNewIndexInContainer=4)
at nsPresShell.cpp:5290
5290 NS_PRECONDITION(!mIsDocumentGone, "Unexpected ContentAppended");
(gdb) n
5291 NS_PRECONDITION(aDocument == mDocument, "Unexpected aDocument");
(gdb) n
5293 if (!mDidInitialReflow) {
(gdb) n
5297 WillCauseReflow();
(gdb) n
5301 mFrameConstructor->ContentAppended(aContainer, aNewIndexInContainer);
(gdb) s
nsCSSFrameConstructor::ContentAppended(nsIContent*, int) (this=0x8846a90, aContainer=0x88cc228, aNewIndexInContainer=4) at nsCSSFrameConstructor.cpp:8769
8769 AUTO_LAYOUT_PHASE_ENTRY_POINT(mPresShell->GetPresContext(), FrameC);
(gdb) n
[ ... boring stuff deleted ...]
(gdb)
9031 LAYOUT_PHASE_TEMP_EXIT();
(gdb) call dumpContent("",aContainer)
body@0x88cc228 onload="setTimeout(foopy, 300);" intrinsicstate=[00000000] refcount=6<
Text@0x82b9350 refcount=2<\u000a >
div@0x883c850 intrinsicstate=[00000000] refcount=5<
Text@0x883c878 refcount=2<A>
div@0x86f1fc0 intrinsicstate=[00000000] refcount=5<
>
>
Text@0x883c8c8 refcount=2<\u000a>
<XUL toolbarpalette@0x880d978>
<XUL toolbar@0x88c5680>
>
(gdb) p aContainer->GetChildCount()
$4 = 5
(gdb) n
9032 mDocument->BindingManager()->ProcessAttachedQueue();
(gdb) bt 3
#0 nsCSSFrameConstructor::ContentAppended(nsIContent*, int) (this=0x8846a90, aContainer=0x88cc228, aNewIndexInContainer=4) at nsCSSFrameConstructor.cpp:9032
#1 0x41048d11 in PresShell::ContentAppended(nsIDocument*, nsIContent*, int) (this=0x88454a0, aDocument=0x2, aContainer=0x88cc228, aNewIndexInContainer=4)
at nsPresShell.cpp:5301
#2 0x4125a1e9 in nsNodeUtils::IMPL_MUTATION_NOTIFICATION_FW_A(nsINode*, nsIDocument*, nsIContent*, int) (content_=0x88cc228, document=0x85fdd48,
aContainer=0x88cc228, aNewIndexInContainer=4) at nsNodeUtils.cpp:92
(More stack frames follow...)
(gdb) n
9033 LAYOUT_PHASE_TEMP_REENTER();
(gdb) n
9036 if (!state.mPseudoFrames.IsEmpty()) {
(gdb) call dumpContent("",aContainer)
body@0x88cc228 onload="setTimeout(foopy, 300);" intrinsicstate=[00000000] refcount=7<
Text@0x82b9350 refcount=5<\u000a >
div@0x883c850 intrinsicstate=[00000000] refcount=6<
Text@0x883c878 refcount=2<A>
div@0x86f1fc0 intrinsicstate=[00000000] refcount=5<
>
>
Text@0x883c8c8 refcount=5<\u000a>
<XUL toolbar@0x88c5680>
>
(gdb) p aContainer->GetChildCount()
$5 = 4
(gdb) c
Continuing.
AFTER: Observer->ContentAppended aContainer=0x883c718 aContainer->GetChildCount()=4 aNewIndexInContainer=4
BEFORE: Observer->ContentAppended aContainer=0x883c718 aContainer->GetChildCount()=4 aNewIndexInContainer=4
Breakpoint 5, nsContentList::ContentAppended(nsIDocument*, nsIContent*, int) (this=0x88b5778, aDocument=0x848d978, aContainer=0x883c718,
aNewIndexInContainer=4) at nsContentList.cpp:558
558 NS_PRECONDITION(aContainer, "Can't get at the new content if no container!");
(gdb) c
Continuing.
aNewIndexInContainer=4 count=4
=============================================================================================
as you can see the <XUL toolbarpalette@0x88a1480> was removed by
nsCSSFrameConstructor::ContentAppended() when executing this block:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/base/nsCSSFrameConstructor.cpp&rev=1.1242&root=/cvsroot&mark=9031-9033#9028
As an experiment I removed this block (and the one in
nsCSSFrameConstructor::ContentInserted) and then added calls
to "document->BindingManager()->ProcessAttachedQueue()" in the nsNodeUtils
notification macros (after all observers have been called). That fixed it...
Not sure what the real solution would be though...
Assignee: mats.palmgren → nobody
Updated•19 years ago
|
Attachment #229127 -
Attachment is obsolete: true
Attachment #229127 -
Flags: superreview?(bugmail)
Attachment #229127 -
Flags: review?(bugmail)
Ouch, tricky. I wonder why we're starting to run into this now, but presumably it somehow is my changes to implement nsIMutationObserver?
Sounds like what happens is that an XBL constructor modifies the tree outside of the binding in ways we don't deal well with. They really should not do that.
Reporter | ||
Comment 8•19 years ago
|
||
I think this regressed between 2006-05-16 and 2005-05-17, but I'm not as sure about that regression window as I'd like to be.
> Sounds like what happens is that an XBL constructor modifies the tree outside
> of the binding in ways we don't deal well with. They really should not do that.
Do you mean "XBL binding constructors should not be *allowed* to do that"? Because untrusted content can provide XBL bindings too.
Comment 9•19 years ago
|
||
The real issue is that the constructor is running at an unsafe time (inside frame construction, in fact). We have existing bugs about that...
Reporter | ||
Comment 10•19 years ago
|
||
I'm guessing you mean bug 267833, "Fire XBL constructors from EndUpdate(), not before".
Depends on: 267833
Comment 11•19 years ago
|
||
Yeah
Comment 12•18 years ago
|
||
Hi all, I'm not sure if the crash I have since few days (3-4) are due to this bug or if it's another issue. Here are some TB: TB22457547H, TB22448463M.
I'm seeing those crash when typing "inurl:bleh" in Googlebar Lite 4.2 search-box
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060824 Minefield/3.0a1 ID:2006082404 [cairo]
Reporter | ||
Comment 13•18 years ago
|
||
nsContentUtils::ComparePosition is a trunk topcrash (#2) but I don't know if the topcrash is due to this bug.
Comment 14•18 years ago
|
||
Getting this one a lot recently (not shure if its related but its a crash @ nsContentUtils::ComparePosition)
TB22807231E, TB22807286M, TB22777222Q, TB22777200G, TB22777154E
Always seems to happen when you type:
the meaning of life, the universe and everything
... into Google for some odd reason.
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060901 Minefield/3.0a1
Comment 15•18 years ago
|
||
Ryan, if you can reproduce consistently, can you see if the regression range for the crash you're seeing is the same as the one Jesse mentioned in comment 8?
Comment 16•18 years ago
|
||
(In reply to comment #14)
> Getting this one a lot recently (not shure if its related but its a crash @
> nsContentUtils::ComparePosition)
>
> TB22807231E, TB22807286M, TB22777222Q, TB22777200G, TB22777154E
>
> Always seems to happen when you type:
>
> the meaning of life, the universe and everything
>
> ... into Google for some odd reason.
>
> Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060901
> Minefield/3.0a1
>
I'm experiencing the same problem but having a hard time isolating it. When I search for "test" in the search box with Google selected as my search engine I crash. When I search for "test" with any other search engine (i.e. Wikipedia) everything works accordingly. When I create a new profile and search for "test" with Google selected everything works too. So it looks like some extension I have installed or something. I am going to try and disable each and test further.
Adblock Filterset.G Updater 0.3.0.4
Adblock Plus 0.7.1.2
CustomizeGoogle 0.50
Deepest Sender 0.7.7
DOM Inspector 1.9a1
DownThemAll! 0.9.9.6.3
Flashblock 1.5.2
IE View Lite 1.2.5
Nightly Tester Tools 1.1
refspoof 0.7.9
Talkback 3.0a1
Translate 0.6.0.9
BTW, it doesn't need to be "test", I originally experienced the error searching for something else, I'm just using that as my example.
TB23000954E, TB23000700X, TB23000687W
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060906 Minefield/3.0a1 ID:2006090605 [cairo]
Comment 17•18 years ago
|
||
(In reply to comment #16)
> (In reply to comment #14)
> > Getting this one a lot recently (not shure if its related but its a crash @
> > nsContentUtils::ComparePosition)
> >
> > TB22807231E, TB22807286M, TB22777222Q, TB22777200G, TB22777154E
> >
> > Always seems to happen when you type:
> >
> > the meaning of life, the universe and everything
> >
> > ... into Google for some odd reason.
> >
> > Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060901
> > Minefield/3.0a1
> >
>
> I'm experiencing the same problem but having a hard time isolating it. When I
> search for "test" in the search box with Google selected as my search engine I
> crash. When I search for "test" with any other search engine (i.e. Wikipedia)
> everything works accordingly. When I create a new profile and search for
> "test" with Google selected everything works too. So it looks like some
> extension I have installed or something. I am going to try and disable each
> and test further.
>
> Adblock Filterset.G Updater 0.3.0.4
> Adblock Plus 0.7.1.2
> CustomizeGoogle 0.50
> Deepest Sender 0.7.7
> DOM Inspector 1.9a1
> DownThemAll! 0.9.9.6.3
> Flashblock 1.5.2
> IE View Lite 1.2.5
> Nightly Tester Tools 1.1
> refspoof 0.7.9
> Talkback 3.0a1
> Translate 0.6.0.9
>
> BTW, it doesn't need to be "test", I originally experienced the error searching
> for something else, I'm just using that as my example.
>
> TB23000954E, TB23000700X, TB23000687W
>
> Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060906
> Minefield/3.0a1 ID:2006090605 [cairo]
>
CustomizeGoogle is the culprit. Would this be considered an extension issue, or a Firefox issue? I did not have this problem prior to Gecko/20060906 so it seems like something was changed that caused the extension to start failing.
Here is the crash that I isolated as being directly caused by CustomizeGoogle 0.50. Immediately following this crash I disabled CustomizeGoogle, restarted Firefox and attempted the exact same search without error.
TB23001468E
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060907 Minefield/3.0a1 ID:2006090704 [cairo]
Comment 18•18 years ago
|
||
> Would this be considered an extension issue, or a Firefox issue?
Depends. We'd need to see it in a debugger to tell, generally.
It sounds like you're seeing a slightly different problem than the originally reported bug here (different steps to reproduce, different regression range). Could you please file a separate bug on it and cc me? In that bug, please provide steps to reproduce starting with a vanilla profile (including detailed steps on installing the CustomizeGoogle extension). Assume that you're asking someone who doesn't normally use Firefox to reproduce (which is in fact the case), so please describe any UI operations involved clearly without assumptions about UI knowledge (e.g. "click in the textbox on the top right of the browser window" rather than "click in the search box").
Thanks!
Updated•18 years ago
|
Flags: blocking1.9?
Comment 19•18 years ago
|
||
I filed bug 356748 with the information that was here so we at least have a bug for the topcrash.
Updated•18 years ago
|
Flags: blocking1.9? → blocking1.9+
Reporter | ||
Comment 20•18 years ago
|
||
Doesn't crash now (probably due to the patch in bug 267833).
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Reporter | ||
Updated•18 years ago
|
Flags: in-testsuite?
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
Assignee | ||
Updated•14 years ago
|
Crash Signature: [@ nsContentUtils::ComparePosition]
You need to log in
before you can comment on or make changes to this bug.
Description
•