Bug 865252 - Don't heavily prefer cache when loading source mapped sources; r=rcampbell
authorNick Fitzgerald <fitzgen@gmail.com>
Mon, 13 May 2013 14:53:00 +0300
changeset 132404 0c312bf9446a310f5f6080a214ba12e9589b862b
parent 132403 fa63467ea33034052826d7fd9f5f85c07e037d0f
child 132405 108c6b3bf66e7018a1fcbbf78981eca801fe76f7
push id24701
push userryanvm@gmail.com
push dateTue, 21 May 2013 02:25:31 +0000
treeherdermozilla-central@4ac6c72b06c8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrcampbell
bugs865252
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
Bug 865252 - Don't heavily prefer cache when loading source mapped sources; r=rcampbell
toolkit/devtools/debugger/server/dbg-script-actors.js
toolkit/devtools/debugger/tests/unit/head_dbg.js
toolkit/devtools/debugger/tests/unit/test_sourcemaps-07.js
toolkit/devtools/debugger/tests/unit/xpcshell.ini
--- a/toolkit/devtools/debugger/server/dbg-script-actors.js
+++ b/toolkit/devtools/debugger/server/dbg-script-actors.js
@@ -1376,23 +1376,23 @@ PauseScopedActor.prototype = {
 
 /**
  * A SourceActor provides information about the source of a script.
  *
  * @param aUrl String
  *        The url of the source we are representing.
  * @param aThreadActor ThreadActor
  *        The current thread actor.
- * @param aSourceContent String
- *        Optional. The contents of the source, if we already know it.
+ * @param aSourceMap SourceMapConsumer
+ *        Optional. The source map that introduced this source, if available.
  */
-function SourceActor(aUrl, aThreadActor, aSourceContent=null) {
+function SourceActor(aUrl, aThreadActor, aSourceMap=null) {
   this._threadActor = aThreadActor;
   this._url = aUrl;
-  this._sourceContent = aSourceContent;
+  this._sourceMap = aSourceMap;
 }
 
 SourceActor.prototype = {
   constructor: SourceActor,
   actorPrefix: "source",
 
   get threadActor() this._threadActor,
   get url() this._url,
@@ -1410,45 +1410,53 @@ SourceActor.prototype = {
       delete this.registeredPool.sourceActors[this.actorID];
     }
   },
 
   /**
    * Handler for the "source" packet.
    */
   onSource: function SA_onSource(aRequest) {
-    if (this._sourceContent) {
+    let sourceContent = null;
+    if (this._sourceMap) {
+      sourceContent = this._sourceMap.sourceContentFor(this._url);
+    }
+
+    if (sourceContent) {
       return {
         from: this.actorID,
         source: this.threadActor.createValueGrip(
-          this._sourceContent, this.threadActor.threadLifetimePool)
+          sourceContent, this.threadActor.threadLifetimePool)
       };
     }
 
-    return fetch(this._url)
-      .then(function(aSource) {
+    // XXX bug 865252: Don't load from the cache if this is a source mapped
+    // source because we can't guarantee that the cache has the most up to date
+    // content for this source like we can if it isn't source mapped.
+    return fetch(this._url, { loadFromCache: !this._sourceMap })
+      .then((aSource) => {
         return this.threadActor.createValueGrip(
           aSource, this.threadActor.threadLifetimePool);
-      }.bind(this))
-      .then(function (aSourceGrip) {
+      })
+      .then((aSourceGrip) => {
         return {
           from: this.actorID,
           source: aSourceGrip
         };
-      }.bind(this), function (aError) {
+      }, (aError) => {
         let msg = "Got an exception during SA_onSource: " + aError +
           "\n" + aError.stack;
         Cu.reportError(msg);
         dumpn(msg);
         return {
           "from": this.actorID,
           "error": "loadSourceError",
           "message": "Could not load the source for " + this._url + "."
         };
-      }.bind(this));
+      });
   }
 };
 
 SourceActor.prototype.requestTypes = {
   "source": SourceActor.prototype.onSource
 };
 
 
@@ -2443,30 +2451,30 @@ ThreadSources.prototype = {
   /**
    * Add a source to the current set of sources.
    *
    * 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 String aSourceContent
-   *        Optional. The content of the source, if we already know it.
+   * @param optional SourceMapConsumer aSourceMap
+   *        The source map that introduced this source.
    * @returns a SourceActor representing the source or null.
    */
-  source: function TS_source(aURL, aSourceContent=null) {
+  source: function TS_source(aURL, aSourceMap=null) {
     if (!this._allow(aURL)) {
       return null;
     }
 
     if (aURL in this._sourceActors) {
       return this._sourceActors[aURL];
     }
 
-    let actor = new SourceActor(aURL, this._thread, aSourceContent);
+    let actor = new SourceActor(aURL, this._thread, aSourceMap);
     this._thread.threadLifetimePool.addActor(actor);
     this._sourceActors[aURL] = actor;
     try {
       this._onNewSource(actor);
     } catch (e) {
       reportError(e);
     }
     return actor;
@@ -2478,18 +2486,17 @@ ThreadSources.prototype = {
   sourcesForScript: function TS_sourcesForScript(aScript) {
     if (!this._useSourceMaps || !aScript.sourceMapURL) {
       return resolve([this.source(aScript.url)].filter(isNotNull));
     }
 
     return this.sourceMap(aScript)
       .then((aSourceMap) => {
         return [
-          this.source(s, aSourceMap.sourceContentFor(s))
-          for (s of aSourceMap.sources)
+          this.source(s, aSourceMap) for (s of aSourceMap.sources)
         ];
       }, (e) => {
         reportError(e);
         delete this._sourceMaps[this._normalize(aScript.sourceMapURL, aScript.url)];
         delete this._sourceMapsByGeneratedSource[aScript.url];
         return [this.source(aScript.url)];
       })
       .then(function (aSources) {
@@ -2653,17 +2660,17 @@ function isNotNull(aThing) {
  * @param aURL String
  *        The URL we will request.
  * @returns Promise
  *
  * 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) {
+function fetch(aURL, aOptions={ loadFromCache: true }) {
   let deferred = defer();
   let scheme;
   let url = aURL.split(" -> ").pop();
   let charset;
 
   try {
     scheme = Services.io.extractScheme(url);
   } catch (e) {
@@ -2720,17 +2727,19 @@ function fetch(aURL) {
             return;
           }
 
           charset = channel.contentCharset;
           deferred.resolve(chunks.join(""));
         }
       };
 
-      channel.loadFlags = channel.LOAD_FROM_CACHE;
+      channel.loadFlags = aOptions.loadFromCache
+        ? channel.LOAD_FROM_CACHE
+        : channel.LOAD_BYPASS_CACHE;
       channel.asyncOpen(streamListener, null);
       break;
   }
 
   return deferred.promise.then(function (source) {
     return convertToUnicode(source, charset);
   });
 }
--- a/toolkit/devtools/debugger/tests/unit/head_dbg.js
+++ b/toolkit/devtools/debugger/tests/unit/head_dbg.js
@@ -202,8 +202,23 @@ function readFile(aFileName) {
     .createInstance(Ci.nsIFileInputStream);
   s.init(f, -1, -1, false);
   try {
     return NetUtil.readInputStreamToString(s, s.available());
   } finally {
     s.close();
   }
 }
+
+function writeFile(aFileName, aContent) {
+  let file = do_get_file(aFileName, true);
+  let stream = Cc["@mozilla.org/network/file-output-stream;1"]
+    .createInstance(Ci.nsIFileOutputStream);
+  stream.init(file, -1, -1, 0);
+  try {
+    do {
+      let numWritten = stream.write(aContent, aContent.length);
+      aContent = aContent.slice(numWritten);
+    } while (aContent.length > 0);
+  } finally {
+    stream.close();
+  }
+}
new file mode 100644
--- /dev/null
+++ b/toolkit/devtools/debugger/tests/unit/test_sourcemaps-07.js
@@ -0,0 +1,67 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Test that we don't permanently cache sources from source maps.
+ */
+
+var gDebuggee;
+var gClient;
+var gThreadClient;
+
+Components.utils.import("resource:///modules/devtools/SourceMap.jsm");
+
+function run_test()
+{
+  initTestDebuggerServer();
+  gDebuggee = addTestGlobal("test-source-map");
+  gClient = new DebuggerClient(DebuggerServer.connectPipe());
+  gClient.connect(function() {
+    attachTestTabAndResume(gClient, "test-source-map", function(aResponse, aTabClient, aThreadClient) {
+      gThreadClient = aThreadClient;
+      test_cached_original_sources();
+    });
+  });
+  do_test_pending();
+}
+
+function test_cached_original_sources()
+{
+  writeFile("temp.foobar", "initial content");
+
+  gClient.addOneTimeListener("newSource", onNewSource);
+
+  let node = new SourceNode(1, 0,
+                            getFileUrl("temp.foobar"),
+                            "function funcFromTemp() {}\n");
+  let { code, map } = node.toStringWithSourceMap({
+    file: "abc.js"
+  });
+  code += "//@ sourceMappingURL=data:text/json;base64," + btoa(map.toString());
+
+
+  Components.utils.evalInSandbox(code, gDebuggee, "1.8",
+                                 "http://example.com/www/js/abc.js", 1);
+}
+
+function onNewSource(aEvent, aPacket) {
+  let sourceClient = gThreadClient.source(aPacket.source);
+  sourceClient.source(function (aResponse) {
+    do_check_true(!aResponse.error,
+                  "Should not be an error grabbing the source");
+    do_check_eq(aResponse.source, "initial content",
+                "The correct source content should be sent");
+
+    writeFile("temp.foobar", "new content");
+
+    sourceClient.source(function (aResponse) {
+      do_check_true(!aResponse.error,
+                    "Should not be an error grabbing the source");
+      do_check_eq(aResponse.source, "new content",
+                  "The correct source content should not be cached, so we should get the new content");
+
+      do_get_file("temp.foobar").remove(false);
+      finishClient(gClient);
+    });
+  });
+}
--- a/toolkit/devtools/debugger/tests/unit/xpcshell.ini
+++ b/toolkit/devtools/debugger/tests/unit/xpcshell.ini
@@ -83,16 +83,19 @@ reason = bug 820380
 [test_sourcemaps-03.js]
 [test_sourcemaps-04.js]
 skip-if = toolkit == "gonk"
 reason = bug 820380
 [test_sourcemaps-05.js]
 skip-if = toolkit == "gonk"
 reason = bug 820380
 [test_sourcemaps-06.js]
+[test_sourcemaps-07.js]
+skip-if = toolkit == "gonk"
+reason = bug 820380
 [test_objectgrips-01.js]
 [test_objectgrips-02.js]
 [test_objectgrips-03.js]
 [test_objectgrips-04.js]
 [test_interrupt.js]
 [test_stepping-01.js]
 [test_stepping-02.js]
 [test_stepping-03.js]