Skip to content

script: propagate &mut JSContext in GlobalScope::report_an_error#44658

Open
elomscansio wants to merge 2 commits into
servo:mainfrom
elomscansio:port-GlobalScope-report_an_error
Open

script: propagate &mut JSContext in GlobalScope::report_an_error#44658
elomscansio wants to merge 2 commits into
servo:mainfrom
elomscansio:port-GlobalScope-report_an_error

Conversation

@elomscansio
Copy link
Copy Markdown
Contributor

Propagate &mut JSContext in GlobalScope::report_an_error and related call sites.

Testing: Successful build is enough
Fixes: Part of #42638

Signed-off-by: Emmanuel Paul Elom <elomemmanuel007@gmail.com>
Comment thread components/script/dom/bindings/error.rs Outdated
value.handle(),
can_gc,
);
pub(crate) fn report_pending_exception(cx: &mut js::context::JSContext) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function took cx and InRealm so now it should take &mut https://doc.servo.org/mozjs/realm/struct.CurrentRealm.html

@servo-highfive servo-highfive added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. S-awaiting-review There is new code that needs to be reviewed. and removed S-awaiting-review There is new code that needs to be reviewed. S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels May 1, 2026
Comment thread components/script/dom/bindings/error.rs Outdated
Comment on lines +353 to +354
let mut realm = enter_auto_realm(cx, &*global);
let cx = &mut realm.current_realm();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because we now get realm we do not need to enter it anymore and we can also move global obtaining down

Comment on lines 111 to +115

'Console': {
'cx': ['Log', 'Clear', 'Debug', 'Info', 'Warn', 'Error', 'Trace']
},

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sagudev Do you know why i'm having this error.

   Compiling servo-script-bindings v0.2.0 (/home/elomscansio/Desktop/PROJECTS/SERVO_SETUP/servo/components/script_bindings)
   Compiling servo-script v0.2.0 (/home/elomscansio/Desktop/PROJECTS/SERVO_SETUP/servo/components/script)
error[E0053]: method `Log` has an incompatible type for trait
   --> components/script/dom/console.rs:716:16
    |
716 |     fn Log(cx: &mut js::context::JSContext, global: &GlobalScope, messages: Vec<HandleVa...
    |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `JSContext`, found `&mut js::context::JSContext`
    |
    = note: expected signature `fn(script_bindings::script_runtime::JSContext, &globalscope::GlobalScope, Vec<js::rust::Handle<'_, _>>)`
               found signature `fn(&mut js::context::JSContext, &globalscope::GlobalScope, Vec<js::rust::Handle<'_, _>>)`
help: change the parameter type to match the trait
    |
716 -     fn Log(cx: &mut js::context::JSContext, global: &GlobalScope, messages: Vec<HandleValue>) {
716 +     fn Log(cx: script_bindings::script_runtime::JSContext, global: &GlobalScope, messages: Vec<HandleValue>) {
    |

error[E0053]: method `Debug` has an incompatible type for trait
   --> components/script/dom/console.rs:741:18
    |
741 |     fn Debug(cx: &mut js::context::JSContext, global: &GlobalScope, messages: Vec<Handle...
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `JSContext`, found `&mut js::context::JSContext`
    |
    = note: expected signature `fn(script_bindings::script_runtime::JSContext, &globalscope::GlobalScope, Vec<js::rust::Handle<'_, _>>)`
               found signature `fn(&mut js::context::JSContext, &globalscope::GlobalScope, Vec<js::rust::Handle<'_, _>>)`
help: change the parameter type to match the trait
    |
741 -     fn Debug(cx: &mut js::context::JSContext, global: &GlobalScope, messages: Vec<HandleValue>) {
741 +     fn Debug(cx: script_bindings::script_runtime::JSContext, global: &GlobalScope, messages: Vec<HandleValue>) {
    |

error[E0053]: method `Info` has an incompatible type for trait
   --> components/script/dom/console.rs:752:17
    |
752 |     fn Info(cx: &mut js::context::JSContext, global: &GlobalScope, messages: Vec<HandleV...
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `JSContext`, found `&mut js::context::JSContext`
    |
    = note: expected signature `fn(script_bindings::script_runtime::JSContext, &globalscope::GlobalScope, Vec<js::rust::Handle<'_, _>>)`
               found signature `fn(&mut js::context::JSContext, &globalscope::GlobalScope, Vec<js::rust::Handle<'_, _>>)`
help: change the parameter type to match the trait
    |
752 -     fn Info(cx: &mut js::context::JSContext, global: &GlobalScope, messages: Vec<HandleValue>) {
752 +     fn Info(cx: script_bindings::script_runtime::JSContext, global: &GlobalScope, messages: Vec<HandleValue>) {
    |

error[E0053]: method `Warn` has an incompatible type for trait
   --> components/script/dom/console.rs:763:17
    |
763 |     fn Warn(cx: &mut js::context::JSContext, global: &GlobalScope, messages: Vec<HandleV...
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `JSContext`, found `&mut js::context::JSContext`
    |
    = note: expected signature `fn(script_bindings::script_runtime::JSContext, &globalscope::GlobalScope, Vec<js::rust::Handle<'_, _>>)`
               found signature `fn(&mut js::context::JSContext, &globalscope::GlobalScope, Vec<js::rust::Handle<'_, _>>)`
help: change the parameter type to match the trait
    |
763 -     fn Warn(cx: &mut js::context::JSContext, global: &GlobalScope, messages: Vec<HandleValue>) {
763 +     fn Warn(cx: script_bindings::script_runtime::JSContext, global: &GlobalScope, messages: Vec<HandleValue>) {
    |

error[E0053]: method `Error` has an incompatible type for trait
   --> components/script/dom/console.rs:774:18
    |
774 |     fn Error(cx: &mut js::context::JSContext, global: &GlobalScope, messages: Vec<Handle...
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `JSContext`, found `&mut js::context::JSContext`
    |
    = note: expected signature `fn(script_bindings::script_runtime::JSContext, &globalscope::GlobalScope, Vec<js::rust::Handle<'_, _>>)`
               found signature `fn(&mut js::context::JSContext, &globalscope::GlobalScope, Vec<js::rust::Handle<'_, _>>)`
help: change the parameter type to match the trait
    |
774 -     fn Error(cx: &mut js::context::JSContext, global: &GlobalScope, messages: Vec<HandleValue>) {
774 +     fn Error(cx: script_bindings::script_runtime::JSContext, global: &GlobalScope, messages: Vec<HandleValue>) {
    |

error[E0053]: method `Trace` has an incompatible type for trait
   --> components/script/dom/console.rs:785:18
    |
785 |     fn Trace(cx: &mut js::context::JSContext, global: &GlobalScope, messages: Vec<Handle...
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `JSContext`, found `&mut js::context::JSContext`
    |
    = note: expected signature `fn(script_bindings::script_runtime::JSContext, &globalscope::GlobalScope, Vec<js::rust::Handle<'_, _>>)`
               found signature `fn(&mut js::context::JSContext, &globalscope::GlobalScope, Vec<js::rust::Handle<'_, _>>)`
help: change the parameter type to match the trait
    |
785 -     fn Trace(cx: &mut js::context::JSContext, global: &GlobalScope, messages: Vec<HandleValue>) {
785 +     fn Trace(cx: script_bindings::script_runtime::JSContext, global: &GlobalScope, messages: Vec<HandleValue>) {
    |

For more information about this error, try `rustc --explain E0053`.
error: could not compile `servo-script` (lib) due to 6 previous errors
      Timing report saved to /home/elomscansio/Desktop/PROJECTS/SERVO_SETUP/servo/target/cargo-timings/cargo-timing-20260501T155032.617163731Z.html
Failed in 0:01:36

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, this will might be a problem in codegen, I will check what is wrong tomorrow.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, you need to write lowercase console:

Suggested change
'Console': {
'cx': ['Log', 'Clear', 'Debug', 'Info', 'Warn', 'Error', 'Trace']
},
'console': {
'cx': ['Log', 'Clear', 'Debug', 'Info', 'Warn', 'Error', 'Trace']
},

because the trait is consoleMethods. This is one of the rarest cases where trait is lowercased.

…m instead of JSContext

Signed-off-by: Emmanuel Paul Elom <elomemmanuel007@gmail.com>
@elomscansio elomscansio force-pushed the port-GlobalScope-report_an_error branch from 38052f4 to 2fcc799 Compare May 1, 2026 16:06
@@ -429,9 +432,9 @@ pub(crate) fn take_and_report_pending_exception_for_api(

let return_value = javascript_error_info_from_error_info(cx, &error_info, value.handle());
GlobalScope::from_safe_context(cx.into(), in_realm).report_an_error(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated, but we should used GlobalScope::from_current_realm here, so we can avoid in realm proof.

Comment on lines +674 to +677
let window = &mut document.window();
let global = window.global();

let mut realm = enter_auto_realm(cx, &*global);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before we entered realm of window so we should keep doing this:

Suggested change
let window = &mut document.window();
let global = window.global();
let mut realm = enter_auto_realm(cx, &*global);
let window = document.window();
let mut realm = enter_auto_realm(cx, window);

let url = cformat!("{}", handler.url);
let options = unsafe { CompileOptionsWrapper::new_raw(*cx, url, handler.line as u32) };
let options =
unsafe { CompileOptionsWrapper::new_raw(cx.raw_cx(), url, handler.line as u32) };
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
unsafe { CompileOptionsWrapper::new_raw(cx.raw_cx(), url, handler.line as u32) };
CompileOptionsWrapper::new(cx, url, handler.line as u32);

let mut ids = unsafe { IdVector::new(*cx) };
let mut ids = unsafe { IdVector::new(cx.raw_cx()) };
if !unsafe {
GetPropertyKeys(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In such cases we should use functions from wrappers2 if possible: https://doc.servo.org/mozjs/rust/wrappers2/fn.GetPropertyKeys.html

This goes for many methods in this file that need cx.raw_cx().

Comment on lines +1453 to +1454
let mut cx = unsafe { script_bindings::script_runtime::temp_cx() };
let cx = &mut cx;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTE-TO-SELF: This will likely need to be perma cx, tho I think we should get it from SM.

@servo-highfive servo-highfive added the S-needs-rebase There are merge conflict errors. label May 4, 2026
@TimvdLippe
Copy link
Copy Markdown
Contributor

@elomscansio what's the ststus of this PR? The migration was going along nicely and we would love to continue going. Is there anything we can help you with or is life busy in general that you want to revisit this later?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-awaiting-review There is new code that needs to be reviewed. S-needs-rebase There are merge conflict errors.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants