1588007 - Cannot register a fetch event listener breakpoint

Status: RESOLVED FIXED

(DevTools :: Debugger, enhancement, P3)

Description

[(no longer active)]

Thank you for helping make Firefox better. If you are reporting a defect, please complete the following:

What were you doing?

Please tell us what site you were on, and what steps led to the error you are reporting

1. Go to https://www.youtube.com/
2. Go to about:debugging, This Nightly, select Youtube's service worker, click on Inspect.
3. Select Debugger, expend Event Listener Breakpoints, in the search box type "fetch".

What happened?

No results found.

What should have happened?

I should be able to register a fetch event listener breakpoint.

Comment 1

[David Walsh :davidwalsh]

How difficult would this be Logan? I recall we were having a problem with fetch and images; is that still a problem?

Comment 2

[Logan Smyth [:loganfsmyth]]

I think this would be pretty straightforward to do. I think all we need is to add

{
    name: "Service Worker",
    items: [
      globalEvent("serviceworker", "fetch"),
    ]
}

to devtools/server/actors/utils/event-breakpoints.js.

The main thing is that this would also trigger if a fetch event were dispatched on a Window or any other global. We don't currently differentiate the type of global at all. That may be fine though, since it's not an event name I'd expect to appear anywhere else anyway.

One other concern could potentially be that we have a Worker section already, but that's actually about the Worker object not the worker global, which could be vaguely confusing for users, but it is what it is.

Comment 3

[:Harald Kirschner :digitarald]

The main thing is that this would also trigger if a fetch event were dispatched on a Window or any other global. We don't currently differentiate the type of global at all. That may be fine though, since it's not an event name I'd expect to appear anywhere else anyway.

Having a new Service Worker section would make sense as they offer more events like onactivate, and oninstall. They also have message which should be already covered by Worker. I'd be also fine with merging them under Worker for now, as search will make them discoverable.

David, given Logan's guidance, this seems to be a good-first-bug?

Comment 4

[David Walsh :davidwalsh]

I'd be afraid to tell someone that anything server-related is a good first bug.

Comment 5

[:Harald Kirschner :digitarald]

:logan, seems like this would be a quick fix for you?

Comment 6

[Logan Smyth [:loganfsmyth]]

I can if needed, but I agree with Jason that this seems like a good first bug. It may be in the server, but it's a relatively minor change and could be a good first task for someone looking to get into the server code a bit.

Comment 7

[Andrew Sutherland [:asuth] (he/him)]

Note that there is special handling for "fetch" in ServiceWorkers that might need/want some special-casing. Specifically, our implementation and the spec track whether a "fetch" listener was added during install and this information is saved and used to determine whether to send "fetch" events to the ServiceWorker. Fetch events will never be dispatched if a handler wasn't added during install time. If, at subsequent runtimes, a "fetch" handler is added, we generate a warning:

• Spec update that landed: https://github.com/whatwg/dom/commit/7a48d64b41c586afc2e57d25eab99e8cfba5de1a
• Our implementation hookup to addEventListener from bug 1181127: https://searchfox.org/mozilla-central/rev/8b7aa8af652f87d39349067a5bc9c0256bf6dedc/dom/workers/WorkerScope.cpp#784
• Our exposure to devtools of the per-ServiceWorker flag: https://searchfox.org/mozilla-central/rev/8b7aa8af652f87d39349067a5bc9c0256bf6dedc/dom/interfaces/base/nsIServiceWorkerManager.idl#61

Comment 8

[Logan Smyth [:loganfsmyth]]

With the way that event listener breakpoints are implemented, that doesn't sound like something that would be an issue thankfully.

Comment 9

[Chujun Lu]

Attachment: Bug 1588007-Add fetch event listener breakpoint for service worker r=loganfsmyth

From Harald's comment, it seems there're more events to add, hence making a service worker section.
I didn't find a related test in server. Let me know if I miss any!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ffd9d340f304
Add fetch event listener breakpoint for service worker r=loganfsmyth

Comment 11

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/ffd9d340f304