Bug 865252 - Don't heavily prefer cache when loading source mapped sources; r=robcee
☠☠ backed out by 81fe6ab72d45 ☠ ☠
authorNick Fitzgerald <nfitzgerald@mozilla.com>
Sat, 11 May 2013 10:22:38 +0100
changeset 131553 752dca088d71
parent 131552 6ebf2274acf5
child 131554 141e3ccc715f
push id1605
push userjwalker@mozilla.com
push date2013-05-11 09:27 +0000
treeherderfx-team@4dbd432e7667 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrobcee
bugs865252
milestone23.0a1
Bug 865252 - Don't heavily prefer cache when loading source mapped sources; r=robcee
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
 };
 
 
@@ -2439,30 +2447,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;
@@ -2474,18 +2482,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) {
@@ -2649,17 +2656,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) {
@@ -2716,17 +2723,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() {
+    attachTestGlobalClientAndResume(gClient, "test-source-map", function(aResponse, 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]