Let the jetpack kernel create sandboxes in the jetpack process

RESOLVED FIXED

Status

Mozilla Labs
Jetpack Prototype
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Benjamin Smedberg, Assigned: Benjamin Smedberg)

Tracking

Details

Attachments

(1 attachment)

19.31 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
ted
: review+
Details | Diff | Splinter Review
(Assignee)

Description

8 years ago
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

8 years ago
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.
(Assignee)

Updated

8 years ago
Duplicate of this bug: 574039
(Assignee)

Comment 3

8 years ago
Created attachment 456458 [details] [diff] [review]
.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.
Attachment #456458 - Flags: review?(ted.mielczarek)
Attachment #456458 - Flags: review?(bent.mozilla)
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.
Attachment #456458 - Flags: review?(bent.mozilla) → review+
(Assignee)

Comment 5

8 years ago
http://hg.mozilla.org/mozilla-central/rev/053bf6cd63e9
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
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.
Attachment #456458 - Flags: review?(ted.mielczarek) → review+
Filed bug 582598 on adding a workaround to xpcshell.
You need to log in before you can comment on or make changes to this bug.