Closed Bug 459095 Opened 14 years ago Closed 4 years ago

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]


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






(Reporter: bzbarsky, Unassigned)


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.
Flags: blocking1.9.1?
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.
#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
#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
#29 0x01b24b58 in nsDocument::DispatchEvent (this=0x154a200, aEvent=0x759e9f0, _r
l=0xbfffcdd0) at /Users/bzbarsky/mozilla/debug/mozilla/content/base/src/nsDocumen
#30 0x01af1db9 in nsContentUtils::DispatchTrustedEvent (aDoc=0x154a200, aTarget=0
a200, aEventName=@0xbfffce80, aCanBubble=1, aCancelable=1, aDefaultAction=0x0) at
#31 0x01b24be2 in nsDocument::DispatchContentLoadedEvents (this=0x154a200) at /Us
#32 0x01b35a2e in nsRunnableMethod<nsDocument>::Run (this=0x758f940) at nsThreadU
#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
#35 0x035d3f53 in nsBaseAppShell::NativeEventCallback (this=0x83e030) at /Users/b
#36 0x0358cca0 in nsAppShell::ProcessGeckoEvents (aInfo=0x83e030) at /Users/bzbar
#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
#46 0x04065026 in nsAppStartup::Run (this=0x858740) at /Users/bzbarsky/mozilla/de
#47 0x000e3d53 in XRE_main (argc=4, argv=0xbfffe600, aAppData=0x80eca0) at /Users
#48 0x000027cb in main (argc=4, argv=0xbfffe600) at /Users/bzbarsky/mozilla/debug

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?
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...
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.
What do we think the security risk is here?
I'm not sure if there's a security risk here. It's more of a risk of misbehavior (properties being lost, etc).
That, plus general interfering with testing...
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 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.
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.
AFAIK (and mrbkap agrees) there's no security risk here, so we're not going to block on this.
Assignee: nobody → jonas
Flags: wanted1.9.2+
Flags: blocking1.9.1?
Flags: blocking1.9.1-
Priority: -- → P1
Component: Content → DOM
QA Contact: content → general
Severity: normal → major
Priority: P1 → P2
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.
Assignee: jonas → nobody
Flags: needinfo?(mrbkap)
I suspect that this is a non-issue with WebIDL bindings, because we no longer try to wrap globals as non-globals.
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.
Closed: 4 years ago
Flags: needinfo?(mrbkap)
Resolution: --- → DUPLICATE
Duplicate of bug: 670324
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.