script: Add new Window globals as debuggees #38333
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
90d1e8e to
5865229
Compare
95595db to
c02c3fd
Compare
Signed-off-by: Delan Azabani <dazabani@igalia.com> Co-authored-by: atbrakhi <atbrakhi@igalia.com>
c02c3fd to
e2eabd4
Compare
sagudev
reviewed
Aug 4, 2025
Member
sagudev
left a comment
There was a problem hiding this comment.
Looks alright at first glance, I will take another look tomorrow with fresh eyes.
| /// Event for Rust → JS calls in [`crate::dom::DebuggerGlobalScope`]. | ||
| pub(crate) struct DebuggerEvent { | ||
| event: Event, | ||
| global: MutNullableDom<GlobalScope>, |
Member
There was a problem hiding this comment.
Why do we use MutNullableDom here? We do not need nor mutability nor null-ability.
Member
Author
There was a problem hiding this comment.
no reason, i just didn’t understand how it’s safe to use Dom<T> here. wouldn’t it momentarily be a forbidden “on-stack value” when used in the struct initialiser, but before moving it to the Box?
in any case, fixed in 11d5a2e
jdm
reviewed
Aug 4, 2025
Signed-off-by: Delan Azabani <dazabani@igalia.com>
Signed-off-by: Delan Azabani <dazabani@igalia.com>
Signed-off-by: Delan Azabani <dazabani@igalia.com>
jdm
approved these changes
Aug 5, 2025
Signed-off-by: Delan Azabani <dazabani@igalia.com>
sagudev
approved these changes
Aug 5, 2025
github-merge-queue bot
pushed a commit
that referenced
this pull request
Aug 5, 2025
#38333 adds new code to the debugger script containing some console.log() calls, which unlike native rust log messages can’t be turned off. this patch removes them for now, until we find a better approach. Testing: no testable changes in this patch Fixes: noisy logging as of #38333 Signed-off-by: Delan Azabani <dazabani@igalia.com> Co-authored-by: atbrakhi <atbrakhi@igalia.com>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Aug 11, 2025
currently our devtools impl creates source actors in script, when executing scripts in HTMLScriptElement or DedicatedWorkerGlobalScope. this approach is cumbersome, and it means that many pathways to running scripts are missed, such as imported ES modules. with the [SpiderMonkey Debugger API](https://firefox-source-docs.mozilla.org/js/Debugger/), we can pick up all of the scripts and all of their sources without any extra code, as long as we tell it about every global we create (#38333, #38551). this patch adds a [Debugger#onNewScript() hook](https://firefox-source-docs.mozilla.org/js/Debugger/Debugger.html#onnewscript-script-global) to the debugger script, which calls DebuggerGlobalScope#notifyNewSource() to notify our script system when a new script runs. if the source is relevant to the file tree in the Sources tab, script tells devtools to create a source actor. Testing: adds several new automated devtools tests Fixes: part of #36027 Signed-off-by: Delan Azabani <dazabani@igalia.com> Co-authored-by: atbrakhi <atbrakhi@igalia.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
to debug the scripts in a page with the SpiderMonkey Debugger API, we need to pass the page’s global object to Debugger.prototype.addDebuggee(). we could pick up the global via the onNewGlobalObject() hook, but if our script system passes the global to the debugger script instead, we can later add details like the PipelineId that will help servo identify debuggees that the API is notifying us about (#38334).
this patch plumbs new Window globals from script into addDebuggee() via the debugger script. to call into the debugger script with structured input, we create a new DOM event type, DebuggerEvent, that the debugger script listens for as the “addDebuggee” event.
Testing: no testable effects yet, but will be used in #37667
Fixes: part of #36027