495883 - "ASSERTION: Unable to get an nsIScriptGlobalObject from the ScriptGlobalObjectOwner!"

Status: RESOLVED FIXED

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

Description

[Jesse Ruderman]

Attachment: stack trace

###!!! ASSERTION: Unable to get an nsIScriptGlobalObject from the ScriptGlobalObjectOwner!: 'global', file dom/src/jsurl/nsJSProtocolHandler.cpp, line 166

The page script is calling cloneNode.  Based on the stack, I'd guess the subtree being cloned includes a frame whose src attribute is a javascript: URL.

I can't reproduce this in a way that allows me to make a reduced testcase :(  sicking looked at this a little but thought bz would understand it better.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Yikes.  

So we're cloning a document.  That's fine.  But then we create a frameloader for the frame that happens to be in that document!  We should almost certainly be checking whether the document has a container before doing that.

We need to fix this on 1.9.1 too, imo, though I wouldn't block initial release on it...

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase

Oh, this is extra fun.  The first load (from inside BindToTree) fails because in the frame loader we go to CheckForRecursiveLoad(), which ends up in nsFrameLoader::EnsureDocShell, which can't get a win_private from the docshell (because the docshell has no treeowner), so returns error.  But then we set the src attr node from the attr map on the new element, which triggers another load attempt, and this time mDocShell is non-null (even though it was never properly set up), so we go ahead and try to do the load in it.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase that doesn't involve cloneNode on documents

There are variants of this (e.g. getting a document out of an iframe before removing that iframe from the DOM) that also don't need the "createDocument can create HTML documents" thing.  Heck, I could probably toss an <html:iframe> into a random XML document...

I would have thought our data document content policy would have prevented these documents from doing loads at all, though.  Why is it not catching them?  Do we not flag them as data documents?

Comment 4

[Johnny Stenback (:jst)]

I think we should try to get to this one for 1.9.2.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

We shouldn't even create the frame loader if it is not needed.

Error handling is simplified a bit so that OOM isn't any different than
for example data document handling.

Posted the patch to tryserver.

I filed a followup bug 511084 to figure out what to do with
nsGenericHTMLFrameElement::EnsureFrameLoader(). It has the !GetParent check.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

http://hg.mozilla.org/mozilla-central/rev/853aa4000eaa

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/24b30660a3c1