1177832 - Add `sandboxPrototype` to loader options

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect)

Description

[James Long (:jlongster)]

We need this for a temporary hack to use loaders in the frontend in devtools. It's just adding a simple option to the loaders so hopefully that's ok. Eventually I'd like to have a better solution for frontend modules, but we may not get that until ES6 modules land.

The `sandboxPrototype` option passed to a loader will be passed to the `prototype` option of the Sandbox that the loader creates. This allows you to sort of evaluate code in the context of another object (that object sort of becomes a "global"). The hack is that it's not *actually* the global, but a prototype of the global so `this !== object`. However, this still works really well for the devtools as a migration path to something better.

Comment 1

[James Long (:jlongster)]

Attachment: 1177832.patch

Comment 2

[Matteo Ferretti [:zer0] [:matteo]]

Comment on attachment 8626654 [details] [diff] [review]
1177832.patch

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

Looks good to me! I'd like to have a small test in `test-loader` to check the new behavior (something similar to 'test console global by default'
or 'test shared globals' where, instead, we check the sandbox's prototype).

Comment 3

[James Long (:jlongster)]

Attachment: 1177832.patch

Attached with a test. I'm going to need your help running this test. Meaning if you could try running it, and fixing any errors, that would be great. It's really tiny so it shouldn't be hard to fix up. I can't get the tests running locally at all.

I'll ping you on IRC tomorrow and we can work on the test.

Comment 4

[Matteo Ferretti [:zer0] [:matteo]]

Comment on attachment 8627366 [details] [diff] [review]
1177832.patch

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

I fixed the two small issue mentioned, and it works fine. I'm going to push on SDK repo on your behalf the fixed patch.

::: addon-sdk/source/test/test-loader.js
@@ +368,5 @@
>  }
>  
> +exports['test prototype of global'] = function (assert) {
> +  let uri = root + '/fixtures/loader/globals/';
> +  let loader = Loader({ paths: { '': uri }, sandboxPrototype: { globalFoo: 5 }});

you forget to use add `sharedGlobal: true`, otherwise the `sandboxPrototype` won't be used.

@@ +371,5 @@
> +  let uri = root + '/fixtures/loader/globals/';
> +  let loader = Loader({ paths: { '': uri }, sandboxPrototype: { globalFoo: 5 }});
> +  let program = main(loader, 'main');
> +
> +  assert.ok(typeof program.globalFoo === 5, '`globalFoo` exists');

I guess the `typeof` is a left over.

Comment 5

[[github robot]]

Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/03391c6fa7c5d95900a3aa6ffbae6c055a2731a7
Bug 1177832 - Add `sandboxPrototype` to loader options

https://github.com/mozilla/addon-sdk/commit/f5ee714b2690284312b22fd0182e9a280b1e8737
Merge pull request #2019 from ZER0/sandbox/1177832

Fix Bug 1177832 - Add `sandboxPrototype` to loader options r=@zer0

Comment 6

[Matteo Ferretti [:zer0] [:matteo]]

I forgot that github robot set the bug to resolved fixed, but you want to wait until you land in m-c maybe? Feel free to change the status.

Comment 7

[James Long (:jlongster)]

Good point. Let's leave it open until it lands on fx-team.

Comment 8

[James Long (:jlongster)]

Matteo, I did end up using this technique. In combination with bug 1180955, I was able to create a loader that *only* handles modules under a specific URI, and delegate the rest to the normal loader. This solves all of my concerns, as only my modules are now eval'ed with the window as a sandbox prototype, making it a normal browser env.

(noting this because recently I talked about doing `require("window")` but the multiple loader trick worked out so well)

Comment 9

[James Long (:jlongster)]

this landed on fx-team in https://github.com/mozilla/gecko-dev/commit/a184de4560179f88771da4b3cb2e12e30780963c