Closed Bug 1563607 Opened 2 years ago Closed 2 years ago

Show active service workers in threads pane

Categories

(DevTools :: Debugger, enhancement, P2)

enhancement

Tracking

(firefox72 fixed)

RESOLVED FIXED
Firefox 72
Tracking Status
firefox72 --- fixed

People

(Reporter: bhackett1024, Assigned: bhackett1024)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [debugger-mvp])

Attachments

(5 files, 2 obsolete files)

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.

Whiteboard: [debugger-mvp]

CC Perry

Flags: needinfo?(perry)

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.

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.

Flags: needinfo?(perry)
Duplicate of this bug: 1536902

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

Whiteboard: [debugger-mvp] → [debugger-reserve]

removing from 70 while we wait on platform

Assignee: bhackett1024 → nobody
No longer blocks: dbg-70
Whiteboard: [debugger-reserve]
Blocks: dbg-71
Duplicate of this bug: 1226285
Duplicate of this bug: 1221892
Whiteboard: [debugger-mvp]

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

Assignee: nobody → bhackett1024
Attached patch WIP (obsolete) — Splinter Review

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.

Attached patch patchSplinter Review

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.

Attachment #9093465 - Attachment is obsolete: true

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.

Attachment #9097146 - Attachment is obsolete: true
Attachment #9097145 - Attachment description: Bug 1563607 Part 2 - Match service workers when debugging browsing contexts and pref is enabled, r=jdescottes. → Bug 1563607 Part 2 - Attach to service workers in debugger when pref is enabled, r=jdescottes.
Pushed by bhackett@mozilla.com:
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.

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`.
Flags: needinfo?(bhackett1024)
Pushed by bhackett@mozilla.com:
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.
Backout by ccoroiu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f690fa2d0e1f
Backed out 4 changesets for causing hunks failed when backout Bug 1592616 on a CLOSED TREE
Pushed by dluca@mozilla.com:
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.
Regressions: 1594699
Flags: needinfo?(bhackett1024)
Regressions: 1598047
Regressions: 1594617
You need to log in before you can comment on or make changes to this bug.