Closed Bug 317265 Opened 16 years ago Closed 16 years ago

[FIX]Crash when following the steps in this evil testcase, using float:right [@ nsINodeInfo::GetDocument]

Categories

(Core :: XBL, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9alpha1

People

(Reporter: martijn.martijn, Assigned: bzbarsky)

Details

(5 keywords)

Crash Data

Attachments

(2 files, 1 obsolete file)

See upcoming testcase (which is strange html, but that's the only way it seems to crash)

First click on the button, then hover over the red bordered box to get the crash.
If it doesn't crash the first time, reload the document and try it again, it should crash within 3 times normally (it does for me).

It seems a regression, it doesn't crash with a 2005-04-18 build, but crashes with a 2005-04-19 build:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-04-18+08%3A00%3A00&maxdate=2005-04-19+07%3A00%3A00&cvsroot=%2Fcvsroot

I got this backtrace from a debug build:
Program received signal SIGSEGV, Segmentation fault.
0x0599837d in nsINodeInfo::GetDocument() const (this=0xfeeefeee)
---Type <return> to continue, or q <return> to quit---
    at ../../../dist/include/content/nsINodeInfo.h:269
269         return mOwnerManager->GetDocument();
Current language:  auto; currently c++
(gdb) bt
#0  0x0599837d in nsINodeInfo::GetDocument() const (this=0xfeeefeee)
    at ../../../dist/include/content/nsINodeInfo.h:269
#1  0x05997a40 in nsIContent::GetOwnerDoc() const (this=<incomplete type>)
    at ../../../dist/include/content/nsIContent.h:171
#2  0x0578436a in nsXBLBinding::AllowScripts() (this=0xfa88a50)
    at c:/mozilla/mozilla/content/xbl/src/nsXBLBinding.cpp:1211
#3  0x05783105 in nsXBLBinding::ExecuteAttachedHandler() (this=0xfa88a50)
    at c:/mozilla/mozilla/content/xbl/src/nsXBLBinding.cpp:788
#4  0x057830fa in nsXBLBinding::ExecuteAttachedHandler() (this=0xfa88a88)
    at c:/mozilla/mozilla/content/xbl/src/nsXBLBinding.cpp:786
#5  0x057a2666 in nsBindingManager::ProcessAttachedQueue() (this=0xf803a88)
    at c:/mozilla/mozilla/content/xbl/src/nsBindingManager.cpp:789
#6  0x05395415 in nsCSSFrameConstructor::ContentInserted(nsIContent*, nsIContent
*, int, nsILayoutHistoryState*, int) (this=0xfa66300, aContainer=0xf816238,
    aChild=0xfa8e3a8, aIndexInContainer=1, aFrameState=0xfa62290,
    aInReinsertContent=0)
    at c:/mozilla/mozilla/layout/base/nsCSSFrameConstructor.cpp:9457
#7  0x05399e89 in nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*)
    (this=0xfa66300, aContent=0xfa8e3a8)
    at c:/mozilla/mozilla/layout/base/nsCSSFrameConstructor.cpp:11497
#8  0x05397240 in nsCSSFrameConstructor::RestyleElement(nsIContent*, nsIFrame*,
nsChangeHint) (this=0xfa66300, aContent=0xfa8e3a8, aPrimaryFrame=0xfa8cafc,
    aMinHint=0)
    at c:/mozilla/mozilla/layout/base/nsCSSFrameConstructor.cpp:10376
---Type <return> to continue, or q <return> to quit---
#9  0x0539d6ff in nsCSSFrameConstructor::ProcessOneRestyle(nsIContent*, nsReStyl
eHint, nsChangeHint) (this=0xfa66300, aContent=0xfa8e3a8,
    aRestyleHint=eReStyle_Self, aChangeHint=0)
    at c:/mozilla/mozilla/layout/base/nsCSSFrameConstructor.cpp:13210
#10 0x0539d980 in nsCSSFrameConstructor::ProcessPendingRestyles() (
    this=0xfa66300)
    at c:/mozilla/mozilla/layout/base/nsCSSFrameConstructor.cpp:13257
#11 0x0539dc45 in nsCSSFrameConstructor::RestyleEvent::HandleEvent() (
    this=0xfa4dfd8)
    at c:/mozilla/mozilla/layout/base/nsCSSFrameConstructor.cpp:13323
#12 0x0539dc81 in HandleRestyleEvent(PLEvent*) (aEvent=0xfa4dfd8)
    at c:/mozilla/mozilla/layout/base/nsCSSFrameConstructor.cpp:13332
#13 0x009e3826 in PL_HandleEvent (self=0xfa4dfd8)
    at c:/mozilla/mozilla/xpcom/threads/plevent.c:688
#14 0x009e36b7 in PL_ProcessPendingEvents (self=0x2242378)
    at c:/mozilla/mozilla/xpcom/threads/plevent.c:623
#15 0x009e6eab in nsEventQueueImpl::ProcessPendingEvents() (this=0x224aeb8)
    at c:/mozilla/mozilla/xpcom/threads/nsEventQueue.cpp:417
#16 0x6304ca12 in nsWindow::DispatchPendingEvents() (this=0xfa90340)
    at c:/mozilla/mozilla/widget/src/windows/nsWindow.cpp:4118
#17 0x6304dd8b in nsWindow::ProcessMessage(unsigned, unsigned, long, long*) (
    this=0xfa90340, msg=512, wParam=0, lParam=3998701, aRetValue=0x22fac0)
    at c:/mozilla/mozilla/widget/src/windows/nsWindow.cpp:4498
#18 0x630452d9 in nsWindow::WindowProc(HWND__*, unsigned, unsigned, long) (
---Type <return> to continue, or q <return> to quit---\
    hWnd=0x1f03a8, msg=512, wParam=0, lParam=3998701)
    at c:/mozilla/mozilla/widget/src/windows/nsWindow.cpp:1347
#19 0x77d37b17 in USER32!SetWindowPlacement ()
   from /cygdrive/c/WINDOWS/system32/user32.dll
#20 0x001f03a8 in ?? ()
#21 0x00000200 in ?? ()
(gdb) cont
Continuing.

Program received signal SIGSEGV, Segmentation fault.
0x0599837d in nsINodeInfo::GetDocument() const (this=0xfeeefeee)
    at ../../../dist/include/content/nsINodeInfo.h:269
269         return mOwnerManager->GetDocument();
(gdb) cont
Continuing.

Program exited with code 0200.
(gdb)
Attached file testcase
Summary: Crash when following the steps in this evil testcase, using float:right → Crash when following the steps in this evil testcase, using float:right [@ nsINodeInfo::GetDocument]
Confirming with Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8) Gecko/20051120 Firefox/1.5

It crashed after I reloaded once
Attached patch This should fix it... (obsolete) — Splinter Review
Assignee: general → bzbarsky
Status: NEW → ASSIGNED
Attachment #203840 - Flags: superreview?(bryner)
Attachment #203840 - Flags: review?(bryner)
I wish I could target 1.8.1 or something... :(
OS: Windows XP → All
Priority: -- → P1
Hardware: PC → All
Summary: Crash when following the steps in this evil testcase, using float:right [@ nsINodeInfo::GetDocument] → [FIX]Crash when following the steps in this evil testcase, using float:right [@ nsINodeInfo::GetDocument]
Target Milestone: --- → mozilla1.9alpha
Comment on attachment 203840 [details] [diff] [review]
This should fix it...

>--- content/xbl/src/nsBindingManager.cpp	1 Sep 2005 04:38:29 -0000	1.137
>+++ content/xbl/src/nsBindingManager.cpp	21 Nov 2005 21:22:13 -0000
>@@ -340,16 +340,31 @@ nsBindingManager::GetBinding(nsIContent*
>+  if (oldBinding) {
>+    PRInt32 index = mAttachedStack.IndexOf(oldBinding);
>+    if (index != -1) {
>+      mAttachedStack.RemoveElementAt(index);
>+      NS_RELEASE(oldBinding);

if (mAttachedStack.RemoveElement(oldBinding)) {
  NS_RELEASE(oldBinding);
}

? r+sr=me, either way.
Attachment #203840 - Flags: superreview?(bryner)
Attachment #203840 - Flags: superreview+
Attachment #203840 - Flags: review?(bryner)
Attachment #203840 - Flags: review+
Attachment #203840 - Attachment is obsolete: true
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Using testcase: https://bugzilla.mozilla.org/attachment.cgi?id=203772&action=view

this is Verified FIXED using SeaMonkey 1.5a

Mozilla/5.0 (Windows; U; Windows NT5.1; en-US; rv:1.8b5) Gecko/20051006 Firefox/1.5
Status: RESOLVED → VERIFIED
Stephen, I think it's really great that you're doing all this verification work, but your user agent ID from comment is from 2005-10-06, which is a build before this fix.
That's a little odd, isn't it?
(In reply to comment #9)
> Stephen, I think it's really great that you're doing all this verification
> work, but your user agent ID from comment is from 2005-10-06, which is a build
> before this fix.
> That's a little odd, isn't it?

It sure is; thanks for catching that.  I should've explained:  my user agent is overridden by general.useragent.override to use Firefox (to enable me to get in sites that sniff for their user agent).

I didn't realize that setting it in about:config would override what displays in about:

In this case, I'm really using 2005-11-23-10, sorry about that.

(The titlebar doesn't change, but all about: pages do, when using general.useragent.override.)

Thanks for catching that; this bug is still verified.
Boris, is this patch something for 1.8.1 or 1.8.0.1?
Because of what you said in comment 4:
"I wish I could target 1.8.1 or something... :("
Comment on attachment 203933 [details] [diff] [review]
Updated to comments

Yeah, I think we should take this on branches. It's a pretty simple crash fix, and not doing this causes us to make virtual calls on garbage memory, which is never good.
Attachment #203933 - Flags: approval1.8.1?
Attachment #203933 - Flags: approval1.8.0.1?
Comment on attachment 203933 [details] [diff] [review]
Updated to comments

a=dveditz for 1.8/1.8.0.1 branches. Please add the fixed1.8.1 and fixed1.8.0.1 keywords when checked in.
Attachment #203933 - Flags: approval1.8.1?
Attachment #203933 - Flags: approval1.8.1+
Attachment #203933 - Flags: approval1.8.0.1?
Attachment #203933 - Flags: approval1.8.0.1+
Fixed on branches.
Target Milestone: mozilla1.9alpha → mozilla1.8.1
v.fixed on 1.8.0.1 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1, no crash with testcase in comment #1
Target Milestone: mozilla1.8.1 → mozilla1.9alpha
Crash Signature: [@ nsINodeInfo::GetDocument]
You need to log in before you can comment on or make changes to this bug.