443869 - Fix some request model badness in nsDOMClassInfo

Status: RESOLVED FIXED

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

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch

Working on bug 437152 I found that nsDOMClassInfo needs two JSAutoRequests and one JSAutoSuspendRequest (when switching contexts on the stack). Patch attached.

Comment 1

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Needs the patch for bug 443870.

Comment 2

[Johnny Stenback (:jst)]

Comment on attachment 328294 [details] [diff] [review]
Patch

- In nsWindowSH::NewResolve():

+      JSAutoRequest ar(cx);
+
       *_retval = (::JS_ValueToId(cx, id, &interned_id) &&
                   OBJ_LOOKUP_PROPERTY(cx, innerObj, interned_id, &pobj,
                                       &prop));

I don't see any context switching code above this, so I'm wondering if this request really should be in the code that ends up calling into here w/o actually being in a request, rather than here?

+        JSAutoRequest ar(cx);
+
         *_retval = ::JS_DefineElement(cx, obj, JSVAL_TO_INT(id), JSVAL_VOID,
                                       nsnull, nsnull, 0);

Same thing here.
 
r- based on that, the rest looks good. If this turns out to be the right thing to do, then that's fine as long as we understand (and comment) why the auto request needs to be here.

Comment 3

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch

I can no longer hit the request level assertion there so I don't know what's going on. Let's just hope it magically got fixed ;)

Comment 4

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Pushed changeset 6083213f3de2.