459095 - Assertions about incorrect class flags from nsNodeSH::PreCreate [###!!! ASSERTION: Non-global object has the wrong flags: '!(jsclazz->flags & JSCLASS_IS_GLOBAL)', js/src/xpconnect/src/xpcwrappednative.cpp, line 899]

Status: RESOLVED DUPLICATE of bug 670324

(Core :: DOM: Core & HTML, defect, P2)

Description

[Boris Zbarsky [:bzbarsky]]

nsNodeSH::PreCreate wraps the parent.  In this case, the node is a document, so the parent is a window.  doc->GetJSObject() is null.  So we make the WrapNative call, which tries to wrap the window as a non-globa, and hence asserts:

###!!! ASSERTION: Non-global object has the wrong flags: '!(jsclazz->flags & JSCLASS_IS_GLOBAL)', file /Users/bzbarsky/mozilla/debug/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 899

Not sure what the upshot here is....

The testcase is the testcase from bug 458637.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

bz: What's the stack? The assertion there assumes that we'll always create (and hold onto) the window's JS object, so that call should never create a new wrapped native JSObject.

Comment 2

[Boris Zbarsky [:bzbarsky]]

#5  0x00a53e95 in XPCWrappedNative::Init (this=0x75a4330, ccx=@0xbfffa818, parent=0x6fca60, isGlobal=0, sci=0xbfffa6ac) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp:898
#6  0x00a54b4a in XPCWrappedNative::GetNewOrUsed (ccx=@0xbfffa818, Object=0x7599d6c, Scope=0x87fb4a0, Interface=0x85b880, isGlobal=0, resultWrapper=0xbfffa76c) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp:485
#7  0x00a25d9c in XPCConvert::NativeInterface2JSObject (ccx=@0xbfffa818, dest=0xbfffa8f4, src=0x7599d6c, iid=0x2180c5c, scope=0x6fca60, allowNativeWrapper=0, isGlobal=0, pErr=0xbfffa8ac) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/xpconnect/src/xpcconvert.cpp:1096
#8  0x00a03013 in nsXPConnect::WrapNative (this=0x82dc50, aJSContext=0x14ebc00, aScope=0x6fca60, aCOMObj=0x7599d6c, aIID=@0x2180c5c, _retval=0xbfffa8f4) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/xpconnect/src/nsXPConnect.cpp:1254
#9  0x01dffb45 in nsDOMClassInfo::WrapNative (cx=0x14ebc00, scope=0x6fca60, native=0x7599d6c, aIID=@0x2180c5c, vp=0xbfffa974, aHolder=0xbfffa970) at /Users/bzbarsky/mozilla/debug/mozilla/dom/src/base/nsDOMClassInfo.cpp:1626
#10 0x01e01678 in nsNodeSH::PreCreate (this=0x85f920, nativeObj=0x154a200, cx=0x14ebc00, globalObj=0x6fca60, parentObj=0xbfffaa88) at /Users/bzbarsky/mozilla/debug/mozilla/dom/src/base/nsDOMClassInfo.cpp:6895
#11 0x00a545f2 in XPCWrappedNative::GetNewOrUsed (ccx=@0xbfffac5c, Object=0x154a2c0, Scope=0x87fb4a0, Interface=0x458ad70, isGlobal=0, resultWrapper=0xbfffab4c) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp:403
#12 0x00a25d9c in XPCConvert::NativeInterface2JSObject (ccx=@0xbfffac5c, dest=0xbfffabf4, src=0x154a2c0, iid=0xabb634, scope=0x4433540, allowNativeWrapper=1, isGlobal=0, pErr=0xbfffabf0) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/xpconnect/src/xpcconvert.cpp:1096
#13 0x00a70966 in xpc_qsXPCOMObjectToJsval (ccx=@0xbfffac5c, p=0x154a2c0, iid=@0xabb634, rval=0xbfffb950) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/xpconnect/src/xpcquickstubs.cpp:729
#14 0x00a94142 in nsIDOMEvent_GetTarget (cx=0x14ebc00, obj=0x4433540, id=7132516, vp=0xbfffb950) at dom_quickstubs.cpp:5518
#15 0x0025c650 in js_NativeGet (cx=0x14ebc00, obj=0x4433540, pobj=0x44fd200, sprop=0x128d8f0, vp=0xbfffb950) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/jsobj.cpp:3624
#16 0x0025d415 in js_GetPropertyHelper (cx=0x14ebc00, obj=0x4433540, id=7132516, vp=0xbfffb950, entryp=0xbfffb884) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/jsobj.cpp:3773
#17 0x0021d608 in js_Interpret (cx=0x14ebc00) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/jsinterp.cpp:4276
#18 0x00247352 in js_Invoke (cx=0x14ebc00, argc=1, vp=0x10dd820, flags=0) at jsinterp.cpp:1324
#19 0x00a4aff4 in nsXPCWrappedJSClass::CallMethod (this=0xb576ab0, wrapper=0x754d750, methodIndex=3, info=0x1116cd0, nativeParams=0xbfffc894) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp:1523
#20 0x00a42bcd in nsXPCWrappedJS::CallMethod (this=0x754d750, methodIndex=3, info=0x1116cd0, params=0xbfffc894) at /Users/bzbarsky/mozilla/debug/mozilla/js/src/xpconnect/src/xpcwrappedjs.cpp:565
#21 0x00474df0 in PrepareAndDispatch (self=0x754d230, methodIndex=3, args=0xbfffc9b4) at /Users/bzbarsky/mozilla/debug/mozilla/xpcom/reflect/xptcall/src/md/unix/xptcstubs_unixish_x86.cpp:93
#22 0x00474e4f in nsXPTCStubBase::Stub3 (this=0x754d230) at xptcstubsdef.inc:1
#23 0x01bc62b9 in nsEventListenerManager::HandleEventSubType (this=0x75a4a50, aListenerStruct=0x75a4a8c, aListener=0x754d230, aDOMEvent=0x759e9f0, aCurrentTarget=0x154a200, aPhaseFlags=6) at /Users/bzbarsky/mozilla/debug/mozilla/content/events/src/nsEventListenerManager.cpp:1079
#24 0x01bc7e9c in nsEventListenerManager::HandleEvent (this=0x75a4a50, aPresContext=0x0, aEvent=0xc21fab0, aDOMEvent=0xbfffcc98, aCurrentTarget=0x154a200, aFlags=6, aEventStatus=0xbfffcc9c) at /Users/bzbarsky/mozilla/debug/mozilla/content/events/src/nsEventListenerManager.cpp:1193
#25 0x01bfbb62 in nsEventTargetChainItem::HandleEvent (this=0x1504e20, aVisitor=@0xbfffcc90, aFlags=6, aMayHaveNewListenerManagers=1) at /Users/bzbarsky/mozilla/debug/mozilla/content/events/src/nsEventDispatcher.cpp:236
#26 0x01bfbda2 in nsEventTargetChainItem::HandleEventTargetChain (this=0x1504fa0, aVisitor=@0xbfffcc90, aFlags=6, aCallback=0x0, aMayHaveNewListenerManagers=1) at /Users/bzbarsky/mozilla/debug/mozilla/content/events/src/nsEventDispatcher.cpp:300
#27 0x01bfc5f8 in nsEventDispatcher::Dispatch (aTarget=0x154a200, aPresContext=0x0, aEvent=0xc21fab0, aDOMEvent=0x759e9f0, aEventStatus=0xbfffcd70, aCallback=0x0) at /Users/bzbarsky/mozilla/debug/mozilla/content/events/src/nsEventDispatcher.cpp:514
#28 0x01bfc936 in nsEventDispatcher::DispatchDOMEvent (aTarget=0x154a200, aEvent=
 aDOMEvent=0x759e9f0, aPresContext=0x0, aEventStatus=0xbfffcd70) at /Users/bzbars
ozilla/debug/mozilla/content/events/src/nsEventDispatcher.cpp:576
#29 0x01b24b58 in nsDocument::DispatchEvent (this=0x154a200, aEvent=0x759e9f0, _r
l=0xbfffcdd0) at /Users/bzbarsky/mozilla/debug/mozilla/content/base/src/nsDocumen
p:6080
#30 0x01af1db9 in nsContentUtils::DispatchTrustedEvent (aDoc=0x154a200, aTarget=0
a200, aEventName=@0xbfffce80, aCanBubble=1, aCancelable=1, aDefaultAction=0x0) at
ers/bzbarsky/mozilla/debug/mozilla/content/base/src/nsContentUtils.cpp:3029
#31 0x01b24be2 in nsDocument::DispatchContentLoadedEvents (this=0x154a200) at /Us
bzbarsky/mozilla/debug/mozilla/content/base/src/nsDocument.cpp:3832
#32 0x01b35a2e in nsRunnableMethod<nsDocument>::Run (this=0x758f940) at nsThreadU
.h:264
#33 0x00459a44 in nsThread::ProcessNextEvent (this=0x815d00, mayWait=0, result=0x
cfb4) at /Users/bzbarsky/mozilla/debug/mozilla/xpcom/threads/nsThread.cpp:510
#34 0x003e33b8 in NS_ProcessPendingEvents_P (thread=0x815d00, timeout=20) at nsTh
Utils.cpp:180
#35 0x035d3f53 in nsBaseAppShell::NativeEventCallback (this=0x83e030) at /Users/b
sky/mozilla/debug/mozilla/widget/src/xpwidgets/nsBaseAppShell.cpp:121
#36 0x0358cca0 in nsAppShell::ProcessGeckoEvents (aInfo=0x83e030) at /Users/bzbar
mozilla/debug/mozilla/widget/src/cocoa/nsAppShell.mm:302
#37 0x923b665f in CFRunLoopRunSpecific ()
#38 0x923b6cf8 in CFRunLoopRunInMode ()
#39 0x9664e480 in RunCurrentEventLoopInMode ()
#40 0x9664e299 in ReceiveNextEventCommon ()
#41 0x9664e10d in BlockUntilNextEventMatchingListInMode ()
#42 0x914463ed in _DPSNextEvent ()
#43 0x91445ca0 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:
#44 0x9143ecdb in -[NSApplication run] ()
#45 0x0358b656 in nsAppShell::Run (this=0x83e030) at /Users/bzbarsky/mozilla/debu
zilla/widget/src/cocoa/nsAppShell.mm:591
#46 0x04065026 in nsAppStartup::Run (this=0x858740) at /Users/bzbarsky/mozilla/de
mozilla/toolkit/components/startup/src/nsAppStartup.cpp:182
#47 0x000e3d53 in XRE_main (argc=4, argv=0xbfffe600, aAppData=0x80eca0) at /Users
arsky/mozilla/debug/mozilla/toolkit/xre/nsAppRunner.cpp:3263
#48 0x000027cb in main (argc=4, argv=0xbfffe600) at /Users/bzbarsky/mozilla/debug
illa/browser/app/nsBrowserApp.cpp:156

In this case the document being wrapped is an nsHTMLDocument.

Note that in frame 10:

(gdb) frame 10
#10 0x01e01678 in nsNodeSH::PreCreate (this=0x85f920, nativeObj=0x154a200, cx=0x14ebc00, globalObj=0x6fca60, parentObj=0xbfffaa88) at /Users/bzbarsky/mozilla/debug/mozilla/dom/src/base/nsDOMClassInfo.cpp:6895
6895                               getter_AddRefs(holder));
(gdb) p nativeObj
$2 = (nsHTMLDocument *) 0x154a200
(gdb) p ((nsHTMLDocument*)nativeObj)->GetWindow()->mDocument.mRawPtr
$5 = (class nsIDOMDocument *) 0x1573890
(gdb) set $x = ((nsHTMLDocument*)nativeObj)->GetWindow()->mDocument.mRawPtr
(gdb) whats $x
0x21e6de8 <_ZTV13nsXMLDocument+1096>:   0x1cc54e4 <_ZThn144_N13nsXMLDocument14QueryInterfaceERK4nsIDPPv>

So perhaps we should be clearing the HTML document's window when we swap it out of the document viewer and replace it with the XSLT result document?

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Shouldn't the JS object for the window exist already since we must have called SetNewDocument (and thus nsIXPConnect::InitClassesWithNewWrappedGlobal) on our window? I wonder why I can't reproduce this...

Comment 4

[Boris Zbarsky [:bzbarsky]]

You saved the testcase from bug 458637 to local disk and ran it there, right?

And yes, I'd think the JS object for the window should already exist... I have no idea why it doesn't.  I can reliably reproduce with a clean m-c build.

Comment 5

[Johnny Stenback (:jst)]

What do we think the security risk is here?

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

I'm not sure if there's a security risk here. It's more of a risk of misbehavior (properties being lost, etc).

Comment 7

[Boris Zbarsky [:bzbarsky]]

That, plus general interfering with testing...

Comment 8

[Boris Zbarsky [:bzbarsky]]

Blake and I just poked at this some more.  We're firing DOMContentLoaded on the HTML document that got replaced by the XSLT result.  This HTML document has a non-null mScriptGlobalObject, which is an inner window.  This is NOT the current inner for the outer, though: the current inner's document is the XSLT result.

In any case, we dispatch the event, have a script global, so the event propagates to that, then to the chrome event handler, and then the password manager tries to get event.target and we lose.  Or at least we lose if the async runnable that fires DOMContentLoaded races against us really tearing down this document no one's holding refs to and wins the race.

I suspect (and Blake seems to agree) that just doing more teardown on the HTML doc (setting the script global object to null, calling RemovedFromDocshell()) when setting up the XSLT result doc would be good here.

Comment 9

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

For what it's worth, we have bugs on the fact that the inner window gets replaced when XSLT does its magic document-swap. The problem is that firebug has installed event listeners on the old inner window, to listen to when the document finishes loading etc. These don't work since the inner window is swapped out.

So eventually we'll probably try to keep the old inner window when doing this swap.

Not sure if that affects things here though.

Comment 10

[Johnny Stenback (:jst)]

AFAIK (and mrbkap agrees) there's no security risk here, so we're not going to block on this.

Comment 11

[Andrew Overholt [:overholt]]

Blake, do we still care about this? I'm going to guess Jonas isn't going to work on this so I'll unassign him.

Comment 12

[Boris Zbarsky [:bzbarsky]]

I suspect that this is a non-issue with WebIDL bindings, because we no longer try to wrap globals as non-globals.

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

It looks like this testcase required Firebug to assert. If I run this testcase with the current Firefox developer tools active, I run into bug 670324, which makes sense as we've completely changed how DOM bindings work (which would fix this bug) but I don't think anybody's looked at what sicking was talking about in comment 9. Duping this bug over as the other bug has some work towards the latter problem.