10BC0 devtools: Create source actors from Debugger API notifications by delan · Pull Request #38334 · servo/servo · GitHub
[go: up one dir, main page]
Skip to content

devtools: Create source actors from Debugger API notifications #38334

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Aug 11, 2025
Merged

devtools: Create source actors from Debugger API notifications #38334

merged 1 commit into from
Aug 11, 2025

Conversation

delan
Copy link
Member
@delan delan commented Jul 29, 2025 •
edited
Loading

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, 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 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

@delan delan force-pushed the debugger-notify-sources branch from c031273 to 10bda07 Compare July 30, 2025 11:29
@delan delan mentioned this pull request Jul 30, 2025
Merged
@delan delan force-pushed the debugger-notify-sources branch from ab02658 to 6076d6b Compare July 30, 2025 13:22
This was referenced Jul 30, 2025
Merged
Open
@delan delan force-pushed the debugger-notify-sources branch 3 times, most recently from 24dd6b3 to 4e57fef Compare August 4, 2025 11:53
@delan delan mentioned this pull request Aug 4, 2025
Merged
github-merge-queue bot pushed a commit that referenced this pull request Aug 5, 2025
@delan @atbrakhi
script: Add new Window globals as debuggees (#38333)
92a9d24 
to debug the scripts in a page with the [SpiderMonkey Debugger
API](https://firefox-source-docs.mozilla.org/js/Debugger/), we need to
pass the page’s global object to
[Debugger.prototype.**addDebuggee()**](https://firefox-source-docs.mozilla.org/js/Debugger/Debugger.html#adddebuggee-global).
we could pick up the global via the [onNewGlobalObject()
hook](https://firefox-source-docs.mozilla.org/js/Debugger/Debugger.html#onnewglobalobject-global),
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 glo
10BC0
bals 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

---------

Signed-off-by: Delan Azabani <dazabani@igalia.com>
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
@delan delan force-pushed the debugger-notify-sources branch 2 times, most recently from 13cc146 to 7279871 Compare August 5, 2025 10:33
github-merge-queue bot pushed a commit that referenced this pull request Aug 5, 2025
@delan @atbrakhi
script: Tell SpiderMonkey whether scripts are inline (#38363)
3eddfea 
to use the [SpiderMonkey Debugger
API](https://firefox-source-docs.mozilla.org/js/Debugger/) as the single
source of truth about scripts and their sources for devtools purposes
(#38334), the debugger script needs to be able to distinguish
inline scripts from other scripts, because inline scripts are a special
case where the source contents need to come from the Servo parser.

the mechanism for this is
[Debugger.Script.prototype.**introductionType**](https://firefox-source-docs.mozilla.org/js/Debugger/Debugger.Source.html#introductiontype),
which is `inlineScript` for inline scripts or a variety of other values
for other kinds of scripts, but only the embedder can provide this
information.

this patch bumps mozjs to servo/mozjs#603, which expands on
CompileOptionsWrapper, making it a safe wrapper around CompileOptions.
to construct one from safe code, use Runtime::new_compile_options().
then you can call `set_introduction_type(&'static CStr)` on the new
instance. we also make Runtime::evaluate_script() take a
CompileOptionsWrapper from the caller, instead of constructing one
internally.

in this patch, we set the introductionType to `c"inlineScript"` when
calling run_a_classic_script() and compile_module_script() for inline
scripts, and leave it unset all other cases.

Testing: will undergo automated tests in #38334
Fixes: part of #36027, part of #38378

---------

Signed-off-by: Delan Azabani <dazabani@igalia.com>
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
@delan delan force-pushed the debugger-notify-sources branch 2 times, most recently from c1b1517 to f2c52a7 Compare August 7, 2025 10:53
jdm
jdm reviewed Aug 7, 2025
View reviewed changes
@delan delan force-pushed the debugger-notify-sources branch 5 times, most recently from 26ad7a6 to e3f8d08 Compare August 8, 2025 13:13
@delan delan mentioned this pull request Aug 8, 2025
Merged
@delan delan force-pushed the debugger-notify-sources branch from 52b3d86 to e4706d8 Compare August 8, 2025 14:02
@delan delan mentioned this pull request Aug 8, 2025
Merged
@delan delan force-pushed the debugger-notify-sources branch 2 times, most recently from 0b3ab41 to 09efa67 Compare August 9, 2025 05:29
github-merge-queue bot pushed a commit that referenced this pull request Aug 9, 2025
@delan @atbrakhi
script: Set correct introductionType values in more places (#38550)
6471587 
to use the [SpiderMonkey Debugger
API](https://firefox-source-docs.mozilla.org/js/Debugger/) as the single
source of truth about scripts and their sources for devtools purposes
(#38334), we need to keep track of whether scripts come from
an actual file or from things like setTimeout(), because for some
[introductionType](https://firefox-source-docs.mozilla.org/js/Debugger/Debugger.Source.html#introductiontype)
[values](https://firefox-source-docs.mozilla.org/devtools-user/debugger-api/debugger.source/#accessor-properties-of-the-debugger-source-prototype-object),
we want to disregard the script unless it has a [`//# sourceURL=`
override](https://tc39.es/ecma426/#sec-linking-eval)
([displayURL](https://firefox-source-docs.mozilla.org/js/Debugger/Debugger.Source.html#displayurl)).

this patch builds on #38363, setting the correct introductionType value
in several more cases.

Testing: will undergo many automated tests in #38334
Fixes: part of #36027

---------

Signed-off-by: Delan Azabani <dazabani@igalia.com>
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
@delan delan force-pushed the debugger-notify-sources branch 2 times, most recently from b6d83e7 to 2de3a24 Compare August 9, 2025 11:48
github-merge-queue bot pushed a commit that referenced this pull request Aug 9, 2025
@delan @atbrakhi
script: Add new worker globals as debuggees (#38551)
a3e0a34 
to debug workers in a page with the [SpiderMonkey Debugger
API](https://firefox-source-docs.mozilla.org/js/Debugger/), we need to
pass the worker’s global object to
[Debugger.prototype.**addDebuggee()**](https://firefox-source-docs.mozilla.org/js/Debugger/Debugger.html#adddebuggee-global).
we could pick up the global via the [onNewGlobalObject()
hook](https://firefox-source-docs.mozilla.org/js/Debugger/Debugger.html#onnewglobalobject-global),
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 creates a debugger global in worker threads, runs the
debugger script in those new globals, and plumbs new worker globals from
those threads into addDebuggee() via the debugger script. since worker
threads can’t generate PipelineId values, but they only ever run workers
on behalf of one pipeline, we use that pipeline’s PipelineId as the
PipelineId of the debugger global, rather than generating a unique
PipelineId like we do in script threads.

Testing: will undergo many automated tests in #38334
Fixes: part of #36027

Signed-off-by: Delan Azabani <dazabani@igalia.com>
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
@delan delan force-pushed the debugger-notify-sources branch from 2de3a24 to 8170724 Compare August 9, 2025 13:24
delan added a commit that referenced this pull request Aug 9, 2025
@delan @atbrakhi
Create source actors from Debugger API notifications (#38334)
ed2ae45 
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
Signed-off-by: Delan Azabani <dazabani@igalia.com>
delan added a commit that referenced this pull request Aug 9, 2025
@delan @atbrakhi
Create source actors from Debugger API notifications (#38334)
4657f62 
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
Signed-off-by: Delan Azabani <dazabani@igalia.com>
@delan delan force-pushed the debugger-notify-sources branch 3 times, most recently from 9096bc5 to 6c3828e Compare August 9, 2025 14:50
delan added a commit that referenced this pull request Aug 9, 2025
@delan @atbrakhi
Create source actors from Debugger API notifications (#38334)
bcb0954 
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
Signed-off-by: Delan Azabani <dazabani@igalia.com>
delan added a commit that referenced this pull request Aug 9, 2025
@delan @atbrakhi
Create source actors from Debugger API notifications (#38334)
f323919 
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
Signed-off-by: Delan Azabani <dazabani@igalia.com>
@delan delan force-pushed the debugger-notify-sources branch 2 times, most recently from b2687dc to 47215a6 Compare August 9, 2025 15:00
@delan delan marked this pull request as ready for review August 9, 2025 15:06
@delan delan requested a review from gterzian as a code owner August 9, 2025 15:06
@delan delan requested review from jdm and sagudev August 9, 2025 15:07
@delan delan force-pushed the debugger-notify-sources branch from 47215a6 to b7bf8fa Compare August 9, 2025 15:28
jdm
jdm approved these changes Aug 9, 2025
View reviewed changes
Copy link
Member
@jdm jdm left a comment

Choose a reason for hiding this comment

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

This is a nice improvement!

delan
delan commented Aug 10, 2025
View reviewed changes
@delan delan enabled auto-merge August 11, 2025 05:38
@delan @atbrakhi
Create source actors from Debugger API notifications
1b4af27 
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
Signed-off-by: Delan Azabani <dazabani@igalia.com>
@delan delan force-pushed the debugger-notify-sources branch from 482ec4e to 1b4af27 Compare August 11, 2025 05:52
delan added a commit that referenced this pull request Aug 11, 2025
@delan @atbrakhi
Create source actors from Debugger API notifications (#38334)
9ed96db 
Co-authored-by: atbrakhi <atbrakhi@igalia.com>
Signed-off-by: Delan Azabani <dazabani@igalia.com>
@delan delan added this pull request to the merge queue Aug 11, 2025
Merged via the queue into main with commit 4784668 Aug 11, 2025
21 checks passed
@delan delan deleted the debugger-notify-sources branch August 11, 2025 06:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdm jdm 3AEF jdm approved these changes

@gterzian gterzian Awaiting requested review from gterzian gterzian is a code owner

@sagudev sagudev Awaiting requested review from sagudev

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@delan @jdm
0