401569 - Crash [nsINode::HasSlots] with flash embed, bindings and changing styles

Status: VERIFIED FIXED

(Core :: XBL, defect, P3)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, which crashes current trunk build after 200ms for me.
This seems to have regressed between 2007-10-19 and 2007-10-20, so I guess a regression from bug 398135, somehow.

Breakpad doesn't come up, so I guess the crash itself is flash related.

Comment 1

[Martijn Wargers (dead)]

Attachment: testcase2

The first testcase doesn't seem to crash online. This one should crash, using data urls.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Basic issue:

#0  0xb6bb7ba2 in ~nsXBLBinding (this=0x8cc77e0)
    at ../../../../mozilla/content/xbl/src/nsXBLBinding.cpp:290
...
#4  0xb6bd7479 in nsXBLService::FlushStyleBindings (this=0x84a0de8, aContent=0x8928ad0)
    at ../../../../mozilla/content/xbl/src/nsXBLService.cpp:617
...
#11 0xb6743776 in PresShell::RecreateFramesFor (this=0x8789548, aContent=0x8928ad0)
    at ../../../mozilla/layout/base/nsPresShell.cpp:3337
#12 0xb69fe5bc in nsObjectLoadingContent::EnsureInstantiation (this=0x8928aec, 
    aInstance=0xbfffcfdc)
    at ../../../../mozilla/content/base/src/nsObjectLoadingContent.cpp:689
#13 0xb6c8c3c5 in nsHTMLPluginObjElementSH::GetPluginInstance (this=0x8cca018, 
    wrapper=0x8ccd098, _result=0xbfffcfdc)
    at ../../../../mozilla/dom/src/base/nsDOMClassInfo.cpp:8623
#14 0xb6c8da70 in nsHTMLPluginObjElementSH::NewResolve (this=0x8cca018, 
    wrapper=0x8ccd098, cx=0x88a5f60, obj=0xafe1f340, id=-1311515364, flags=0, 
    objp=0xbfffd080, _retval=0xbfffd084)
    at ../../../../mozilla/dom/src/base/nsDOMClassInfo.cpp:9095
...
#17 0xb7f726d7 in js_LookupProperty (cx=0x88a5f60, obj=0xafe1f340, id=-1311515364, 
    objp=0xbfffd25c, propp=0xbfffd258) at ../../../mozilla/js/src/jsobj.c:3268
#18 0xb7f74a28 in js_DeleteProperty (cx=0x88a5f60, obj=0xafe1f340, id=-1311515364, 
    rval=0xbfffd2c8) at ../../../mozilla/js/src/jsobj.c:3941
...
#22 0xb6bbaab1 in nsXBLBinding::ChangeDocument (this=0x8cc77e0, aOldDocument=0x8a69a88, 
    aNewDocument=0x0) at ../../../../mozilla/content/xbl/src/nsXBLBinding.cpp:1130
#23 0xb6bd745e in nsXBLService::FlushStyleBindings (this=0x84a0de8, aContent=0x8928ad0)
    at ../../../../mozilla/content/xbl/src/nsXBLService.cpp:613

So we're reentering FlushStyleBindings for the same binding, and crashing when we unwind.  Of course we're also reentering frame construction...

What _really_ needs to happen here is to not resolve in this case.  Which means fixing bug 400794.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

This was fixed by the patch in bug 400705, right?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Yeah.

Comment 5

[Martijn Wargers (dead)]

Attachment: backtrace from debug build

I'm sorry, I'm still crashing with testcase2 (I should have told that bug 400705 didn't fix it for me).
Breakpad doesn't come up for me with the crash. This is a backtrace of the crash from my debug build.

Comment 6

[Martijn Wargers (dead)]

gklayout.dll!nsINode::HasSlots()  Line 649 + 0x3 bytes	C++
 	gklayout.dll!nsINode::UnsetFlags(unsigned long aFlagsToUnset=8192)  Line 619 + 0x8 bytes	C++
 	gklayout.dll!nsXBLBinding::ChangeDocument(nsIDocument * aOldDocument=0x05f570a8, nsIDocument * aNewDocument=0x00000000)  Line 1136	C++
 	gklayout.dll!nsXBLService::FlushStyleBindings(nsIContent * aContent=0x063510a0)  Line 612	C++
 	gklayout.dll!nsXBLService::LoadBindings(nsIContent * aContent=0x063510a0, nsIURI * aURL=0x04d950a0, nsIPrincipal * aOriginPrincipal=0x0663bd38, int aAugmentFlag=0, nsXBLBinding * * aBinding=0x0012e380, int * aResolveStyle=0x0012e354)  Line 523	C++
 	gklayout.dll!nsCSSFrameConstructor::ConstructFrameInternal(nsFrameConstructorState & aState={...}, nsIContent * aContent=0x063510a0, nsIFrame * aParentFrame=0x05fbbe0c, nsIAtom * aTag=0x00f65698, int aNameSpaceID=0, nsStyleContext * aStyleContext=0x05fbbdbc, nsFrameItems & aFrameItems={...}, int aXBLBaseTag=0)  Line 7698 + 0x53 bytes	C++
 	gklayout.dll!nsCSSFrameConstructor::ConstructFrame(nsFrameConstructorState & aState={...}, nsIContent * aContent=0x063510a0, nsIFrame * aParentFrame=0x05fbbe0c, nsFrameItems & aFrameItems={...})  Line 7653 + 0x35 bytes	C++
 	gklayout.dll!nsCSSFrameConstructor::ContentInserted(nsIContent * aContainer=0x0635a678, nsIContent * aChild=0x063510a0, int aIndexInContainer=1, nsILayoutHistoryState * aFrameState=0x05f647c0)  Line 9190	C++
 	gklayout.dll!nsCSSFrameConstructor::RecreateFramesForContent(nsIContent * aContent=0x063510a0)  Line 11297 + 0x25 bytes	C++
 	gklayout.dll!nsCSSFrameConstructor::RestyleElement(nsIContent * aContent=0x063510a0, nsIFrame * aPrimaryFrame=0x05fbbf04, nsChangeHint aMinHint=0)  Line 10128	C++
 	gklayout.dll!nsCSSFrameConstructor::ProcessOneRestyle(nsIContent * aContent=0x063510a0, nsReStyleHint aRestyleHint=eReStyle_Self, nsChangeHint aChangeHint=0)  Line 13189	C++
etc...

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Hmm.. that LoadBindings is flushing here seems really bad, right?

Comment 8

[Boris Zbarsky [:bzbarsky]]

Huh?  LoadBindings doesn't flush...

Comment 9

[Boris Zbarsky [:bzbarsky]]

Anyway, I was on crack.  Bug 400794 couldn't have fixed this.  The lookup is happening from inside the delete, and the lookup itself is enough to cause breakage; we're not running fields.

We really do need bug 400794 fixed here.

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Qué!

Are you saying that Bug 400794 would or would not help here? Or did you mean to say "bug 400705" at one of the two places above.

Comment 11

[Boris Zbarsky [:bzbarsky]]

I meant "bug 400705" the first time, sorry.  Fixing bug 400794 really should fix it.  I'm just waiting for the jsapi changes to land, and then it's easy to do.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Fixed by checkin for bug 400794.

Comment 13

[Martijn Wargers (dead)]

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007111605 Minefield/3.0b2pre