script: Propagate js::context::JSContext through Event::fire.#45040
Conversation
servo-highfive
added
the
S-awaiting-review
There was a problem hiding this comment.
non-blocking: we could use SM functions from https://doc.servo.org/mozjs/rust/wrappers2/index.html
There was a problem hiding this comment.
I'd like to leave this for a followup.
| ) { | ||
| let cx = GlobalScope::get_cx(); | ||
| // SAFETY: always safe from a JS engine hook. | ||
| let mut cx = unsafe { temp_cx() }; |
There was a problem hiding this comment.
When we are in SM hook, this is perma cx from thin air not temporary (to be removed someday), so we should do like:
There was a problem hiding this comment.
There is no cx pointer to adopt, so I'm not sure exactly what to do.
There was a problem hiding this comment.
Ah, I see. jsshell uses TLS: https://searchfox.org/firefox-main/source/js/src/shell/js.cpp#1301 so we should use it here to, meaning that we just need to inline temp_cx.
Side-note: We should probably have this kind of function (so perma cx) in mozjs under mozjs::context::JSContext::get to match https://doc.servo.org/mozjs/rust/struct.Runtime.html#method.get
There was a problem hiding this comment.
I figure I'll leave this to be addressed by servo/mozjs#748
servo-highfive
added
S-needs-code-changes
Signed-off-by: Josh Matthews <josh@joshmatthews.net>
Signed-off-by: Josh Matthews <josh@joshmatthews.net>
servo-highfive
added
S-awaiting-review
servo-highfive
removed
the
S-awaiting-review
servo-highfive
added
the
S-awaiting-review
servo-highfive
added
the
S-awaiting-merge
servo-highfive
removed
the
S-awaiting-merge
This is unofficially perma cx. Sometimes it is needed: servo/servo#45040 (comment) and https://doc.servo.org/mozjs/rust/struct.Runtime.html#method.get is planned to be removed. --------- Signed-off-by: Sam <16504129+sagudev@users.noreply.github.com>
4 participants
Testing: No behaviour changes; existing tests are sufficient.
Fixes: Part of #42638