Closed Bug 1328307 Opened 3 years ago Closed 5 months ago

Should AutoNoJSAPI do more?

Categories

(Core :: DOM: Core & HTML, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox53 --- wontfix
firefox70 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(1 file)

Things I'm a bit surprised it doesn't do:

1)  Enter a null compartment.
2)  Hide the scripted caller.

Specifically, putting an AutoNoJSAPI on the stack when the JS stack is nonempty does absolutely nothing to change the value returned by IncumbentGlobal(), afaict.  Seems wrong.
Flags: needinfo?(bobbyholley)
Yeah, it should probably do both those things.
Flags: needinfo?(bobbyholley)
Priority: -- → P3
Component: DOM → DOM: Core & HTML

So I tried doing this and it ran into a problem with workers. Specifically, worker runnables rely on the current Realm of the worker's one JSContext to figure out which global to operate in. See https://searchfox.org/mozilla-central/rev/e62c920f7f6463239c6634113f8a8351e263b936/dom/workers/WorkerRunnable.cpp#277-282

This is basically only a problem for worker debugger runnables that are running against a debugger sandbox. Without this change they end up with the sandbox global as the global; with this change they end up with the worker debugger global instead, and run in the wrong global. Looking into what the options there are now.

Depends on: 1573589
Depends on: 1573590
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/08af9d64c2fc
AutoNoJSAPI should make it clearer that script is not running.  r=bholley
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
Assignee: nobody → bzbarsky

Bogdan, do you know why this was uplifted? I didn't request uplift for this, and I think it should just be riding the trains. Or is this just the normal "Firefox 70 is now beta" changeover?

Flags: needinfo?(btara)

It was not uplifted. I did a planned merge from central to beta and, i ran bugherder on that push (by mistake).
Thus, your patch was merged to beta because it was in central at the right time. :)

Flags: needinfo?(btara)
You need to log in before you can comment on or make changes to this bug.