Closed Bug 1596939 Opened 1 year ago Closed 1 year ago

Support reliable pausing in service worker event handlers

Categories

(DevTools :: Debugger, enhancement)

enhancement
Not set
normal

Tracking

(firefox73 fixed)

RESOLVED FIXED
Firefox 73
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.

Depends on: 1595964

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: nobody → bhackett1024

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.
Depends on: 1601279
Pushed by bhackett@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a6848f4d219d
Part 1 - Service worker operations should be debuggee runnables, r=asuth.
https://hg.mozilla.org/integration/autoland/rev/c21c15689e77
Part 2 - Support pausing new service workers for a specific origin, r=jdescottes.
https://hg.mozilla.org/integration/autoland/rev/74a982a9144a
Part 3 - Pause new service workers for the target's origin, r=jlast.
Pushed by bhackett@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/efaae247045d
Part 1 - Service worker operations should be debuggee runnables, r=asuth.
https://hg.mozilla.org/integration/autoland/rev/1bcf78aceaf6
Part 2 - Support pausing new service workers for a specific origin, r=jdescottes.
https://hg.mozilla.org/integration/autoland/rev/12842404e970
Part 3 - Pause new service workers for the target's origin, r=jlast.
Flags: needinfo?(bhackett1024)
You need to log in before you can comment on or make changes to this bug.