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 id27858
push userkwierso@gmail.com
push dateFri, 21 Nov 2014 01:35:46 +0000
treeherdermozilla-central@6309710dd71d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimb
bugs1074745
milestone36.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
 };