1599926 - Step into messages passed between workers/iframes/etc

Status: NEW

(DevTools :: Debugger, enhancement, P3)

Description

[:Harald Kirschner :digitarald]

User feedback: https://discourse.mozilla.org/t/debugger-issues-with-iframes/49037

Chrome 65: https://developers.google.com/web/updates/2018/01/devtools#stepping

Comment 1

[Logan Smyth [:loganfsmyth]]

The Chrome post there also supports stepping into setTimeout, is that something we'd also want to track here/somewhere?

I think the interesting piece here is figuring out how to coordinate these two different debugger servers. We could detect the user stepping into postMessage (potentially with a tiny bit of new logic) using the same infrastructure we have for event breakpoints on setTimeout and such. It seems like ideally, there'd be a flag that we could set on the message, just before it is sent, to say "the worker receiving this event should pause when it receives it" or something like that. We could then set that flag when the step-in is detected, and then the worker could get a event-breakpoint-like notification just before calling the message handler, and then worker server could set up the onStep hooks.

Brian, since you've looked into workers before, do you have a sense for if that would make sense, and if so how complicated it would be to have an additional flag sent along with the message? Do any alternatives come to mind? We could of course try to have the client itself coordinate between the threads, but that seems like it would be difficult and prone to races.

Comment 2

[Brian Hackett [Laid off!]]

I guess I would prefer using conditional breakpoints based on some identifying information in the message (if there even is any, I don't know...) so that additional C++ state isn't needed. I don't see an easy way to implement this though and don't have a strong opinion.

Comment 3

[Logan Smyth [:loganfsmyth]]

Fair enough! I don't know how we'd be able to differentiate the message when it is received in the worker without have some data associated with it, but without digging into things more.

The conditional breakpoint part is what I'd like to avoid because it seems like to do that we'd need a flow of:

1. User is paused somewhere
2. User runs step-in so we resume execution waiting for onEnterFrame
3. We detect that they hit postMessage
4. We then pause again and send a message to the client telling it that it needs to tell an entirely different debugger server to add a conditional breakpoint
5. Once the conditional breakpoint is set, the client has to tell the server to resume
6. The conditional breakpoint is hit

whereas if there is some data passed to the worker directly, with something along the lines of:

// Main thread calling `postMessage`
function onDebuggerNotification(notification) {
  if (notification.type === "postMessage" &&  this.steppingMode === "step-in") {
    SetPrivateDebuggerPauseFlag(notification.event);
  }
}

// Worker thread waiting for onMessage
function onDebuggerNotification(notification) {
  if (notification.type === "domEvent" && notification.event.type === "message" && HasPrivateDebuggerPauseFlag(notification.event)) {
    // register normal step-in logic on this debugger server.
  }
}

Comment 4

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

Do we already have some kind of plan in place for tracing causality in devtools? Like a "we got here from a user click which created a setTimeout" thing? I know there's backtrack (bug 1507282) targeted more at the gecko developer base, but it seems like in many ways that's what wanted here. A way that we can flow labels through control/data-flow so that the debugger can propagate a desire to pause things and also explain how we got somewhere.

In the worker debugging space, because we already have a hierarchy of runnable sub-classes and in particular WorkerDebuggeeRunnable, we could create a toy implementation of this that works for Worker.postMessage and DedicatedWorkerGlobalScope.postMessage once the debugger calls some hook like setPropagatingDebuggerLabel/clearPropagatingDebuggerLabel that would border the actual calls to postMessage. But this would be relatively one-off. Something more comprehensive would need to be involved for when IPC gets involved for MessagePort.postMessage, Client.postMessage, ServiceWorker.postMessage, etc.