Support extracting source mapping url defined by //@ sourceMappingURL=<url> comment. (issue7045003)

35 views
Skip to first unread message

podi...@chromium.org

unread,
May 18, 2011, 9:21:17 AM5/18/11
to mnag...@chromium.org, v8-...@googlegroups.com
Reviewers: Mikhail Naganov (Chromium),

Description:
Support extracting source mapping url defined by //@ sourceMappingURL=<url>
comment.


Please review this at http://codereview.chromium.org/7045003/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files:
M src/messages.js
M src/mirror-debugger.js
M test/mjsunit/debug-compile-event.js


Index: src/messages.js
diff --git a/src/messages.js b/src/messages.js
index
de388214c7dffeec6b4ae71e353e29184ccb46cc..4fd7c4b411e252a7aaa19cfbcd57bec73511fd84
100644
--- a/src/messages.js
+++ b/src/messages.js
@@ -497,6 +497,26 @@ Script.prototype.lineCount = function() {
Script.prototype.nameOrSourceURL = function() {
if (this.name)
return this.name;
+ return this.findMagicComment("sourceURL");
+}
+
+
+/**
+ * Returns script source mapping url, if present.
+ *
+ * @return {?string} script source mapping url, if present.
+ */
+Script.prototype.sourceMappingURL = function() {
+ return this.findMagicComment("sourceMappingURL");
+}
+
+
+/**
+ * Returns the value of the magic comment "//@ name=<value>", if present.
+ *
+ * @return {?string} the value of the magic comment, if present.
+ */
+Script.prototype.findMagicComment = function(name) {
// TODO(608): the spaces in a regexp below had to be escaped as \040
// because this file is being processed by js2c whose handling of spaces
// in regexps is broken. Also, ['"] are excluded from allowed URLs to
@@ -504,21 +524,20 @@ Script.prototype.nameOrSourceURL = function() {
// A better solution would be to detect these special comments in
// the scanner/parser.
var source = ToString(this.source);
- var sourceUrlPos = %StringIndexOf(source, "sourceURL=", 0);
- if (sourceUrlPos > 4) {
- var sourceUrlPattern =
- /\/\/@[\040\t]sourceURL=[\040\t]*([^\s\'\"]*)[\040\t]*$/gm;
+ var position = %StringIndexOf(source, name + "=", 0);
+ if (position > 4) {
+ var pattern = "\/\/@[ \t]" + name
+ "=[\040\t]*([^\\s\'\"]*)[\040\t]*$";
+ var regexp = new $RegExp(pattern, "gm");
// Don't reuse lastMatchInfo here, so we create a new array with room
// for four captures (array with length one longer than the index
// of the fourth capture, where the numbering is zero-based).
var matchInfo = new InternalArray(CAPTURE(3) + 1);
var match =
- %_RegExpExec(sourceUrlPattern, source, sourceUrlPos - 4,
matchInfo);
+ %_RegExpExec(regexp, source, position - 4, matchInfo);
if (match) {
return SubString(source, matchInfo[CAPTURE(2)],
matchInfo[CAPTURE(3)]);
}
}
- return this.name;
}


Index: src/mirror-debugger.js
diff --git a/src/mirror-debugger.js b/src/mirror-debugger.js
index
3a0353515c1d449053959fd2049d60015b3c54c7..a544be999953c9a7d9377045c2c4a5da6548009c
100644
--- a/src/mirror-debugger.js
+++ b/src/mirror-debugger.js
@@ -1782,6 +1782,11 @@ ScriptMirror.prototype.source = function() {
};


+ScriptMirror.prototype.sourceMappingURL = function() {
+ return this.script_.sourceMappingURL();
+};
+
+
ScriptMirror.prototype.lineOffset = function() {
return this.script_.line_offset;
};
Index: test/mjsunit/debug-compile-event.js
diff --git a/test/mjsunit/debug-compile-event.js
b/test/mjsunit/debug-compile-event.js
index
b00a907a3cf79f4d1316fff559751f479549febc..581ad41b91267071a84ab3fd8adba9d1ab624263
100644
--- a/test/mjsunit/debug-compile-event.js
+++ b/test/mjsunit/debug-compile-event.js
@@ -81,9 +81,15 @@ function listener(event, exec_state, event_data, data) {
assertTrue('context' in msg.body.script);

// Check that we pick script name from //@ sourceURL, iff present
- assertEquals(current_source.indexOf('sourceURL') >= 0 ?
+ assertEquals(current_source.indexOf('sourceURL') >= 0 ?
'myscript.js' : undefined,
event_data.script().name());
+
+ // Check that we pick source mapping url from //@ sourceMappingURL,
if
+ // present.
+ assertEquals(current_source.indexOf('sourceMappingURL') >= 0 ?
+ 'http://example.com/sourceMapping.json' : undefined,
+ event_data.script().sourceMappingURL());
}
} catch (e) {
exception = e
@@ -103,7 +109,8 @@ compileSource('eval("eval(\'(function(){return
a;})\')")');
source_count += 2; // Using eval causes additional compilation event.
compileSource('JSON.parse(\'{"a":1,"b":2}\')');
// Using JSON.parse does not causes additional compilation events.
-compileSource('x=1; //@ sourceURL=myscript.js');
+compileSource('x=1; //@ sourceURL=myscript.js\n' +
+ '//@
sourceMappingURL=http://example.com/sourceMapping.json');

// Make sure that the debug event listener was invoked.
assertFalse(exception, "exception in listener")


mnag...@chromium.org

unread,
May 18, 2011, 9:49:08 AM5/18/11
to podi...@chromium.org, v8-...@googlegroups.com
On 2011/05/18 13:21:17, podivilov wrote:

Can you please elaborate more on explaining why this is needed, and how
this can
be used?

http://codereview.chromium.org/7045003/

podi...@chromium.org

unread,
May 18, 2011, 10:08:23 AM5/18/11
to mnag...@chromium.org, v8-...@googlegroups.com
On 2011/05/18 13:49:08, Mikhail Naganov (Chromium) wrote:
> On 2011/05/18 13:21:17, podivilov wrote:

> Can you please elaborate more on explaining why this is needed, and how
> this
can
> be used?

When debugging compiled script in chrome dev tools, user will have an
option to
specify source mapping location and to debug sources rather then compiled
code.
Also, compiler may emit //@ sourceMappingURL=<url> comment in compiled
script.
In that case we will pick up the mapping automatically.

Please see chrome feature request
http://code.google.com/p/chromium/issues/detail?id=26912.


http://codereview.chromium.org/7045003/

mnag...@chromium.org

unread,
May 19, 2011, 2:32:08 AM5/19/11
to podi...@chromium.org, v8-...@googlegroups.com
I think, this change is premature.

Does any of JS compilers actually emit such a comment? No.

And I'm sceptic that any of JS compilers will ever support emitting such
URLs
because application compilation and deployment are logically separate
steps, and
the compiler might not know, how the URL will look like. Using relative URLs
might help, but that looks lame.

To me, a better approach would be to mimic the behavior of debugging symbols
loading for native applications. That is, user specifies a directory (or a
base
URL) for mappings, and in that dir, for each JS file a corresponding mapping
file can be found, named in some predictable manner. This requires no
support
from a VM engine, all symbol handling can be implemented in a debugger.

Pavel Podivilov

unread,
May 19, 2011, 5:02:22 AM5/19/11
to podi...@chromium.org, mnag...@chromium.org, v8-...@googlegroups.com
I think, this change is premature.

Does any of JS compilers actually emit such a comment? No.

We already agreed about sourceMappingURL comment with John Lenz, so it's just a matter or time.
 

And I'm sceptic that any of JS compilers will ever support emitting such URLs
because application compilation and deployment are logically separate steps, and
the compiler might not know, how the URL will look like. Using relative URLs
might help, but that looks lame.

Compiler may emit a template that will be populated during deployment.
 

To me, a better approach would be to mimic the behavior of debugging symbols
loading for native applications. That is, user specifies a directory (or a base
URL) for mappings, and in that dir, for each JS file a corresponding mapping
file can be found, named in some predictable manner. This requires no support
from a VM engine, all symbol handling can be implemented in a debugger.

In any case user will have an option to specify mappings location explicitly.  This change is just about providing a way to do it automatically.

I'm not blocked by this cl, so we may defer it until there is a real usage.
Reply all
Reply to author
Forward
0 new messages