Support reliable pausing in service worker event handlers
Categories
(DevTools :: Debugger, enhancement)
Tracking
(firefox73 fixed)
Tracking | Status | |
---|---|---|
firefox73 | --- | fixed |
People
(Reporter: bhackett1024, Assigned: bhackett1024)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
Attachments
(3 files)
We don't yet have a way to interrupt a service worker such that we can guarantee the devtools will have attached to the worker thread and installed breakpoints and other traps. This should be fixed, so that breakpoints set in the service worker's handlers can be reliably hit.
I'm not sure if it's possible to hit breakpoints reliably in the service worker's main script. When I wrote the first patch in bug 1563607 the client could attach to service workers using the same mechanism we use for normal workers (block the worker script from executing at startup), but since then the parent intercept mode has been enabled and I haven't been able to get this to work anymore (after trying for several hours). I'm not sure about this diagnosis and maybe this could be made to work. Another option would be to allow the devtools to interrupt execution of the different event listeners the worker sets up (install, activate, fetch) so that the thread actor can be created first.
Assignee | ||
Comment 1•5 years ago
|
||
Bug 1595964 is the missing piece of the puzzle from comment 0. When we pause a service worker before its main script starts, it will be in the evaluating state. This is not exposed to the devtools, but fixing that makes it straightforward for the debugger to attach to the service worker before its main script has started and ensure we can hit breakpoints anywhere in the service worker's script.
With this fix breakpoints should be hit reliably, except in cases where the SW starts running in a new content process before the debugger attaches and notifies it to pause matching service workers. I haven't noticed this causing a problem in testing, and I think it would be best to deal with another time for a couple reasons:
- The target code is actively undergoing changes due to Fission. I've been kicked off that project and I don't feel comfortable working in this area anymore.
- After Fission the content process containing the SW will normally be the same as the one with the content for that origin, which content toolboxes will already have attached to (since they contain content which the toolbox is targeting).
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 2•5 years ago
|
||
Depends on D54288
Assignee | ||
Comment 3•5 years ago
|
||
Depends on D54292
Assignee | ||
Comment 4•5 years ago
|
||
Depends on D54293
Comment 5•5 years ago
|
||
To elaborate on comment 1 slightly:
- Right now ServiceWorkers are spawned randomly in existing content processes under parent-intercept. The only case the ServiceWorker would spawn a new process on its own would be if there were no content processes to begin with.
- This will likely change to be more deterministic, and the change will likely be to be same-process as the initiating document/window because of general SPECTRE concerns, despite performance concerns.
- Indeed, under fission rules, the ServiceWorker will definitely end up in one of the matching set of fission processes, which probably is a set of one.
Comment 7•5 years ago
|
||
Backed out 6 changesets (Bug 1595964, Bug 1596939) for causing failures in browser_application_panel_list-domain-workers.js
Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=280136365&resultStatus=testfailed%2Cbusted%2Cexception&revision=65c870147654362c1bfa0013726498943636278c
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=280136365&repo=autoland&lineNumber=987
Backout: https://hg.mozilla.org/integration/autoland/rev/ae020aef580f046a2e5277b726c3e2d4f50734e1
Comment 9•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/efaae247045d
https://hg.mozilla.org/mozilla-central/rev/1bcf78aceaf6
https://hg.mozilla.org/mozilla-central/rev/12842404e970
Assignee | ||
Updated•5 years ago
|
Description
•