Bug 1074745 - allow setting sourceMapURL of a Debugger.Source and move warning to compiler. r=jimb
authorJames Long <longster@gmail.com>
Wed, 19 Nov 2014 16:02:00 +0100
changeset 216614 7775bda3aa20620e9bac01b004dd3a10bdb3f3f8
parent 216613 97bee197641244562ea79a9b9be7549027dc91c9
child 216615 73c4111cab17d7351f26d691a04266b71cecf445
push idunknown
push userunknown
push dateunknown
reviewersjimb
bugs1074745
milestone36.0a1
Bug 1074745 - allow setting sourceMapURL of a Debugger.Source and move warning to compiler. r=jimb
js/src/doc/Debugger/Debugger.Source.md
js/src/frontend/BytecodeCompiler.cpp
js/src/jit-test/tests/debug/Source-sourceMapURL-deprecated.js
js/src/jit-test/tests/debug/Source-sourceMapURL.js
js/src/jsscript.cpp
js/src/vm/Debugger.cpp
--- a/js/src/doc/Debugger/Debugger.Source.md
+++ b/js/src/doc/Debugger/Debugger.Source.md
@@ -80,16 +80,21 @@ from its prototype:
     the source positions in this source to the corresponding source
     positions in the original source, then this property's value is that
     URL. Otherwise, this is `null`.
 
     (On the web, the translator may provide the source map URL in a
     specially formatted comment in the JavaScript source code, or via a
     header in the HTTP reply that carried the generated JavaScript.)
 
+    This property is writable, so you can change the source map URL by
+    setting it. All Debugger.Source objects referencing the same
+    source will see the change. Setting an empty string has no affect
+    and will not change existing value.
+
 `element`
 :   The [`Debugger.Object`][object] instance referring to the DOM element to which
     this source code belongs, if any, or `undefined` if it belongs to no DOM
     element. Source belongs to a DOM element in the following cases:
 
     * Source belongs to a `<script>` element if it is the element's text
       content (that is, it is written out as the body of the `<script>`
       element in the markup text), or is the source document referenced by its
--- a/js/src/frontend/BytecodeCompiler.cpp
+++ b/js/src/frontend/BytecodeCompiler.cpp
@@ -50,16 +50,17 @@ SetDisplayURL(ExclusiveContext *cx, Toke
     }
     return true;
 }
 
 static bool
 SetSourceMap(ExclusiveContext *cx, TokenStream &tokenStream, ScriptSource *ss)
 {
     if (tokenStream.hasSourceMapURL()) {
+        MOZ_ASSERT(!ss->hasSourceMapURL());
         if (!ss->setSourceMapURL(cx, tokenStream.sourceMapURL()))
             return false;
     }
     return true;
 }
 
 static bool
 CheckArgumentsWithinEval(JSContext *cx, Parser<FullParseHandler> &parser, HandleFunction fun)
@@ -409,16 +410,23 @@ frontend::CompileScript(ExclusiveContext
     if (!SetSourceMap(cx, parser.tokenStream, ss))
         return nullptr;
 
     /*
      * Source map URLs passed as a compile option (usually via a HTTP source map
      * header) override any source map urls passed as comment pragmas.
      */
     if (options.sourceMapURL()) {
+        // Warn about the replacement, but use the new one.
+        if (ss->hasSourceMapURL()) {
+            if(!parser.report(ParseWarning, false, nullptr, JSMSG_ALREADY_HAS_PRAGMA,
+                              ss->filename(), "//# sourceMappingURL"))
+                return nullptr;
+        }
+
         if (!ss->setSourceMapURL(cx, options.sourceMapURL()))
             return nullptr;
     }
 
     /*
      * Nowadays the threaded interpreter needs a last return instruction, so we
      * do have to emit that here.
      */
--- a/js/src/jit-test/tests/debug/Source-sourceMapURL-deprecated.js
+++ b/js/src/jit-test/tests/debug/Source-sourceMapURL-deprecated.js
@@ -4,16 +4,21 @@ let g = newGlobal();
 let dbg = new Debugger;
 let gw = dbg.addDebuggee(g);
 
 function getSourceMapURL() {
     let fw = gw.makeDebuggeeValue(g.f);
     return fw.script.source.sourceMapURL;
 }
 
+function setSourceMapURL(url) {
+    let fw = gw.makeDebuggeeValue(g.f);
+    fw.script.source.sourceMapURL = url;
+}
+
 // Without a source map
 g.evaluate("function f(x) { return 2*x; }");
 assertEq(getSourceMapURL(), null);
 
 // With a source map
 g.evaluate("function f(x) { return 2*x; }", {sourceMapURL: 'file:///var/foo.js.map'});
 assertEq(getSourceMapURL(), 'file:///var/foo.js.map');
 
@@ -63,8 +68,15 @@ g.evaluate('function f() {}\n' +
            '//@ sourceMappingURL=http://example.com/bar.js.map');
 assertEq(getSourceMapURL(), 'http://example.com/bar.js.map');
 
 // With both a comment and the evaluate option.
 g.evaluate('function f() {}\n' +
            '//@ sourceMappingURL=http://example.com/foo.js.map',
            {sourceMapURL: 'http://example.com/bar.js.map'});
 assertEq(getSourceMapURL(), 'http://example.com/foo.js.map');
+
+// Make sure setting the sourceMapURL manually works
+setSourceMapURL('baz.js.map');
+assertEq(getSourceMapURL(), 'baz.js.map');
+
+setSourceMapURL('');
+assertEq(getSourceMapURL(), 'baz.js.map');
--- a/js/src/jit-test/tests/debug/Source-sourceMapURL.js
+++ b/js/src/jit-test/tests/debug/Source-sourceMapURL.js
@@ -4,16 +4,21 @@ let g = newGlobal();
 let dbg = new Debugger;
 let gw = dbg.addDebuggee(g);
 
 function getSourceMapURL() {
     let fw = gw.makeDebuggeeValue(g.f);
     return fw.script.source.sourceMapURL;
 }
 
+function setSourceMapURL(url) {
+    let fw = gw.makeDebuggeeValue(g.f);
+    fw.script.source.sourceMapURL = url;
+}
+
 // Without a source map
 g.evaluate("function f(x) { return 2*x; }");
 assertEq(getSourceMapURL(), null);
 
 // With a source map
 g.evaluate("function f(x) { return 2*x; }", {sourceMapURL: 'file:///var/foo.js.map'});
 assertEq(getSourceMapURL(), 'file:///var/foo.js.map');
 
@@ -63,8 +68,15 @@ g.evaluate('function f() {}\n' +
            '//# sourceMappingURL=http://example.com/bar.js.map');
 assertEq(getSourceMapURL(), 'http://example.com/bar.js.map');
 
 // With both a comment and the evaluate option.
 g.evaluate('function f() {}\n' +
            '//# sourceMappingURL=http://example.com/foo.js.map',
            {sourceMapURL: 'http://example.com/bar.js.map'});
 assertEq(getSourceMapURL(), 'http://example.com/foo.js.map');
+
+// Make sure setting the sourceMapURL manually works
+setSourceMapURL('baz.js.map');
+assertEq(getSourceMapURL(), 'baz.js.map');
+
+setSourceMapURL('');
+assertEq(getSourceMapURL(), 'baz.js.map');
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -2098,27 +2098,16 @@ ScriptSource::setDisplayURL(ExclusiveCon
     displayURL_ = DuplicateString(cx, displayURL);
     return displayURL_ != nullptr;
 }
 
 bool
 ScriptSource::setSourceMapURL(ExclusiveContext *cx, const char16_t *sourceMapURL)
 {
     MOZ_ASSERT(sourceMapURL);
-    if (hasSourceMapURL()) {
-        // Warn about the replacement, but use the new one.
-        if (cx->isJSContext()) {
-            JS_ReportErrorFlagsAndNumber(cx->asJSContext(), JSREPORT_WARNING,
-                                         js_GetErrorMessage, nullptr,
-                                         JSMSG_ALREADY_HAS_PRAGMA, filename_.get(),
-                                         "//# sourceMappingURL");
-        }
-
-        sourceMapURL_ = nullptr;
-    }
 
     size_t len = js_strlen(sourceMapURL) + 1;
     if (len == 1)
         return true;
 
     sourceMapURL_ = DuplicateString(cx, sourceMapURL);
     return sourceMapURL_ != nullptr;
 }
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -4999,16 +4999,36 @@ DebuggerSource_getIntroductionType(JSCon
         args.rval().setString(str);
     } else {
         args.rval().setUndefined();
     }
     return true;
 }
 
 static bool
+DebuggerSource_setSourceMapUrl(JSContext *cx, unsigned argc, Value *vp)
+{
+    THIS_DEBUGSOURCE_REFERENT(cx, argc, vp, "sourceMapURL", args, obj, sourceObject);
+    ScriptSource *ss = sourceObject->source();
+    MOZ_ASSERT(ss);
+
+    JSString *str = ToString<CanGC>(cx, args[0]);
+    if (!str)
+        return false;
+
+    AutoStableStringChars stableChars(cx);
+    if (!stableChars.initTwoByte(cx, str))
+        return false;
+
+    ss->setSourceMapURL(cx, stableChars.twoByteChars());
+    args.rval().setUndefined();
+    return true;
+}
+
+static bool
 DebuggerSource_getSourceMapUrl(JSContext *cx, unsigned argc, Value *vp)
 {
     THIS_DEBUGSOURCE_REFERENT(cx, argc, vp, "(get sourceMapURL)", args, obj, sourceObject);
 
     ScriptSource *ss = sourceObject->source();
     MOZ_ASSERT(ss);
 
     if (ss->hasSourceMapURL()) {
@@ -5027,17 +5047,17 @@ static const JSPropertySpec DebuggerSour
     JS_PSG("text", DebuggerSource_getText, 0),
     JS_PSG("url", DebuggerSource_getUrl, 0),
     JS_PSG("element", DebuggerSource_getElement, 0),
     JS_PSG("displayURL", DebuggerSource_getDisplayURL, 0),
     JS_PSG("introductionScript", DebuggerSource_getIntroductionScript, 0),
     JS_PSG("introductionOffset", DebuggerSource_getIntroductionOffset, 0),
     JS_PSG("introductionType", DebuggerSource_getIntroductionType, 0),
     JS_PSG("elementAttributeName", DebuggerSource_getElementProperty, 0),
-    JS_PSG("sourceMapURL", DebuggerSource_getSourceMapUrl, 0),
+    JS_PSGS("sourceMapURL", DebuggerSource_getSourceMapUrl, DebuggerSource_setSourceMapUrl, 0),
     JS_PS_END
 };
 
 static const JSFunctionSpec DebuggerSource_methods[] = {
     JS_FS_END
 };