736752 - Compartment mismatch in JetPack 'test-content-proxy.testTypedArrays'

Status: RESOLVED FIXED

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

Description

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

The test at

https://github.com/mozilla/addon-sdk/blob/master/packages/api-utils/tests/test-content-proxy.js#L743

hits a compartment mismatch; see

https://tbpl.mozilla.org/?noignore=1&jobname=jetpack

Hoping for smart people who can tell me what I need to do here.

Comment 1

[Bobby Holley (:bholley)]

crash stack?

Also, when did this start happening?

Comment 2

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to Bobby Holley (:bholley) from comment #1)
> crash stack?
> 
> Also, when did this start happening?

This push had JP tests as permaorange-but-really-greenish: https://tbpl.mozilla.org/?noignore=1&jobname=jetpack&rev=e5f6caa40409

The next push after that turned them permared-but-really-orangish: https://tbpl.mozilla.org/?noignore=1&jobname=jetpack&rev=ee56787a20fb

Comment 3

[Alexandre Poirot [:ochameau]]

The unit test isn't perfect, may be too agressive on its checks.


Jetpack (over) use sandboxes. So in this particular test case we have 3 sandboxes and the content document:
- 1 chrome sandbox hosting the test module (not really important here)
- 1 content document
- 1 content sandbox with same principal than the document, hosting the proxy code
- 1 another similar sandbox, hosting the content script

It looks like we are hitting some wrapper behavior difference. I can't exactly tell if the new behavior is expected or not.
Here is a minimal (scratchpad) testcase that highlights what changed.
let Cu = Components.utils;

// First content sandbox that simulates the proxy code
let s1 = Cu.Sandbox(content, {wantXrays:true});
function foo(doc) {
  let canvas = doc.createElement("canvas");
  let context = canvas.getContext("2d");
  let proxy = context.getImageData(0,0, 1, 1);
  return {
    proxy: proxy,
    get data () proxy.data
  };
}
Cu.evalInSandbox(foo, s1);

// Second content sandbox that simulates the content script code
let s2 = Cu.Sandbox(content, {wantXrays:true});
function bar() {
  // Here, on nightly, it is different.
  // I assume we get some king of wrappers whereas we didn't get any visible wrapper before.
  return obj.proxy.data === obj.data;
}

// Call first sandbox code and give the result back to the second one
s2.obj = s1.foo(content.document);
let rv = Cu.evalInSandbox("("+bar+"())", s2)

// `rv` was true, now it is false
Cu.reportError(">> " + rv);

Comment 4

[Bobby Holley (:bholley)]

(In reply to Alexandre Poirot (:ochameau) from comment #3)
> The unit test isn't perfect, may be too agressive on its checks.

Possibly, but it shouldn't matter - JS should never be able to trigger a compartment mismatch. So it's almost certainly a bug in spidermonkey or XPConnect.

If somebody posts a reduced testcase that hits the cross-compartment assertion, I'll fire up gdb to see what's going on.

Comment 5

[Alexandre Poirot [:ochameau]]

Attachment: Backtrace

(In reply to Bobby Holley (:bholley) from comment #4)
> If somebody posts a reduced testcase that hits the cross-compartment
> assertion, I'll fire up gdb to see what's going on.

The script inlined in comment 3 is such testcase. It hits this assertion.
Here is the backtrace while running it manually in scratchpad.

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

static JSBool
nsIDOMImageData_GetData(JSContext *cx, JSObject *obj, jsid id, jsval *vp)
{
    XPC_QS_ASSERT_CONTEXT_OK(cx);
    nsIDOMImageData *self;
    xpc_qsSelfRef selfref;
    if (!xpc_qsUnwrapThis(cx, obj, &self, &selfref.ptr, vp, nsnull, true))
        return JS_FALSE;
    self->GetData(vp);
    return JS_TRUE;
}

After the call, obj and *vp aren't in the same compartment (according to the bt).

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

That's because GetData needs to JS_WrapValue the retval, I like told you on IRC on Saturday.

Comment 8

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Patch v1

Something like this, perhaps

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/ab2ff3b5611f

Yeah, I did want to push it, but I was still at work then ;)