Devtools: Support worker scripts in `Debugger > Source` panel #36562

atbrakhi · 2025-04-16T08:01:10Z

This patch adds support for listing worker scripts in debugger > source panel

For example:

<!-- test.html -->
<!doctype html><meta charset=utf-8>
<script>
    setTimeout(() => {
        console.log("inline classic");
        new Worker("worker.js");
        
        const blob = new Blob([`console.log("blob worker");`], { type: "text/javascript" });
        const blobURL = URL.createObjectURL(blob);
        new Worker(blobURL);
    }, 2000);
</script>

// worker.js
console.log("external classic worker");

./mach run --devtools=6080 http://127.0.0.1:3000/test.html

Another example:

./mach run --devtools=6080 https://charming.daz.cat/

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes partially implement devtools: Implement Debugger tab #36027
These changes require tests, but they are blocked on Devtools: Set up automated tests #36325

…hread` (#36631) This is part of #36562, where I discovered that we are sending thread as `context` and `id` for worker is called `worked_id`. In this PR, I have renamed the `worker_id` to `id` and thread is sent as `thread` and not context. These chages were tested in #36562 Some logs for context(this logs are from firefox instance) `worker_id` should be `id` ``` {"_from": "server1.conn0.watcher35", "message": {"from": "server1.conn0.watcher35", "target": {"actor": "server1.conn0.watcher34.process7//workerTarget21/workerTarget5", "consoleActor": "server1.conn0.watcher34.process7//workerTarget21/console2", "id": "f454ea34-c76a-4b6e-a079-43ee3128881e", "name": "", "objectsManagerActor": "server1.conn0.watcher34.process7//workerTarget21/objects-manager4", "relatedDocumentInnerWindowId": 15032385539, "targetType": "worker", "threadActor": "server1.conn0.watcher34.process7//workerTarget21/thread1", "tracerActor": "server1.conn0.watcher34.process7//workerTarget21/tracer3", "traits": {"supportsTopLevelTargetFlag": false}, "type": 0, "url": "http://localhost:8000/Documents/codespace/servo/worker.js"}, "type": "target-available-form"}} ``` Thread should be `thread` and not `context` ``` {"_from": "server1.conn0.watcher35", "message": {"from": "server1.conn0.watcher35", "target": {"actor": "server1.conn0.watcher34.process7//workerTarget21/workerTarget5", "consoleActor": "server1.conn0.watcher34.process7//workerTarget21/console2", "id": "f454ea34-c76a-4b6e-a079-43ee3128881e", "name": "", "objectsManagerActor": "server1.conn0.watcher34.process7//workerTarget21/objects-manager4", "relatedDocumentInnerWindowId": 15032385539, "targetType": "worker", "threadActor": "server1.conn0.watcher34.process7//workerTarget21/thread1", "tracerActor": "server1.conn0.watcher34.process7//workerTarget21/tracer3", "traits": {"supportsTopLevelTargetFlag": false}, "type": 0, "url": "http://localhost:8000/Documents/codespace/servo/worker.js"}, "type": "target-available-form"}} ``` - [x] ./mach build -d does not report any errors - [x] ./mach test-tidy does not report any errors - [x] These changes partially implement #36027 Signed-off-by: atbrakhi <atbrakhi@igalia.com>

components/devtools/actors/browsing_context.rs

components/devtools/actors/watcher.rs

components/devtools/actors/worker.rs

components/script/dom/worker.rs

mrobinson · 2025-04-23T08:38:18Z

components/script/dom/worker.rs

+                let _ = chan.send(ScriptToDevtoolsControlMsg::ScriptSourceLoaded(
+                    pipeline_id,
+                    Some(worker_id),
+                    worker_source_info,
+                ));


I would send both URLs unconditionally as part of SourceInfo as well as a boolean indicating if this is a worker or not. Then the URL calculation should be done in the devtools part. It's really unclear to me why a particular URL is used here as well. The logic seems to be:

If URL is blob:

If blob parsing succeeds, use blob UUID

Otherwise, use worker URL

Otherwise use script URL

Why aren't you falling back to the script URL when blob parsing fails?

@mrobinson thanks for the review. let me try to answer your questions.

I would send both URLs unconditionally as part of SourceInfo as well as a boolean indicating if this is a worker or not

For worker case we will always get url from worker.rs and for non-worker case we will get the url from htmlscriptelement.rs, I am not sure why we need the boolean indication

Then the URL calculation should be done in the devtools part. It's really unclear to me why a particular URL is used here as well

Oh, this crossed my mind and ideally I would have loved to handle it on devtools side but let me try to explain why I decided to do it this way:

It's mostly because of what script_url and worker_url results in both cases! For a normal worker file like test.html in description:

script_url = USVString("worker.js")

but for a blob url (like https://charming.daz.cat/ )

script_url = USVString("blob:https://charming.daz.cat/e4a6cdd9-6d11-4382-9621-eee4f252bd1a")

In both case worker_url is:

worker_url: "http://127.0.0.1:8000/worker.js" worker_url: "blob:https://charming.daz.cat/e4a6cdd9-6d11-4382-9621-eee4f252bd1a"

Firefox client expects worker.js for first case and b38f182a-81d8-4ca1-9458-81d51a3b702b for second case.

Devtools needs the worker_id( i.e b38f182a-81d8-4ca1-9458-81d51a3b702b) and on script side we have functions to parse the url and get the id(parse_blob_url). Hence it was more straight forward to handle this on worker.rs file itslef. Handling it on devtools side would have required extra logic/code.

Does Firefox have the same behavior for all blob URLs? For example, in the non-worker case what does Firefox do with a <script> that is a blob URL?

No. Looking at the logs, client expectations are different for blob URL in worker and non-worker case:

For non-worker case, this is the logs (note that it doesn't expects id but only url "url": "blob:null/2c28e98c-0da2-4198-860a-c72e20b4a49b"})

{"_from": "server1.conn1.watcher34.process9//windowGlobalTarget2", "message": {"array": [["console-message", [{"arguments": ["hello blob script."], "columnNumber": 9, "filename": "blob:null/2c28e98c-0da2-4198-860a-c72e20b4a49b", "innerWindowID": 19327352833, "level": "log", "lineNumber": 1, "sourceId": null, "timeStamp": 1745401545017.463}]]], "from": "server1.conn1.watcher34.process9//windowGlobalTarget2", "type": "resources-available-array"}} {"_from": "server1.conn1.watcher34.process9//windowGlobalTarget2", "message": {"array": [["source", [{"actor": "server1.conn1.watcher34.process9//source20", "extensionName": null, "introductionType": "scriptElement", "isBlackBoxed": false, "isInlineSource": true, "sourceLength": 287, "sourceMapBaseURL": "file:///Users/atbrakhi/Documents/codespace/servo/test.html", "sourceMapURL": null, "sourceStartColumn": 10, "sourceStartLine": 4, "url": "file:///Users/atbrakhi/Documents/codespace/servo/test.html"}, {"actor": "server1.conn1.watcher34.process9//source21", "extensionName": null, "isBlackBoxed": false, "isInlineSource": false, "sourceLength": 48, "sourceMapBaseURL": null, "sourceMapURL": null, "sourceStartColumn": 0, "sourceStartLine": 1, "url": "blob:null/2c28e98c-0da2-4198-860a-c72e20b4a49b"}]]], "from": "server1.conn1.watcher34.process9//windowGlobalTarget2", "type": "resources-available-array"}}

For worker case (note that it expects both "id": "f454ea34-c76a-4b6e-a079-43ee3128881e" and url "url": "http://localhost:8000/Documents/codespace/servo/worker.js". Just sending the sourceInfo is not enough here)

{"_from": "server1.conn0.watcher35", "message": {"from": "server1.conn0.watcher35", "target": {"actor": "server1.conn0.watcher34.process7//workerTarget21/workerTarget5", "consoleActor": "server1.conn0.watcher34.process7//workerTarget21/console2", "id": "f454ea34-c76a-4b6e-a079-43ee3128881e", "name": "", "objectsManagerActor": "server1.conn0.watcher34.process7//workerTarget21/objects-manager4", "relatedDocumentInnerWindowId": 15032385539, "targetType": "worker", "threadActor": "server1.conn0.watcher34.process7//workerTarget21/thread1", "tracerActor": "server1.conn0.watcher34.process7//workerTarget21/tracer3", "traits": {"supportsTopLevelTargetFlag": false}, "type": 0, "url": "http://localhost:8000/Documents/codespace/servo/worker.js"}, "type": "target-available-form"}}

I have created #36727 for this issue

components/devtools/actors/worker.rs

Signed-off-by: atbrakhi <atbrakhi@igalia.com>

mrobinson

Looks good with one change before landing. Thanks!

mrobinson · 2025-04-28T10:19:30Z

components/script/dom/worker.rs

+                let source_info = SourceInfo {
+                    url: worker_url.clone(),
+                    external: true, // Worker scripts are always external.
+                };
+                let _ = chan.send(ScriptToDevtoolsControlMsg::ScriptSourceLoaded(
+                    pipeline_id,
+                    Some(worker_id),
+                    source_info,
+                ));


I think that Option<WorkerId> should be part of SourceInfo instead of the message itself as SourceInfo should include all information about the source.

I agree that SourceInfo should include all the information about the source, and we are sending all information in to the client but I am not sure we should include the worker_id in the sourceInfo struct itself. Some reasons for that:

SourceInfo represents information abouts source code itself(not only specific to worker, could be for both frame and worker and in future for something else as well), this struct will grow with upcoming work related to source panel and will have more fields

worker_id is more of representation for the exectiation context and is not only* specific to sources. In this PR yes, they are realted, but if we consider the whole devtools, worker_id allows us to follow the seperation of concern, as in information about the source is separate from information about where it is running and for what.

existing code in browsing_context also follows this seperation, and i think we should continue that.

Not sure if you meant something else, am I missing something?

I don't see how something like external: bool is very different in this case from worker_id: Option<WorkerId>. In the end SourceInfo is the payload of this message. The message payload should all be in the same data structure or all be in the body of the message. Splitting it between two things somewhat arbitrarily doesn't make sense to me.

Ah, sorry, I misunderstood your comment. I saw your suggestion as worker_id should be part of SourceInfo in all places, and that confused me.

Thanks for clarifying, 0b30138 updates the SourceInfo struct to include worker_id

Signed-off-by: atbrakhi <atbrakhi@igalia.com>

atbrakhi marked this pull request as ready for review April 17, 2025 08:43

atbrakhi requested a review from gterzian as a code owner April 17, 2025 08:43

atbrakhi requested a review from delan April 17, 2025 08:43

This was referenced Apr 17, 2025

devtools: Implement Debugger tab #36027

Open

DevTools: worker_id should be id & thread for workers should be thread #36631

Merged

DevTools: Add resource_available as a common shared util #36632

Merged

atbrakhi requested a review from mrobinson April 23, 2025 07:58

atbrakhi force-pushed the sources_worker branch from e136630 to 74cf4fb Compare April 23, 2025 08:04

mrobinson requested changes Apr 23, 2025

View reviewed changes

jdm reviewed Apr 23, 2025

View reviewed changes

components/devtools/actors/worker.rs Show resolved Hide resolved

atbrakhi added 8 commits April 28, 2025 10:10

Add missing pieces for worker case

96884e3

Signed-off-by: atbrakhi <atbrakhi@igalia.com>

udpate watchTargets and watchResource to consider worker

a96d683

Signed-off-by: atbrakhi <atbrakhi@igalia.com>

fix serialization of TargetActorMsg to be used in WatchTargetsReply

376f6e7

Signed-off-by: atbrakhi <atbrakhi@igalia.com>

worker_id shoud be "id" & thread for workers is thread not context

35cdb71

Signed-off-by: atbrakhi <atbrakhi@igalia.com>

fmt

20e7d5a

Signed-off-by: atbrakhi <atbrakhi@igalia.com>

watchTargets should watch for targetType before sending targets

3865985

Signed-off-by: atbrakhi <atbrakhi@igalia.com>

For worker case send source in ResourceAvailableReply

4863d23

Signed-off-by: atbrakhi <atbrakhi@igalia.com>

fix imports and issue caused by struct renamings

9c74cba

Signed-off-by: atbrakhi <atbrakhi@igalia.com>

atbrakhi force-pushed the sources_worker branch from 74cf4fb to 9c74cba Compare April 28, 2025 08:26

review fix

91d4ac4

Signed-off-by: atbrakhi <atbrakhi@igalia.com>

atbrakhi mentioned this pull request Apr 28, 2025

Devtools: Fix url parsing for worker urls #36727

Open

atbrakhi requested a review from mrobinson April 28, 2025 10:10

mrobinson approved these changes Apr 28, 2025

View reviewed changes

worker_id should be part of SourceInfo

0b30138

Signed-off-by: atbrakhi <atbrakhi@igalia.com>

atbrakhi enabled auto-merge April 29, 2025 08:14

atbrakhi added this pull request to the merge queue Apr 29, 2025

Merged via the queue into servo:main with commit b92542b Apr 29, 2025
21 checks passed

atbrakhi deleted the sources_worker branch April 29, 2025 09:42

Uh oh!

Devtools: Support worker scripts in Debugger > Source panel #36562

Devtools: Support worker scripts in Debugger > Source panel #36562

Uh oh!

Conversation

atbrakhi commented Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mrobinson Apr 23, 2025

Choose a reason for hiding this comment

Uh oh!

atbrakhi Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrobinson Apr 23, 2025

Choose a reason for hiding this comment

Uh oh!

atbrakhi Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atbrakhi Apr 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mrobinson left a comment

Choose a reason for hiding this comment

Uh oh!

mrobinson Apr 28, 2025

Choose a reason for hiding this comment

Uh oh!

atbrakhi Apr 28, 2025

Choose a reason for hiding this comment

Uh oh!

mrobinson Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atbrakhi Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Devtools: Support worker scripts in `Debugger > Source` panel #36562

Devtools: Support worker scripts in `Debugger > Source` panel #36562

atbrakhi commented Apr 16, 2025 •

edited

Loading

atbrakhi Apr 23, 2025 •

edited

Loading

atbrakhi Apr 23, 2025 •

edited

Loading

mrobinson Apr 29, 2025 •

edited

Loading