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

Status: VERIFIED FIXED

(Core :: XBL, defect, P1)

Description

[Martijn Wargers (dead)]

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)

Comment 1

[Martijn Wargers (dead)]

Attachment: testcase

Comment 2

[Noa Groveman]

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

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: This should fix it...

Comment 4

[Boris Zbarsky [:bzbarsky]]

I wish I could target 1.8.1 or something... :(

Comment 5

[Brian Ryner (not reading)]

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.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Updated to comments

Comment 7

[Boris Zbarsky [:bzbarsky]]

Fixed.

Comment 8

[Stephen Donner [:stephend] Not actively reading bugmail]

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

Comment 9

[Martijn Wargers (dead)]

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?

Comment 10

[Stephen Donner [:stephend] Not actively reading bugmail]

(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.

Comment 11

[Martijn Wargers (dead)]

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 12

[Boris Zbarsky [:bzbarsky]]

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.

Comment 13

[Daniel Veditz [:dveditz]]

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.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Fixed on branches.

Comment 15

[Jay Patel [:jay]]

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