1815381 - Support DOM eager evaluation allowlist in worker

Status: RESOLVED FIXED

(DevTools :: Console, defect)

Description

[Tooru Fujisawa [:arai]]

In bug 1806598, we're going to apply eager evaluation allowlist also to getter calls, instead of treating all getter calls side-effect-free.

Then, we don't support DOM allowlist in worker because we generate the allowlist from sandbox, which is not available on worker thread.
which means all DOM getters are blocked in eager evaluation.

There was one testcase which relied on the DOM getters in worker:

https://searchfox.org/mozilla-central/rev/188d0f76a73e0671d12e744a71e9f5701668cc37/devtools/client/webconsole/test/browser/browser_console_evaluation_context_selector.js#110,126-128

setInputValue(hud, "globalThis.location.href");
...
await waitFor(() =>
  evaluationContextSelectorButton.innerText.includes(workerFile)
);

This requires adding WorkerGlobalScope and WorkerLocation getters into allowlist.
(For now, I'm going to modify the testcase not to rely on DOM getters in eager evaluation)

Then, in order to support eager-evaluation allowlist in worker, we need a way to generate the allowlist without creating sandbox.

Possible solution I can think of is to create ChromeUtils method that takes a function and checks if the function is side-effect-free.
That means, move the allowlist creation and check from devtools JS code to native ChromeUtils method.

ChromeUtlls is accessible also on worker thread, and comparing native function pointer and JSJitInfo pointer doesn't require creating objects.

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: Bug 1815381 - Part 1: Use DebuggerObject.prototype.isNativeGetterWithJitInfo in eager evaluation. r?nchevobbe!

Depends on D175088

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: Bug 1815381 - Part 2: Revert workaround for WorkerLocation.prototype.href getter. r?nchevobbe!

Depends on D175089

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: Bug 1815381 - Part 3: Remove DOM getters from explicit allowlist. r?nchevobbe!

Depends on D175090

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Bug 1815381 - Part 4: Add tests with random native getters. r?nchevobbe!

Depends on D175091

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

Thanks a lot arai for working on this. I have positive feedback on the whole patch stack, so feel free to put them in review so I can have a thorough look :)

Comment 6

[Alexandre Poirot [:ochameau]]

The current patch queue seems to fix both bug 1818180 and bug 1822662. Should we resolve them as duplicated bugs?

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/37482f544447
Part 1: Use DebuggerObject.prototype.isNativeGetterWithJitInfo in eager evaluation. r=nchevobbe,devtools-reviewers,ochameau
https://hg.mozilla.org/integration/autoland/rev/58a13013137a
Part 2: Revert workaround for WorkerLocation.prototype.href getter. r=nchevobbe,devtools-reviewers,ochameau
https://hg.mozilla.org/integration/autoland/rev/2dd3c94ff852
Part 3: Remove DOM getters from explicit allowlist. r=nchevobbe,devtools-reviewers,ochameau
https://hg.mozilla.org/integration/autoland/rev/e69bb7d10d11
Part 4: Add tests with random native getters. r=nchevobbe,devtools-reviewers,ochameau

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The following field has been copied from duplicate bugs:

Field	Value	Source
Regressed by	bug 1806598	bug 1818180 and bug 1822662

For more information, please visit BugBot documentation.

Comment 11

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/37482f544447
https://hg.mozilla.org/mozilla-central/rev/58a13013137a
https://hg.mozilla.org/mozilla-central/rev/2dd3c94ff852
https://hg.mozilla.org/mozilla-central/rev/e69bb7d10d11

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1806598

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:arai, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox114 to wontfix.

For more information, please visit BugBot documentation.