Open Bug 1589908 Opened 5 years ago Updated 2 years ago

The Worker name should be reflected in the threads pane

Categories

(DevTools :: Debugger, enhancement, P3)

enhancement

Tracking

(Not tracked)

People

(Reporter: jlast, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

It'd be great if the threads pane took advantage of the worker's name field when determining the name to show.

Aside from DevTools, the name is also accessible from within the Worker itself via global name (or http://self.name), which can help with logging or identifying which thread ID you're in.

https://twitter.com/RReverser/status/1185680151780233219

Attached image EHRhz8eXkAARZTS.png

Ha, thanks for filing. I think I brought this up in slack many moons ago but didn't file an issue.

Priority: -- → P3

Hi, I'd love to work on this bug but I'd like some pointers -
First I looked at how the worker thread being rendered in the <Thread/> secondary pane was represented in the redux store and I found the threads reducer.
I searched for functions that would dispatch an action with type INSERT_THREADS and I found fetchThreads.
It seems like fetchThreads returns a list of Thread plain objects created by createThread. I was thinking that here is the place where I would check whether the thread is a worker and set the name accordingly.
The second argument to createThread however is of type WorkerTargetFront. Turns out the current name is the last path of the URL, which in the case of a web worker constructed from a blob would be its UUID.
I don't yet understand how to use WorkerTargetFront and this is my first time delving into actors/fronts. I was wondering whether it's possible to call self.name on the DedicatedWorkerGlobalScope of the worker in the same manner as the toolbox spawned by about:debugging#workers does.

Flags: needinfo?(jlaster)

Thanks Bryan for looking into this. I think you're on the right path.

I'd start by looking in the worker actor
https://searchfox.org/mozilla-central/source/devtools/server/actors/targets/worker.js#45-71

which has access to the underlying worker

and then see if you can pass down the name field in the form.

once the worker front has the name, it will be a lot easier to decide what to do

Worker --> Worker Actor --> Worker Front --> redux store --> threads pane --> worker component
Flags: needinfo?(jlaster)

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:transfusion, could you have a look please?
For more information, please visit auto_nag documentation.

Flags: needinfo?(bryan.wyern1)
Assignee: nobody → bryan.wyern1
Status: NEW → ASSIGNED

Brian, can we review/land the attached patch?

Honza

Flags: needinfo?(bhackett1024)
Pushed by bhackett@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5af78a6c183e
The Worker name should be reflected in the threads pane r=bhackett
Flags: needinfo?(bhackett1024)
Backout by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/28f9b839e540
Backed out changeset 5af78a6c183e for dt failures on browser_dbg-toolbox-workers.js . CLOSED TREE

I looked into this briefly and the test passed for me locally....

Could you look into this?

Flags: needinfo?(bhackett1024)

(In reply to Jason Laster [:jlast] from comment #12)

Could you look into this?

The problem here is that the evaluateJS call is returning {"type":"undefined"} object, which gets coerced to an [object Object] string that is shown in the debugger and breaking the test (which looks for specific worker names). I don't know why this only fails in some cases. The best fix here is probably to only use the evaluateJS result if it is a string.

Flags: needinfo?(bhackett1024)

Hey Jason, do you mind taking another look at the phabricator revision please? It looks like work was done after the initial review and backout and it seems like a final review is needed in order to land again.

Flags: needinfo?(bryan.wyern1) → needinfo?(jlaster)

Jason, is there anything we could do?

Honza

Thanks, added it to my queue

Flags: needinfo?(jlaster)

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Assignee: bryan.wyern1 → nobody
Status: ASSIGNED → NEW
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: