17736 - DOM objects crash on non DOM JSContext

Status: VERIFIED FIXED

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

Description

[John Bandhauer]

As discussed in email... The DOM objects created by idlc and some of those
handwritten assume that the JS_ContextPrivate will always cough up a
nsJSContext. This will not be true when bug 13419 is fixed (which is ready to
go).

Brendan suggested a system whereby we walk up the object parent tree to the
global object, checking that object's class to verify that it has a private of
type nsISupports (JSCLASS_HAS_PRIVATE | JSCLASS_PRIVATE_IS_NSISUPPORTS), and
then QI'ing that private to nsIScriptGlobalObject (right?).

We will also what to offload some stuff from nsJSContext into that global object
interface. e.g. the security manager and namespace manager references and all
that entails.

I checked in my changes for bug 13419. They are disabled using a comment until
this is fixed. They can be used for testing. in xpcwrappedjsclass.cpp do the
following in the two places where 'AutoPushCompatibleJSContext' appears...

< //    AutoPushCompatibleJSContext autoContext(GetJSContext());
< //    JSContext* cx = autoContext.GetJSContext();
<     JSContext* cx = GetJSContext();
---
>     AutoPushCompatibleJSContext autoContext(GetJSContext());
>     JSContext* cx = autoContext.GetJSContext();
> //    JSContext* cx = GetJSContext();

With this change you should see crashing in (at least) the mail compose window
on document end load.

I have some changes to the xpconnect test components that uses a wrapped JS
object call to nsITimer to trigger this bug in a simpler case. I'll get this in
soon and then attach the html to use the component to this bug.

So who is up for doing the work? I'm busy doing an xpconnect refactoring to fix
its version of this 'JSContext is the locus object' problem. I can do this too
if no one else is going to do it. Brendan has had his nose in this code
and claimed that he might do this work if no one else volunteers. vidur, please
reassign if you'd rather not attack this yourself.

Comment 1

[John Bandhauer]

Attachment: test that uses NT only timer to trigger problem

Comment 2

[John Bandhauer]

I attached the test html that will trigger the calling of a JS function on a non
-DOM context (after that changes to xpcwrappedjsclass.cpp noted above).

This code uses a method on the nsIEcho test object implementation in the
xpctest components which is unfortunately only implemented for Win32 - the
nsITimer stuff is in a static lib built in widgetland and caused build order
problems when I tried to link it into the DLL made in xpconnect/tests/components
on a clobber build on Unix. For NT it works fine.

Comment 3

[vidur (gone)]

Spoke to jband about this at great length. Our options include:
1) Creating a service for JSContext creation. All JSContexts created by the
service would match the assumptions made by the DOM.
2) Have the DOM prime the thread-local context stack and make sure everyone uses
it.
3) Do the right thing and remove all calls to JS_ContextPrivate and take some of
the other steps suggested by jband.

Since the last step will involve a substantial change to both generated and
hand-written code, jband and I decided to defer it for a bit. Reassigning to
jband to hold the bug until we get around to dealing with it. He might consider
one of the other options in the interim.

Comment 4

[Brendan Eich [:brendan]]

I'm branching JS, XUL and DOM for the rest of brutal sharing.  I'll take a stab
at moving stuff out of context and into window private, if it's ok with y'all.

/be

Comment 5

[Brendan Eich [:brendan]]

Hope these deps are true -- yell if not.

/be

Comment 6

[John Bandhauer]

removed the dependency for 18702. I fixed this.

Comment 7

[John Bandhauer]

Today's checkins to DOM and xpconnect code fix this. My attached timer test
now works.

Comment 8

[Prashant Desale]

Verified according to Jbands comments.