No bug: comment fixes for JS debugger sourceMap support. f=fitzgen DONTBUILD
authorJim Blandy <jimb@mozilla.com>
Wed, 05 Jun 2013 18:06:43 -0700
changeset 145679 8135cded2d1c43fc802ebf2670bc981004286e5f
parent 145678 1671444385497dcc2a5ddad46a1450b74ea754a3
child 145680 6b7cddb67921adbe2f44c9b1afe0cb68ad92b9db
push id2697
push userbbajaj@mozilla.com
push dateMon, 05 Aug 2013 18:49:53 +0000
treeherdermozilla-beta@dfec938c7b63 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
milestone24.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
No bug: comment fixes for JS debugger sourceMap support. f=fitzgen DONTBUILD
toolkit/devtools/server/actors/script.js
--- a/toolkit/devtools/server/actors/script.js
+++ b/toolkit/devtools/server/actors/script.js
@@ -1248,17 +1248,17 @@ ThreadActor.prototype = {
     return true;
   },
 
   /**
    * Add the provided script to the server cache.
    *
    * @param aScript Debugger.Script
    *        The source script that will be stored.
-   * @returns true, if the script was added, false otherwise.
+   * @returns a promise of true, if the script was added; of false otherwise.
    */
   _addScript: function TA__addScript(aScript) {
     if (!this._allowSource(aScript.url)) {
       return resolve(false);
     }
 
     // TODO bug 637572: we should be dealing with sources directly, not
     // inferring them through scripts.
@@ -2454,26 +2454,28 @@ function ThreadSources(aThreadActor, aUs
   // source url --> SourceActor
   this._sourceActors = Object.create(null);
   // original url --> generated url
   this._generatedUrlsByOriginalUrl = Object.create(null);
 }
 
 ThreadSources.prototype = {
   /**
-   * Add a source to the current set of sources.
+   * Return the source actor representing |aURL|, creating one if none
+   * exists already. Returns null if |aURL| is not allowed by the 'allow'
+   * predicate.
    *
    * Right now this takes a URL, but in the future it should
    * take a Debugger.Source. See bug 637572.
    *
    * @param String aURL
    *        The source URL.
    * @param optional SourceMapConsumer aSourceMap
-   *        The source map that introduced this source.
-   * @returns a SourceActor representing the source or null.
+   *        The source map that introduced this source, if any.
+   * @returns a SourceActor representing the source at aURL or null.
    */
   source: function TS_source(aURL, aSourceMap=null) {
     if (!this._allow(aURL)) {
       return null;
     }
 
     if (aURL in this._sourceActors) {
       return this._sourceActors[aURL];
@@ -2486,17 +2488,22 @@ ThreadSources.prototype = {
       this._onNewSource(actor);
     } catch (e) {
       reportError(e);
     }
     return actor;
   },
 
   /**
-   * Add all of the sources associated with the given script.
+   * Return a promise of an array of source actors representing all the
+   * sources of |aScript|.
+   *
+   * If source map handling is enabled and |aScript| has a source map, then
+   * use it to find all of |aScript|'s *original* sources; return a promise
+   * of an array of source actors for those.
    */
   sourcesForScript: function TS_sourcesForScript(aScript) {
     if (!this._useSourceMaps || !aScript.sourceMapURL) {
       return resolve([this.source(aScript.url)].filter(isNotNull));
     }
 
     return this.sourceMap(aScript)
       .then((aSourceMap) => {
@@ -2510,39 +2517,42 @@ ThreadSources.prototype = {
         return [this.source(aScript.url)];
       })
       .then(function (aSources) {
         return aSources.filter(isNotNull);
       });
   },
 
   /**
-   * Add the source map for the given script.
+   * Return a promise of a SourceMapConsumer for the source map for
+   * |aScript|; if we already have such a promise extant, return that.
+   * |aScript| must have a non-null sourceMapURL.
    */
   sourceMap: function TS_sourceMap(aScript) {
     if (aScript.url in this._sourceMapsByGeneratedSource) {
       return this._sourceMapsByGeneratedSource[aScript.url];
     }
     dbg_assert(aScript.sourceMapURL);
-    let sourceMapURL = this._normalize(aScript.sourceMapURL,
-                                       aScript.url);
+    let sourceMapURL = this._normalize(aScript.sourceMapURL, aScript.url);
     let map = this._fetchSourceMap(sourceMapURL)
       .then((aSourceMap) => {
         for (let s of aSourceMap.sources) {
           this._generatedUrlsByOriginalUrl[s] = aScript.url;
           this._sourceMapsByOriginalSource[s] = resolve(aSourceMap);
         }
         return aSourceMap;
       });
     this._sourceMapsByGeneratedSource[aScript.url] = map;
     return map;
   },
 
   /**
-   * Fetch the source map located at the given url.
+   * Return a promise of a SourceMapConsumer for the source map located at
+   * |aAbsSourceMapURL|, which must be absolute. If there is already such a
+   * promise extant, return it.
    */
   _fetchSourceMap: function TS__fetchSourceMap(aAbsSourceMapURL) {
     if (aAbsSourceMapURL in this._sourceMaps) {
       return this._sourceMaps[aAbsSourceMapURL];
     } else {
       let promise = fetch(aAbsSourceMapURL).then((rawSourceMap) => {
         let map =  new SourceMapConsumer(rawSourceMap);
         let base = aAbsSourceMapURL.replace(/\/[^\/]+$/, '/');
@@ -2554,17 +2564,17 @@ ThreadSources.prototype = {
         return map;
       });
       this._sourceMaps[aAbsSourceMapURL] = promise;
       return promise;
     }
   },
 
   /**
-   * Returns a promise for the location in the original source if the source is
+   * Returns a promise of the location in the original source if the source is
    * source mapped, otherwise a promise of the same location.
    *
    * TODO bug 637572: take/return a column
    */
   getOriginalLocation: function TS_getOriginalLocation(aSourceUrl, aLine) {
     if (aSourceUrl in this._sourceMapsByGeneratedSource) {
       return this._sourceMapsByGeneratedSource[aSourceUrl]
         .then(function (aSourceMap) {
@@ -2586,16 +2596,21 @@ ThreadSources.prototype = {
       line: aLine
     });
   },
 
   /**
    * Returns a promise of the location in the generated source corresponding to
    * the original source and line given.
    *
+   * When we pass a script S representing generated code to |sourceMap|,
+   * above, that returns a promise P. The process of resolving P populates
+   * the tables this function uses; thus, it won't know that S's original
+   * source URLs map to S until P is resolved.
+   *
    * TODO bug 637572: take/return a column
    */
   getGeneratedLocation: function TS_getGeneratedLocation(aSourceUrl, aLine) {
     if (aSourceUrl in this._sourceMapsByOriginalSource) {
       return this._sourceMapsByOriginalSource[aSourceUrl]
         .then((aSourceMap) => {
           let { line } = aSourceMap.generatedPositionFor({
             source: aSourceUrl,
@@ -2665,16 +2680,17 @@ function isNotNull(aThing) {
 }
 
 /**
  * Performs a request to load the desired URL and returns a promise.
  *
  * @param aURL String
  *        The URL we will request.
  * @returns Promise
+ *        A promise of the document at that URL, as a string.
  *
  * XXX: It may be better to use nsITraceableChannel to get to the sources
  * without relying on caching when we can (not for eval, etc.):
  * http://www.softwareishard.com/blog/firebug/nsitraceablechannel-intercept-http-traffic/
  */
 function fetch(aURL, aOptions={ loadFromCache: true }) {
   let deferred = defer();
   let scheme;