Last Comment Bug 771597 - (dbg-sourcemap) [meta] Debugger source mapping
(dbg-sourcemap)
: [meta] Debugger source mapping
Status: NEW
: feature, meta
Product: Firefox
Classification: Client Software
Component: Developer Tools: Debugger (show other bugs)
: unspecified
: x86 Mac OS X
: P5 normal with 19 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on: 856875 897594 904291 905863 907440 910204 929116 956088 965856 990815 992268 1012873 1013547 1017938 1056988 1112435 1128766 1134566 1135855 1167725 1191558 1228496 1228500 1228503 1237593 669999 715181 765993 772113 772119 840684 849069 849071 852792 860035 865252 870140 878307 881939 917583 927158 956466 1007565 1084534 1087054 1090768 1100367 1129157 1134563 1149115 1152032 1154606 1177329 1188697 1188982 1238734
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-06 11:34 PDT by Nick Fitzgerald [:fitzgen] [⏰PDT; UTC-7]
Modified: 2016-02-22 11:45 PST (History)
38 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments

Description Nick Fitzgerald [:fitzgen] [⏰PDT; UTC-7] 2012-07-06 11:34:28 PDT
When there is a source map for the script being debugged, the debugger should use it to display the original source and allow break points to be set referencing the original source.
Comment 1 Panos Astithas [:past] (away until 7/21) 2012-07-06 12:08:58 PDT
One way to do it would be to keep an additional map in the thread actor next to _scripts, and retrieve the original file in TA__addScript. After that the client would receive an additional 'unminified' property (or whatever) in the 'scripts' response or the 'newScript' notification.

This assumes a post-bug 755661 world, where the script text is retrieved through the protocol and not by the client independently, as we currently do. This world is not that far off, actually.
Comment 2 Nick Fitzgerald [:fitzgen] [⏰PDT; UTC-7] 2012-07-06 13:44:26 PDT
(In reply to Panos Astithas [:past] from comment #1)
> One way to do it would be to keep an additional map in the thread actor next
> to _scripts, and retrieve the original file in TA__addScript. After that the
> client would receive an additional 'unminified' property (or whatever) in
> the 'scripts' response or the 'newScript' notification.

This makes sense to me.

Because multiple original sources can create a single generated source (for example, module imports concat'd together), I imagine this property would be an object of the form { <original source url> : <original source contents> } or something similar. I think we'd also need to add a _sourceMaps property to ThreadActor as well, which would be of the form { <script> : <SourceMapConsumer> } where <script> would be some way of hashing a Debugger.Script (urls wouldn't work if we want to support eval'd scripts).

> This assumes a post-bug 755661 world, where the script text is retrieved
> through the protocol and not by the client independently, as we currently
> do. This world is not that far off, actually.

Are you saying that this bug should depend on bug 755661?

Obviously I'm not as familiar with the debugger as you, but I'm not sure that should be the case here. The original sources are never actually executed by spidermonkey so the problem of the debugger and the debuggee getting different versions of the same script doesn't really apply here. Even after that bug is fixed, I still imagine the client would be the one fetching the original sources.

Am I overlooking something?
Comment 3 Panos Astithas [:past] (away until 7/21) 2012-07-08 00:05:44 PDT
(In reply to Nick Fitzgerald :fitzgen from comment #2)
> (In reply to Panos Astithas [:past] from comment #1)
> > One way to do it would be to keep an additional map in the thread actor next
> > to _scripts, and retrieve the original file in TA__addScript. After that the
> > client would receive an additional 'unminified' property (or whatever) in
> > the 'scripts' response or the 'newScript' notification.
> 
> This makes sense to me.
> 
> Because multiple original sources can create a single generated source (for
> example, module imports concat'd together), I imagine this property would be
> an object of the form { <original source url> : <original source contents> }
> or something similar. I think we'd also need to add a _sourceMaps property
> to ThreadActor as well, which would be of the form { <script> :
> <SourceMapConsumer> } where <script> would be some way of hashing a
> Debugger.Script (urls wouldn't work if we want to support eval'd scripts).

Makes sense.

> > This assumes a post-bug 755661 world, where the script text is retrieved
> > through the protocol and not by the client independently, as we currently
> > do. This world is not that far off, actually.
> 
> Are you saying that this bug should depend on bug 755661?

If we put source map handling in the server (the script actors) we need to have protocol support for returning script source to the client in order for the editor to display it. This is the main part of the work that I intend to do in bug 755661: move the current script-fetching code in the server (until bug 637572 is done) and extend the remote debugging protocol to allow the client to retrieve it. If we end up with source map handling in the client, then the right place for source map handling would not be the script actors, but the thread client in dbg-client.jsm, or even debugger-controller.js if we don't want to attach it to the debugging protocol.

> Obviously I'm not as familiar with the debugger as you, but I'm not sure
> that should be the case here. The original sources are never actually
> executed by spidermonkey so the problem of the debugger and the debuggee
> getting different versions of the same script doesn't really apply here.
> Even after that bug is fixed, I still imagine the client would be the one
> fetching the original sources.
> 
> Am I overlooking something?

The problem is not that SpiderMonkey may execute the original sources, but that the debugger server is running in the same process as the remote browser. In bug 755661 Mark mentions both accessibility issues (script may not be retrieved by the desktop browser due to missing authentication cookies or even network ACLs) and user agent sniffing issues. I can see both still being valid for source maps: the former for obvious reasons and the latter for files with generic names. For example, if a foo-min.js has a foo.js unminified file and a foo-map.js source map in the minified file, then user agent sniffing would cause the desktop browser to retrieve a different unminified file, that would point to a different source map. Obviously source maps specified with HTTP headers wouldn't face this issue.
Comment 4 Nick Fitzgerald [:fitzgen] [⏰PDT; UTC-7] 2012-07-09 11:39:30 PDT
Kevin,

Panos and I had a conversation on IRC discussing using source maps with scripts where there are multiple versions sent based on the user agent of the requester. For example, if you request foo-min.js on mobile, it might be different than foo-min.js on desktop.

I have edited the IRC logs a bit to clean them up.

This was our original plan:

  fitzgen: as long as the debugger server (on mobile) provides the correct source map url, the debugger client (on desktop) can fetch the source map, and from there the original sources, and everything will be correct
  fitzgen: because the source map can be linked to a script through the source or through an http header
  fitzgen: so that's why we need to extend the protocol
  fitzgen: just add something to the protocol that maps a script to a source map url
  fitzgen: and let the debugger client handle everything involving source maps from there

But then Panos thought of one more scenario:

  past: fitzgen: so, I think there is still a case where this plan won't work:
  fitzgen: yeah?
  past: site serves foo-min.js, which points to foo.map, which points to foo.js
  past: but their server sends a different version for *each* of those files depending on the user agent
  past: including the source map
  past: is that something people do?
  fitzgen: past: I don't think so
  fitzgen: I'd imagine they only serve foo-min.js like that
  past: I wonder if they put all files in /desktop and /mobile paths
  fitzgen: yeah, thats the only use case I can imagine that could lead to that situation
  past: but yeah, if that's not something we're worried about, I'm fine with this plan
  fitzgen: past: how much can we be responsible for? personally, I think that if we make sure we get the url for the source map for the file we are actually debugging, we have done as much as should be expected
  past: that's a reasonable argument
  past: fitzgen: I'd say let's get Kevin's thoughts on this
  past: he may want to do some user research or something

And so, we ask your advice on the matter :)
Comment 5 Rob Campbell [:rc] (:robcee) 2012-07-12 09:24:43 PDT
oh yes.
Comment 6 Kevin Dangoor 2012-07-25 07:37:49 PDT
(In reply to Nick Fitzgerald :fitzgen from comment #4)
>   past: site serves foo-min.js, which points to foo.map, which points to
> foo.js
>   past: but their server sends a different version for *each* of those files
> depending on the user agent
>   past: including the source map
>   past: is that something people do?
>   fitzgen: past: I don't think so
>   fitzgen: I'd imagine they only serve foo-min.js like that
>   past: I wonder if they put all files in /desktop and /mobile paths
>   fitzgen: yeah, thats the only use case I can imagine that could lead to
> that situation
>   past: but yeah, if that's not something we're worried about, I'm fine with
> this plan
>   fitzgen: past: how much can we be responsible for? personally, I think
> that if we make sure we get the url for the source map for the file we are
> actually debugging, we have done as much as should be expected
>   past: that's a reasonable argument
>   past: fitzgen: I'd say let's get Kevin's thoughts on this
>   past: he may want to do some user research or something
> 
> And so, we ask your advice on the matter :)

I think this scenario seems less likely, and I also think that people will ultimately change their build output if there is new tools support that makes their lives better.

So, if we support the same kinds of build output that Chrome does, then I think it's fine for us to get the feature out the door and see if anyone has a good reason for this kind of build setup.
Comment 7 Brian Slesinsky 2013-08-12 19:13:06 PDT
Hi, what's the current status? With Firefox 23 it looks like the debugger shows the source code, but breakpoints don't work right.
Comment 8 Nick Fitzgerald [:fitzgen] [⏰PDT; UTC-7] 2013-08-13 11:30:13 PDT
Brian, initial support was introduced in Firefox 23, but it was semi-incomplete. We have been improving support in every release since then. Can you reproduce your problems on Nightly? If so, please file a bug w/ a test case and steps to reproduce so we can fix it.
Comment 9 Brian Slesinsky 2013-08-13 17:29:30 PDT
I don't have full repro steps yet and for GWT this would be a bit involved, but here are some observations from playing around with GWT's Super Dev Mode and nightly 26.0a1 (2013-08-13):
 
- The Java source shows up fine when I turn sourcemaps on. (The order of the files on the left is unclear; they aren't obviously grouped by directory and they aren't always in alphabetical order either. Fortunately search works.)
- If I set a breakpoint in the Java source, it appears in the listing the left, but the breakpoint has no apparent effect.
- If I turn sourcemaps off, there's no corresponding breakpoint listed in the JavaScript file.
- If I turn sourcemaps back on and remove the Java breakpoint, it doesn't work. The breakpoint doesn't disappear on the left side, and if I reload the page, the breakpoint reappears.
- If I turn off breakpoints, set a breakpoint in the JavaScript, then turn sourcemaps back on, it stops at the correct place in the Java code.
Comment 10 Brian Slesinsky 2013-08-13 17:31:14 PDT
Also, saw this stack trace:

Error: 'delete' request packet has no destination.: DC_request@resource://gre/modules/devtools/dbg-client.jsm:581
@resource://gre/modules/devtools/dbg-client.jsm:328
Breakpoints.prototype.removeBreakpoint@chrome://browser/content/devtools/debugger-controller.js:1441
Breakpoints.prototype.destroy@chrome://browser/content/devtools/debugger-controller.js:1247
DebuggerView._destroyEditor@chrome://browser/content/devtools/debugger-view.js:195
DebuggerView.destroy@chrome://browser/content/devtools/debugger-view.js:82
DebuggerController.shutdownDebugger@chrome://browser/content/devtools/debugger-controller.js:122
DebuggerController._onTabDetached@chrome://browser/content/devtools/debugger-controller.js:239
EventEmitter_emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/shared/event-emitter.js:107
TabTarget.prototype.destroy@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/framework/target.js:407
TBOX_destroy@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/framework/toolbox.js:824
Comment 11 Nick Fitzgerald [:fitzgen] [⏰PDT; UTC-7] 2014-01-03 10:59:28 PST
This meta bug doesn't need an assignee.
Comment 12 Nick Fitzgerald [:fitzgen] [⏰PDT; UTC-7] 2014-09-22 10:17:23 PDT
Eddy, bug 833744 is about //# sourceURL which is distinct from //# sourceMappingURL and has nothing to do with source maps. It's exposed to JS via the Debugger.Source.prototype.displayURL property.
Comment 13 Eddy Bruel [:ejpbruel] 2014-09-23 00:16:07 PDT
(In reply to Nick Fitzgerald [:fitzgen] from comment #12)
> Eddy, bug 833744 is about //# sourceURL which is distinct from //#
> sourceMappingURL and has nothing to do with source maps. It's exposed to JS
> via the Debugger.Source.prototype.displayURL property.

Thanks for pointing that out Nick. I had assumed it had to do with source maps because the name is obviously very similar, and you mentioned the source mapping mailing list in the comments. I'll do a bit more investigation next time ;-)
Comment 14 Eddy Bruel [:ejpbruel] 2014-10-29 04:49:29 PDT
Glad I'm not the only pedant here when it comes to bug summaries Nick! :-P
Comment 15 Liz Henry (:lizzard) (needinfo? me) 2015-04-07 14:44:01 PDT
Marking this as a feature to help with tracking, QA, and general awareness of active/upcoming work.

Note You need to log in before you can comment on or make changes to this bug.