1563607 - Show active service workers in threads pane

Status: RESOLVED FIXED

(DevTools :: Debugger, enhancement, P2)

Description

[Brian Hackett [Laid off!]]

Worker threads associated with service workers which are being used on a page should show up in the debugger's threads pane along with the main thread and normal workers.

The main tricky bit in dealing with service workers is that multiple windows can share service worker threads if they are in the same content process. If such a shared service worker is paused in one window's debugger, other non-debugged windows might become unusable. I don't understand some aspects of this behavior, though. If I open multiple tabs which are running in different processes and accessing a URL associated with the same service worker registration, worker threads for the service worker's script are spawned in each of those processes, which should be able to be paused without interfering with threads in other processes. If I open enough tabs that multiple tabs are running in one content process, they share the same service worker, though (the service worker script only starts once in that process). This behavior seems strange and it surprises me that the assignment of tabs to processes has an effect on when service worker scripts run. The related issue of windows that are connected by window.open() calls sharing service workers seems fine/expected; these windows can access each other's contents and expose a host of existing issues in the debugger like being able to break run-to-completion and trying to interact with paused workers.

Besides this issue, it shouldn't be too hard to show threads for service workers in the threads pane, as they seem in other respects identical to normal worker threads.

Comment 1

[Jason Laster [:jlast]]

CC Perry

Comment 2

[Julian Descottes [:jdescottes]]

Some additional context for Perry since I was the one who suggested cc-ing you: I don't know how much of the summary from Brian is specific to the current implementation vs parent-intercept (Bug 1231213).

We discussed with Honza that we should probably focus on a solution that works for the new implementation, so I think you can assume parent-intercept = true for this discussion.

Comment 3

[Perry Jiang [:perry] [no longer employee, use ni?]]

The current implementation is consistent with my understanding of the bug summary, where there will be one Service Worker per (origin, path) per content process. So, if origin A registers a Service Worker and there's two origin A tabs running in separate processes, there will be two (identical) origin A Service Workers, one in each of those tabs. I agree that this very annoying behavior...

The new implementation, however, will have one Service Worker per (origin, path) per browser (so Service Workers and (origin, path) are purely 1:1), where the content process the Service Worker spawns in will be selected at random. The current changes for the new implementation doesn't have a way to get the process ID of the randomly selected process for a given SW, but that doesn't sound difficult to add.

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Moving this report to the reserve list since we need to wait till the platform is more stable.
Honza

Comment 6

[Jason Laster [:jlast]]

removing from 70 while we wait on platform

Comment 9

[:Harald Kirschner :digitarald]

Per Monday's meeting, Brian started looking into this.

Comment 10

[Brian Hackett [Laid off!]]

Attachment: WIP

This WIP shows service workers in the threads pane that are registered when the debugger is open or which were already running when the debugger attached. If the debugger is open when an existing service worker starts up, it does not show up in the threads pane, however. I'd like to fix this but I'm running into a problem writing a test that exposes the problem. I'd like a mochitest that opens a tab, registers a service worker, then opens another tab that interacts with that service worker in some way (calls fetch(), loads an iframe, whatever). I've tried a bunch of things to get this working but every time the load which should go through the service worker just 404s instead. I also haven't been able to find an existing mochitest with similar functionality, as they generally seem to be testing corner cases and are hard to read. So, some help here would be appreciated if I'm going to be able to move forward.

Comment 11

[Brian Hackett [Laid off!]]

Attachment: patch

This works around the issues I was seeing earlier with service workers (i.e. I still don't understand the worker's "scope" property and its effects on the worker's behavior) and adds some fixes so that service workers consistently show up in the threads pane when expected. This does still show all service workers running in the target content process; we'll want to only show workers that are relevant to the current target, but that seems better to do in followup. This new functionality is gated on the devtools.debugger.features.windowless-service-workers preference.

Comment 12

[Brian Hackett [Laid off!]]

Attachment: Bug 1563607 Part 1 - Service worker check evaluation runnable should be a debuggee runnable, r=asuth.

Comment 13

[Brian Hackett [Laid off!]]

Attachment: Bug 1563607 Part 2 - Attach to service workers in debugger when pref is enabled, r=jdescottes.

Depends on D47541

Comment 14

[Brian Hackett [Laid off!]]

Attachment: Bug 1563607 Part 3 - Avoid errors when debugging service workers in content processes, r=jdescottes.

Depends on D47542

Comment 15

[Brian Hackett [Laid off!]]

Attachment: Bug 1563607 Part 4 - Update threads and sources after navigation, r=jlast.

Depends on D47543

Comment 16

[Brian Hackett [Laid off!]]

Attachment: Bug 1563607 Part 5 - Add test for windowless service worker debugging, r=jdescottes.

Depends on D47544

Comment 17

[:Harald Kirschner :digitarald]

One consideration for the UI/implementation side: The threads panel should not just show the active SW, but any SW that is currently being executed for the current scope. This can usually an installing then waiting worker, which is queued after an update is found.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a0d9b36ac4fe
Part 1 - Service worker check evaluation runnable should be a debuggee runnable, r=asuth.
https://hg.mozilla.org/integration/autoland/rev/cec07747fb2a
Part 2 - Attach to service workers in debugger when pref is enabled, r=jdescottes,jlast.
https://hg.mozilla.org/integration/autoland/rev/1dc694b39efa
Part 4 - Update threads and sources after navigation, r=jlast.
https://hg.mozilla.org/integration/autoland/rev/c8f4de2596bc
Part 5 - Add test for windowless service worker debugging, r=jdescottes.

Comment 19

[Daniel Varga [:dvarga]]

Backed out 4 changesets (Bug 1563607) for debugger failures

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedJob=274861861&searchStr=browser-chrome&revision=c8f4de2596bcd6387c84ca02d59389a4dd309c89

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=274878590&repo=autoland&lineNumber=151

Backout link: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedJob=274878856&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=0984b2197e4b9d7d2fc17b4310f9acf46f098066

[task 2019-11-06T17:55:12.886Z] TEST START | Flow
[task 2019-11-06T17:55:27.928Z] Error ------------------------------------------------------------------------------- src/client/firefox/events.js:22:13
[task 2019-11-06T17:55:27.928Z] 
[task 2019-11-06T17:55:27.928Z] Cannot resolve module `devtools/client/shared/workers-listener.js`.
[task 2019-11-06T17:55:27.928Z] 
[task 2019-11-06T17:55:27.928Z]    22| } = require("devtools/client/shared/workers-listener.js");
[task 2019-11-06T17:55:27.928Z]                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[task 2019-11-06T17:55:27.928Z] 
[task 2019-11-06T17:55:27.928Z] 
[task 2019-11-06T17:55:27.928Z] Error -------------------------------------------------------------------------------- src/client/firefox/events.js:65:9
[task 2019-11-06T17:55:27.928Z] 
[task 2019-11-06T17:55:27.928Z] Cannot call `actions.ensureHasThread` because property `ensureHasThread` is missing in `Actions` [1].
[task 2019-11-06T17:55:27.928Z] 
[task 2019-11-06T17:55:27.928Z]    src/client/firefox/events.js:65:9
[task 2019-11-06T17:55:27.928Z]    65|   await actions.ensureHasThread(threadFront.actor);
[task 2019-11-06T17:55:27.928Z]                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[task 2019-11-06T17:55:27.928Z] 
[task 2019-11-06T17:55:27.928Z] References:
[task 2019-11-06T17:55:27.928Z]    src/client/firefox/events.js:33:14
[task 2019-11-06T17:55:27.928Z]    33| let actions: Actions;
[task 2019-11-06T17:55:27.928Z]                     ^^^^^^^ [1]
[task 2019-11-06T17:55:27.928Z] 
[task 2019-11-06T17:55:27.928Z] 
[task 2019-11-06T17:55:27.928Z] Error ------------------------------------------------------------------------------ src/client/firefox/targets.js:76:31
[task 2019-11-06T17:55:27.928Z] 
[task 2019-11-06T17:55:27.928Z] Cannot call `debuggerClient.mainRoot.listAllWorkers` because property `listAllWorkers` is missing in object type [1].
[task 2019-11-06T17:55:27.928Z] 
[task 2019-11-06T17:55:27.928Z]    src/client/firefox/targets.js:76:31
[task 2019-11-06T17:55:27.928Z]     76|     const { service } = await debuggerClient.mainRoot.listAllWorkers();
[task 2019-11-06T17:55:27.928Z]                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[task 2019-11-06T17:55:27.928Z] 
[task 2019-11-06T17:55:27.928Z] References:
[task 2019-11-06T17:55:27.928Z]    src/client/firefox/types.js:255:13
[task 2019-11-06T17:55:27.928Z]                     v
[task 2019-11-06T17:55:27.928Z]    255|   mainRoot: {
[task 2019-11-06T17:55:27.928Z]    256|     traits: any,
[task 2019-11-06T17:55:27.928Z]    257|     getFront: string => Promise<*>,
[task 2019-11-06T17:55:27.928Z]    258|     listProcesses: () => Promise<{ processes: ProcessDescriptor }>,
[task 2019-11-06T17:55:27.928Z]    259|     on: (string, Function) => void,
[task 2019-11-06T17:55:27.928Z]    260|   },
[task 2019-11-06T17:55:27.928Z]           ^ [1]
[task 2019-11-06T17:55:27.928Z] 
[task 2019-11-06T17:55:27.928Z] 
[task 2019-11-06T17:55:27.928Z] Error ------------------------------------------------------------------------------ src/client/firefox/targets.js:81:36
[task 2019-11-06T17:55:27.929Z] 
[task 2019-11-06T17:55:27.929Z] Cannot call `debuggerClient.mainRoot.getWorker` because property `getWorker` is missing in object type [1].
[task 2019-11-06T17:55:27.929Z] 
[task 2019-11-06T17:55:27.929Z]    src/client/firefox/targets.js:81:36
[task 2019-11-06T17:55:27.929Z]     81|         const workerTarget = await debuggerClient.mainRoot.getWorker(id);
[task 2019-11-06T17:55:27.929Z]                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[task 2019-11-06T17:55:27.929Z] 
[task 2019-11-06T17:55:27.929Z] References:
[task 2019-11-06T17:55:27.929Z]    src/client/firefox/types.js:255:13
[task 2019-11-06T17:55:27.929Z]                     v
[task 2019-11-06T17:55:27.929Z]    255|   mainRoot: {
[task 2019-11-06T17:55:27.929Z]    256|     traits: any,
[task 2019-11-06T17:55:27.929Z]    257|     getFront: string => Promise<*>,
[task 2019-11-06T17:55:27.929Z]    258|     listProcesses: () => Promise<{ processes: ProcessDescriptor }>,
[task 2019-11-06T17:55:27.929Z]    259|     on: (string, Function) => void,
[task 2019-11-06T17:55:27.929Z]    260|   },
[task 2019-11-06T17:55:27.929Z]           ^ [1]
[task 2019-11-06T17:55:27.929Z] 
[task 2019-11-06T17:55:27.929Z] 
[task 2019-11-06T17:55:27.929Z] Error ------------------------------------------------------------------------------------------- src/utils/url.js:41:28
[task 2019-11-06T17:55:27.929Z] 
[task 2019-11-06T17:55:27.929Z] Missing type annotation for `firstUrl`.
[task 2019-11-06T17:55:27.929Z] 
[task 2019-11-06T17:55:27.929Z]    41| export function sameOrigin(firstUrl, secondUrl) {
[task 2019-11-06T17:55:27.929Z]                                   ^^^^^^^^
[task 2019-11-06T17:55:27.929Z] 
[task 2019-11-06T17:55:27.929Z] 
[task 2019-11-06T17:55:27.929Z] Error ------------------------------------------------------------------------------------------- src/utils/url.js:41:38
[task 2019-11-06T17:55:27.929Z] 
[task 2019-11-06T17:55:27.929Z] Missing type annotation for `secondUrl`.
[task 2019-11-06T17:55:27.929Z] 
[task 2019-11-06T17:55:27.929Z]    41| export function sameOrigin(firstUrl, secondUrl) {
[task 2019-11-06T17:55:27.929Z]                                             ^^^^^^^^^
[task 2019-11-06T17:55:27.929Z] 
[task 2019-11-06T17:55:27.929Z] 
[task 2019-11-06T17:55:27.929Z] 
[task 2019-11-06T17:55:27.929Z] Found 6 errors
[task 2019-11-06T17:55:27.929Z] 
[task 2019-11-06T17:55:42.881Z] TEST-UNEXPECTED-FAIL flow | Cannot resolve module `devtools/client/shared/workers-listener.js`.
[task 2019-11-06T17:55:42.881Z] TEST-UNEXPECTED-FAIL flow | Cannot call `actions.ensureHasThread` because property `ensureHasThread` is missing in `Actions` [1].
[task 2019-11-06T17:55:42.881Z] TEST-UNEXPECTED-FAIL flow | Cannot call `debuggerClient.mainRoot.listAllWorkers` because property `listAllWorkers` is missing in object type [1].
[task 2019-11-06T17:55:42.881Z] TEST-UNEXPECTED-FAIL flow | Cannot call `debuggerClient.mainRoot.getWorker` because property `getWorker` is missing in object type [1].
[task 2019-11-06T17:55:42.881Z] TEST-UNEXPECTED-FAIL flow | Missing type annotation for `firstUrl`.
[task 2019-11-06T17:55:42.881Z] TEST-UNEXPECTED-FAIL flow | Missing type annotation for `secondUrl`.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/33e9b3eecd98
Part 1 - Service worker check evaluation runnable should be a debuggee runnable, r=asuth.
https://hg.mozilla.org/integration/autoland/rev/6d02ecfce856
Part 2 - Attach to service workers in debugger when pref is enabled, r=jdescottes,jlast.
https://hg.mozilla.org/integration/autoland/rev/ee2bb2d15659
Part 4 - Update threads and sources after navigation, r=jlast.
https://hg.mozilla.org/integration/autoland/rev/edd1135fd507
Part 5 - Add test for windowless service worker debugging, r=jdescottes.

Comment 21

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f690fa2d0e1f
Backed out 4 changesets for causing hunks failed when backout Bug 1592616 on a CLOSED TREE

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0e88cb72397f
Part 1 - Service worker check evaluation runnable should be a debuggee runnable, r=asuth.
https://hg.mozilla.org/integration/autoland/rev/c48a3caec515
Part 2 - Attach to service workers in debugger when pref is enabled, r=jdescottes,jlast.
https://hg.mozilla.org/integration/autoland/rev/237b6839502b
Part 4 - Update threads and sources after navigation, r=jlast.
https://hg.mozilla.org/integration/autoland/rev/e28f8b76aeb7
Part 5 - Add test for windowless service worker debugging, r=jdescottes.

Comment 23

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/0e88cb72397f
https://hg.mozilla.org/mozilla-central/rev/c48a3caec515
https://hg.mozilla.org/mozilla-central/rev/237b6839502b
https://hg.mozilla.org/mozilla-central/rev/e28f8b76aeb7