Similar to other commands it would be good to have some basic support for the script.evaluate command first. We should check what's actually necessary to have once it's a priority - maybe in M4 around April 2022.

Removing the whiteboard flag, as we are moving from milestones to another process. This was the only bugged flagged m4.

The scope for this bug is the following:

• Evaluation of Javascript code in a given browsing context (no realm support)
• No support for returning Javascript data
• By-passing CSP by default
• No support for sandboxes

We should evaluate if using the Debugger API is the way to go and that it supports sandboxes. Note that we already use it in our CDP implementation:

https://searchfox.org/mozilla-central/source/remote/cdp/domains/content/runtime/ExecutionContext.jsm

If it turns out not to be usable (yet) we should fallback to evalInSandbox().

I did some early investigations around the DebuggerObject API.

My stack is at https://treeherder.mozilla.org/jobs?repo=try&revision=0f5e95e93953f1f478554dcdf73d0e7fe511f275

It contains a basic (non spec-compliant) implementation of script.evaluate which can evaluate an expression in content in 3 ways:

• using Debugger executeInGlobal with the actual global
• using Debugger executeInGlobal with a sandbox
• using Cu.evalInSandbox

Observations on executeInGlobal

By default executeInGlobal doesn't run code in any sandbox. If you modify an object from the content page, it will impact the content page. But you can create a debuggee wrapping a sandbox, and then evaluate code in a sandbox (in this patch this.#debugger.addDebuggee(this.#sandbox);).

executeInGlobal will return DebuggerObject instances for non-primitive values. You can get the raw object by calling obj.unsafeDereference().

I also had no issue accessing sessionStorage/localStorage information with any of the three approaches above. So I don't really understand where the current issue with Marionette comes from (Bug 1738436).

Memory investigation

The stack also contains a patch with an additional times parameter to evaluate a given expressions several times.
As an attempt top emulate the object store, each return value is then held in a Map stored in the windowglobal script module.
Finally a memory command is also added to the script module, which returns the residentUnique value from MemoryReporter, after performing a GC.

With that, I ran two memory tests following the same protocol:

• start firefox with --remote-debugging-port
• create a BiDi session
• run browsingContext.getTree to get the id for the existing BC
• navigate the top-level BC to example.com
• call script.memory
• call 10 times script.evaluate with times: 10000 (so in total, we evaluate and store the return values 100 000 times)
• call script.memory

1/ Array with 26 characters: "(['abcdefghijklmnopqrstuvw'])"

• with debugger: 11309056 -> 37765120
• with debugger + sandbox: 11141120 -> 37605376
• with sandbox only: 12939264 -> 44785664

2/ Empty object: "({})"

• with debugger: 11137024 -> 35061760
• with debugger + sandbox: 11126071 -> 35215110
• with sandbox only: -> 11243520 -> 40656896

From that test it seems that Debugger APIs actually have less impact on memory than the raw Cu.executeInGlobal.

Regarding CSPs, it seems like we bypass CSPs whenever we use a sandbox (tried with system principal, null principal and window principal). However for executeInGlobal used without a sandbox, CSPs will apply to the script.

Finally regarding raw performance I timed the execution of my memory tests.
There was no consistent performance difference between any of the configurations I tested. So it feels like whatever performance hit comes from using executeInGlobal might also impact evalInSandbox.

(In reply to Julian Descottes [:jdescottes] from comment #4)

Regarding CSPs, it seems like we bypass CSPs whenever we use a sandbox (tried with system principal, null principal and window principal). However for executeInGlobal used without a sandbox, CSPs will apply to the script.

As discussed in the meeting today, this statement was misleading. CSPs do not prevent from using executeInGlobal successfully.
It will only prevent from evaluating code which contains "eval" or "new Function", similar to what is described in https://bugzilla.mozilla.org/show_bug.cgi?id=1580514
And such code can be evaluated in a sandbox.

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #2)

The scope for this bug is the following:

• Evaluation of Javascript code in a given browsing context (no realm support)
• No support for returning Javascript data
• By-passing CSP by default
• No support for sandboxes

We should evaluate if using the Debugger API is the way to go and that it supports sandboxes. Note that we already use it in our CDP implementation:

https://searchfox.org/mozilla-central/source/remote/cdp/domains/content/runtime/ExecutionContext.jsm

If it turns out not to be usable (yet) we should fallback to evalInSandbox().

Summary of our discussions around this topic. Based on the investigations above, it seems that Debugger APIs are a good fit for us and could pave the way for compatibility with DevTools scenarios in the future. Therefore we will start with those.

Re "By-passing CSP by default", as mentioned in comment #4 and comment #5, CSPs won't fully prevent from using script.evaluate, but they will can still prevent from evaluating some very specific code (basically using eval/new Function). Since the basic implementation will not support sandboxes (which fully bypass CSPs), I assume we are fine with this limitation for now. Maybe we should have a follow up bug, it's not 100% clear if we'll need to lift the limitation or if it's acceptable in the long run.

Also was not listed back when we wrote the comment above, but to be clear awaitPromise will be handled in Bug 1770461.

Quick question Henrik,

I wonder if we should fold Bug 1770476 in here. We already have serialization for primitive types, and this would make it much easier to write tests, if we could at least assert some return value from script evaluate.

What do you think?

You could take both and stack it in phabricator for sure. Would that make sense?

Sure!

Adds a new root script module, and a new windowglobal script module.
The root script module supports the public command evaluate, with the following limitations:

• awaitPromise is not supported
• the RealmTarget type is not supported
• sandbox is not supported for the ContextTarget type
• evaluation return values are not supported
• exception handling is not supported
• ownership model is not supported

One note about using addDebuggee vs using makeGlobalObjectReference: I was trying to implement support for exceptions. In order to fetch details about async errors (basically bug 1770461 + bug 1770477).

Even though I could catch promise rejections by simply awaiting on the promise, the only way I managed to get details such as line/column number, stack etc... was to rely on promiseResolutionSite. Imagine the promise rejected with a simple string (reject("failed")). In that case the error retrieved in a catch() block is just the string "failed" (ie just a primitive). So there's no usable stacktrace, no details etc...

promiseResolutionSite provides all this information, however it seems to only work if I executed the code with a global DebuggerObject created via addDebuggee, not with makeGlobalObjectReference. This might also impact other promise introspection helpers.

https://hg.mozilla.org/mozilla-central/rev/2528596d754a
https://hg.mozilla.org/mozilla-central/rev/8a69c0bf4328