Closed Bug 1518884 Opened 5 years ago Closed 5 years ago

Clarify meaning of a source

Categories

(DevTools :: Debugger, enhancement, P2)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bhackett1024, Assigned: bhackett1024)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Attached patch WIPSplinter Review

'Source' can mean several different things in the devtools server and client code. In the server, a source can be a Debugger.Source, a source actor representing a Debugger.Source, or a source actor representing an HTML file containing several Debugger.Sources for inline scripts. In the client, a source can be one of the above source actors, or an original file whose contents are source-mapped to the contents of a source actor.

This has been the source of problems such as bug 1517167, and it would be nice for maintainability and for future work (such as showing non-JS files in the debugger's editor) if we distinguished between these cases better in the debugger, with a new abstraction:

  • 'Source' is a Debugger.Source (in the server), or a source actor which is 1:1 with a Debugger.Source.

  • 'Resource' is something that is shown in an editor tab, and may have an entry in the debugger's sources pane.

A source may map to one or more resources --- when a source has a source map, there can be resources for both the generated JS and the original URL which was being mapped. A resource may map to one or more sources --- an HTML file can be represented as a resource, which may have multiple inline scripts. Mapping between sources and resources is controlled in some centralized place by the redux state.

Sources are particular to a debugging session and information about them will not persist afterwards. Resources may have a URL which can be used to persist state.

Other parts of the redux store need to consistent / well typed in terms of whether they are referring to sources or resources. For example, a frame can have both a generated location referring to a source, and an original location referring to a resource. An installed breakpoint refers to a source, but persisted information can only refer to a resource's URL.

The attached patch is a WIP that removes the ability for a source actor to correspond with an HTML file, and generates a source in the client for that HTML file which maps to the HTML's inline sources. This doesn't have the separate notion of a resource as described above. The patch has gotten kind of hung up getting everything to work in the client, on account of the difficulties in figuring out what should refer to the HTML file's source vs. the inline sources when these have the same type/representation. The source vs. resource distinction described here should let the patch move forward, but it might be best to first make sure this is a change we actually want to make.

Priority: -- → P2

I've been thinking about this in the context of some issues with windowless worker debugging, and think a few more refinements are in order here. The main issue is, when sources and breakpoints are associated with threads, how do we match up those threads when loading a page with persistently stored tabs / breakpoints?

I don't think there is a good answer to this question. We could match up threads based on what URL they were initially opened with, but when there are multiple threads for the same URL and sources/breakpoints are associated with a single thread, this breaks down. We could number the threads that have opened the same URL, but at that point things are just getting baroque.

It would be simpler and more robust if we didn't associate persistently stored tabs and breakpoints with threads. The refinement to make is that, per the nomenclature in comment 0, sources are associated with specific threads, and resources are not. I'd like to redo the nomenclature above, though. Within the client:

  • 'SourceActor' describes an actor representing a ScriptSource in some thread.

  • 'Source' describes a resource that is associated with zero or more SourceActors at any given time, but is not tied to a specific thread or actor.

Similarly, we would have 'BreakpointActor' to describe a breakpoint installed on a SourceActor, and 'Breakpoint' to describe a breakpoint installed on a Source, and which is associated with zero or more BreakpointActors at any given time.

The client maintains the relationships between SourceActor/Source and BreakpointActor/Breakpoint, but the Actor objects are only used when it is time to interact with the server. In almost all cases, user interactions in the client operate on Sources and Breakpoints, which are independent of the underlying thread.

This changes the experience for the user, somewhat. We can still separate sources for each thread in the sources pane, but if the same URL appears for different threads then clicking those URLs will go to the same tab, i.e. we can't have multiple tabs open for the same URL.

Similarly, setting a breakpoint in a source will set it for all threads. I believe this is the most intuitive behavior for users. If desired, a breakpoint could be set for a specific thread, but it would need some UI support. Similar to setting the condition or log value, users would need to specify which threads the breakpoint is active for; this can be easily carried down into the server via the BreakpointOptions structure (bug 1520972), and while the breakpoint would be installed on every thread it just would not hit for threads it isn't active in. I think we'd want to wait and make sure we need this feature before implementing it, but it at least can be supported with this design.

The client side changes here bring sources and breakpoints to a state similar to that before bug 1241958. I'd like to focus first on making these client side changes by themselves. Then, having a Source -> SourceActor[] map in the client should make it much easier to move the server-side handling of multiple-sources-per-HTML-file into the client.

This patch covers client side changes for the Source/SourceActor and Breakpoint/BreakpointActor split. This is almost done, but a few unit tests are still failing.

This refactoring has landed to github.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: