568698 - Let the jetpack kernel create sandboxes in the jetpack process

Status: RESOLVED FIXED

(Mozilla Labs :: Jetpack Prototype, defect)

Description

[Benjamin Smedberg]

Currently we create two fixed globals in the jetpack process for jetpack code and addon code. This actually isn't going to work well with the commonjs module system, so we want to instead expose a primitive .createSandbox function so that the jetpack kernel (cuddlefish) code can create an arbitrary number of sandboxes.

Proposed API is: 

globalObject = createSandbox();

And then we need the ability to eval a string into the sandbox. I'm taking this bug, but I'm not sure (yet) how to implement the eval bit, so I'd love code-pointers.

Comment 1

[Atul Varma [:atul]]

Not sure what kind of code pointers you're looking for, but I've found the source for Cu.evalInSandbox() useful--though it's covered in XPConnect goop, there are useful bits of JSAPI in there:

  http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/src/xpccomponents.cpp#3550

Also useful may be Pydermonkey's implementation of context.evaluate_script():

  http://code.google.com/p/pydermonkey/source/browse/src/context.cpp#704

Not sure how to get around potential COW issues, though; my guess would be that the returned object from createSandbox() would be wrapped, and that anything attached to/from it would get implicity wrapped as necessary, but I guess it's a question for mrbkap.

Comment 3

[Benjamin Smedberg]

Attachment: .createSandbox and .evalInSandbox, rev. 1

Ted, could you just check the runxpcshelltests.py change here? I have a test which throws an exception, which normally would make xpcshell return with code 3, but this is intentional and so I think we should explicitly exit(0). Exceptions that occur during the "root" xpcshell/head.js runtime are still thrown up to the top and have a failure return code.

Comment 4

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

Comment on attachment 456458 [details] [diff] [review]
.createSandbox and .evalInSandbox, rev. 1

>+ReportJetpackErrors(JSContext* cx, const char* message, JSErrorReport* report)

I can't remember the exact circumstances but you're not guaranteed to get a non-null report here so that could crash.

>+JetpackChild::CreateSandbox(JSContext* cx, uintN argc, jsval* vp)
> ...
>+  if (!JS_InitStandardClasses(cx, obj))
>+    return JS_FALSE;
>+  return JS_TRUE;

Just |return JS_InitStandardClasses(...);|

>+  JSObject* obj;
>+  if (!JSVAL_IS_OBJECT(argv[0]) ||
>+      !(obj = JSVAL_TO_OBJECT(argv[0])) ||
>+      &sGlobalClass != JS_GetClass(cx, obj)) {
>+    JS_ReportError(cx, "The first argument to evalInSandbox must be a global object created using createSandbox.");

Hm. I think you should probably also check that obj != JS_GetGlobalObject(cx) since the sandbox uses the same jsclass as the global?

>+  return JS_EvaluateUCScript(cx, obj, JS_GetStringChars(str), JS_GetStringLength(str), "", 1, ignored.addr());

Not sure what the rules are for this module but keeping < 80 chars seems to be file style.

>+registerReceiver("test sandbox", function() {
>+  var addon = createSandbox();
>+  ok(typeof(addon) == "object", "typeof(addon)");
>+  ok("Date" in addon, "addon.Date exists");
>+  // ok(true, "dummy test");
>+  // ok(addon.Date !== Date, "Date objects are different");

Yank these?

r=me with those addressed.

Comment 5

[Benjamin Smedberg]

http://hg.mozilla.org/mozilla-central/rev/053bf6cd63e9

Comment 6

[(not currently active) Ted Mielczarek]

Comment on attachment 456458 [details] [diff] [review]
.createSandbox and .evalInSandbox, rev. 1

This is unfortunate but it'll do for now. I'll file a followup on fixing xpcshell so we can do something smarter.

Comment 7

[(not currently active) Ted Mielczarek]

Filed bug 582598 on adding a workaround to xpcshell.