1641121 - All server responses that include a source URL uses should include a source actor ID or some other UUID

Status: NEW

(DevTools :: Debugger, enhancement, P3)

Description

[Logan Smyth [:loganfsmyth]]

In my case, I'm focused on making viewSourceInDebugger work consistently across devtools when opening JS files in the debugger. As it has evolved over time from getting (url, line = 1) to now getting (url, line = 1, column = 0, actorID = null) or something roughly along those lines.

Because the actor ID is optional, we have cases where we don't know what version of a file to open. For instance, maybe the page has hot-reloading for scripts. Do we open the most recent version of the file? There are similar issues if the "original" version of a file from a sourcemap has the same URL as the compiled file, which one do we open? We have no way to know.

Potential issues/open questions I see:

1. I'm not 100% sure if we're guaranteed to have attached a debugger yet when sending things like console messages to the client, so we'll need to either always attach the thread first, or use some other UUID to represent the source before the actor itself has been created.
2. I have not surveyed all of the uses to it's possible that there are some where the source's actor ID can't be derived from the data we currently track for messages and things.
3. https://bugzilla.mozilla.org/show_bug.cgi?id=1592584 seems like it would be a blocker, since messages from other threads have no way to create actors in-thread and get their IDs.

Comment 1

[Logan Smyth [:loganfsmyth]]

@ochameau What are your thoughts on this?

Comment 2

[Alexandre Poirot [:ochameau]]

(In reply to Logan Smyth [:loganfsmyth] from comment #0)

In my case, I'm focused on making viewSourceInDebugger work consistently across devtools when opening JS files in the debugger. As it has evolved over time from getting (url, line = 1) to now getting (url, line = 1, column = 0, actorID = null) or something roughly along those lines.

Because the actor ID is optional, we have cases where we don't know what version of a file to open. For instance, maybe the page has hot-reloading for scripts. Do we open the most recent version of the file? There are similar issues if the "original" version of a file from a sourcemap has the same URL as the compiled file, which one do we open? We have no way to know.

Potential issues/open questions I see:

1. I'm not 100% sure if we're guaranteed to have attached a debugger yet when sending things like console messages to the client, so we'll need to either always attach the thread first, or use some other UUID to represent the source before the actor itself has been created.

Your questioning comes in a timely manner as I just moved some code related to console message and was wondering about excactly that!
https://phabricator.services.mozilla.com/D77441?id=290263#inline-450529
We have a followup bug where we want to review and cleanup all the code I just moved.
Including this call to target.attach() which, I think, is there just for this sole reason: having the debugger and thread actor ready to compute source actor IDs.
So, to reply to your question, at least for console messages, we ensure that the debugger is attached for the possibly sole reason of computing source actor IDs.

2. I have not surveyed all of the uses to it's possible that there are some where the source's actor ID can't be derived from the data we currently track for messages and things.

It would be great to do such survey. See what are the callsites of viewSourceInDebugger. We might hit all these usages while working on fission. It might be useful to anticipate them and know what type of resources relates to JS Sources.

3. https://bugzilla.mozilla.org/show_bug.cgi?id=1592584 seems like it would be a blocker, since messages from other threads have no way to create actors in-thread and get their IDs.

This bug is a blocker for many things, while not being a blocker for Fission (Yet?). Many improvement required by Fission will help enabling this, but we might not have time to focus on just this until Fission support is decent. It means by end of Fission M2 milestone, which we forecast around September/October.
I imagine that if we were using platform UUIDs instead of DevTools actor IDs, we could probably pass them around from worker to main thread? But I'm not sure that's a very useful work as we know we want to enable actors in worker threads sooner than later. And this work to pass data between threads will be made obsolete.

So. We should probably coordinate with:

• bug 1642297, where we will investigate if console messages depend on ThreadActor just for SourceID computation or not. If it depends on just that, we can probably try to uncouple sources and make it easier to reason about sources independently of the ThreadActor.
• bug 1644188, where we will change how to retrieve sources in the frontend
• bug 1620280, where we will change how to retrieve sources in the backend
  In the two last bugs, I was expecting to keep SourceActor/SourceFront. But I'm open to any change/simplification.
  Also note, that once this is done, it may be actually easier to change how we handle sources as we will significantly simply on both sides how to listen for sources.

Then, should we use Source Actor ID or some platform UUID?

• Actor ID would probably be easier, as existing code already uses this and using something else would require some backward compat code.
• Platform unique IDs, may help:
  • use such IDs outside of DevTools, or before/after DevTools are opened (ContentDOMReference is such platform ID for DOM Element and it appears to be useful)
  • may help switch to CDP if we want to in future. Or implement debugger related features in /remote/. CDP doesn't have this notion of actors/fronts and is rather involving per resource type unique IDs.

Otherwise, about the frontend codebase, if we are using fronts, may be the API should be: (sourceFront, line, column)?

Comment 3

[Logan Smyth [:loganfsmyth]]

Here's the current state of things for places that eventually call toolbox.viewSourceInStyleEditorByURL and toolbox.viewSourceInDebugger (without a guaranteed sourceID).

• CSS error Console messages

  • No stylesheet actor IDs, opening files relies on URL.

• Network Event Console messages

  • No source actor ID known (and would need multiple to handle HTML w/ multiple scripts)
  • Possibly complicated by the fact that the trace may not be in same thread?
    • https://searchfox.org/mozilla-central/rev/027893497316897b8f292bde48dbb6da2391a331/devtools/server/actors/network-monitor/network-event.js#118-121

• JS error and log messages

  • Will not have an actor ID if the thread actor is not attached
  • Not entirely sure if every message is guaranteed to have a URL in a debuggee?

• EventTooltipHelper

  • Actor ID unavailable if the handler is not a debuggee or is not an "allowed source"

• Element inspector "jump to custom element"

  • No source actor IDs, opening files relies on URL.

• NetMonitor Request stacktraces

  • No source actor IDs, opening files relies on URL.

• Memory Pane Allocation Stacks

  • No source actor IDs, opening files relies on URL.

• Perf Recordings Stacks

  • No source actor IDs, opening files relies on URL.

EDIT: Missed one:

• "Jump to Definition" in Reps for functions jumps to the function definition and provides no source actor ID