Closed Bug 822425 Opened 7 years ago Closed 6 years ago

Expose assertSameCompartment outside the JS engine

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28
Tracking Status
firefox27 --- fixed
firefox28 --- fixed
firefox-esr24 --- fixed
b2g-v1.2 --- fixed
b2g-v1.3 --- fixed

People

(Reporter: mccr8, Assigned: bholley)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file, 1 obsolete file)

It would be nice if we had a friend API for this, so XPConnect can use it instead of making ad hoc compartment checking assertions. Probably we only need object and maybe string.

Then I can file a followup to actually use it.
Only the first one will benefit from our Nightly-mode compartment assertions,
but both are useful for consistency.
Attachment #814877 - Flags: review?(wmccloskey)
In the header file, the one with a context shouldn't be turned into an empty function in a non-debug build, or we won't get release mode assertions.  In the cpp file, you should guard the object-object one inside #ifdef DEBUG.
(In reply to Andrew McCreight [:mccr8] from comment #2)
> In the header file, the one with a context shouldn't be turned into an empty
> function in a non-debug build, or we won't get release mode assertions.  In
> the cpp file, you should guard the object-object one inside #ifdef DEBUG.

Oh hm, that's a good point. It's a shame that this means that we'll have a performance penalty in release builds regardless, but it's probably negligible. And anyway, the once we fold mozjs.dll into libxul on windows, LTO should take care of it.
Attachment #814877 - Attachment is obsolete: true
Attachment #814877 - Flags: review?(wmccloskey)
Attachment #814898 - Flags: review?(continuation)
Comment on attachment 814898 [details] [diff] [review]
Expose a few simple compartment assertions in jsfriendapi. v2

Review of attachment 814898 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsfriendapi.h
@@ +483,5 @@
>  JS_FRIEND_API(JSObject *)
>  GetGlobalForObjectCrossCompartment(JSObject *obj);
>  
> +JS_FRIEND_API(void)
> +AssertSameCompartment(JSContext *cx, JSObject *obj);

If you wanted to avoid the call overhead in release builds, you could wrap this in something like |#ifdef JS_CRASH_DIAGNOSTICS or DEBUG|, otherwise define it as empty.  That's what the JS engine uses, and maybe the CRASH flag is defined for browser code, too?  If you do that, it would be good to check.

@@ +487,5 @@
> +AssertSameCompartment(JSContext *cx, JSObject *obj);
> +
> +#ifdef DEBUG
> +JS_FRIEND_API(void)
> +AssertSameCompartment(JSObject *objA, JSObject *objB);

I'll assume you have a use-case for this form.  I don't think the JS engine has any variants without a cx involved. Instead, it has (cx, obj, obj) etc.
Attachment #814898 - Flags: review?(continuation) → review+
https://hg.mozilla.org/mozilla-central/rev/3eb51e31e094
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Assignee: general → bobbyholley+bmo
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.