Closed Bug 344434 Opened 19 years ago Closed 18 years ago

Crash [@ nsContentUtils::ComparePosition] involving <xul:toolbar> and <xul:toolbarpalette>

Categories

(Core :: XUL, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: jruderman, Unassigned)

References

Details

(Keywords: crash, testcase)

Crash Data

Attachments

(1 file, 1 obsolete file)

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].
Attached file testcase (deleted) —
Blocks: 344486
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
Attached patch Patch rev. 1 (obsolete) (deleted) — Splinter Review
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
(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
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.
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.
The real issue is that the constructor is running at an unsafe time (inside frame construction, in fact). We have existing bugs about that...
I'm guessing you mean bug 267833, "Fire XBL constructors from EndUpdate(), not before".
Depends on: 267833
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]
nsContentUtils::ComparePosition is a trunk topcrash (#2) but I don't know if the topcrash is due to this bug.
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
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?
(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]
(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]
> 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!
Flags: blocking1.9?
I filed bug 356748 with the information that was here so we at least have a bug for the topcrash.
Flags: blocking1.9? → blocking1.9+
Doesn't crash now (probably due to the patch in bug 267833).
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Flags: in-testsuite?
Crashtest checked in.
Flags: in-testsuite? → in-testsuite+
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
Crash Signature: [@ nsContentUtils::ComparePosition]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: