Bug 952127 - notify source map subscribers after pretty-printing; r=bgrins
authorTom Tromey <tom@tromey.com>
Fri, 15 Sep 2017 07:54:56 -0600
changeset 382240 05569ef2d175e7b3814141af3cd8085ce0898ecf
parent 382239 e46cfa3b45ff01b1144199e31a1bd31fe5bffc3c
child 382241 3c6f413d3326197966c99f80c7097229e3d30d22
push id32551
push userkwierso@gmail.com
push dateThu, 21 Sep 2017 23:29:53 +0000
treeherdermozilla-central@d6d6fd889f7b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins
bugs952127
milestone58.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 952127 - notify source map subscribers after pretty-printing; r=bgrins Intercept the applySourceMap source map worker request, so that when a source is pretty-printed, source map subscribers can be updated. That this does not yet handle pretty-printing original sources. This isn't supported yet by the debugger, and since the plan is to handle it by augmenting the existing source map, it should be easy to fix this code when it is implemented. The mochitest is included here for testing but I am going to land it upstream as well. MozReview-Commit-ID: 3Lp1ikO8IzZ
devtools/client/debugger/new/test/mochitest/browser.ini
devtools/client/debugger/new/test/mochitest/browser_dbg-pretty-print-console.js
devtools/client/framework/source-map-url-service.js
devtools/client/framework/toolbox.js
--- a/devtools/client/debugger/new/test/mochitest/browser.ini
+++ b/devtools/client/debugger/new/test/mochitest/browser.ini
@@ -66,16 +66,17 @@ skip-if = debug # bug 1374187
 [browser_dbg-iframes.js]
 [browser_dbg_keyboard_navigation.js]
 [browser_dbg_keyboard-shortcuts.js]
 skip-if = os == "linux" # bug 1351952
 [browser_dbg-pause-exceptions.js]
 skip-if = true # Bug 1393121
 [browser_dbg-navigation.js]
 [browser_dbg-pretty-print.js]
+[browser_dbg-pretty-print-console.js]
 [browser_dbg-pretty-print-paused.js]
 [browser_dbg-scopes-mutations.js]
 [browser_dbg-search-file.js]
 skip-if = os == "win" # Bug 1393121
 [browser_dbg-search-sources.js]
 skip-if = os == "win" # Bug 1393121
 [browser_dbg-search-symbols.js]
 skip-if = os == "win" # Bug 1393121
new file mode 100644
--- /dev/null
+++ b/devtools/client/debugger/new/test/mochitest/browser_dbg-pretty-print-console.js
@@ -0,0 +1,47 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Tests that pretty-printing updates console messages.
+
+async function waitFor(condition) {
+  await BrowserTestUtils.waitForCondition(condition, "waitFor", 10, 500);
+  return condition();
+}
+
+add_task(async function () {
+  const dbg = await initDebugger("doc-minified.html");
+  invokeInTab("arithmetic");
+
+  info("Switch to console and check message");
+  const toolbox = dbg.toolbox;
+  const console = await toolbox.selectTool("webconsole");
+  const hud = console.hud;
+
+  let node = await waitFor(() => hud.ui.outputNode.querySelector(".frame-link-source"));
+  const initialLocation = "math.min.js:3:65";
+  is(node.textContent, initialLocation, "location is correct in minified code");
+
+  info("Switch back to debugger and pretty-print");
+  await toolbox.selectTool("jsdebugger");
+  await selectSource(dbg, "math.min.js", 2);
+  clickElement(dbg, "prettyPrintButton");
+
+  await waitForSource(dbg, "math.min.js:formatted");
+  const ppSrc = findSource(dbg, "math.min.js:formatted");
+
+  ok(ppSrc, "Pretty-printed source exists");
+
+  info("Switch back to console and check message");
+  node = await waitFor(() => {
+    // Wait until the message updates.
+    const found = hud.ui.outputNode.querySelector(".frame-link-source");
+    if (found.textContent == initialLocation) {
+      return null;
+    }
+    return found;
+  });
+
+  is(node.textContent, "math.min.js:formatted:22", "location is correct in minified code");
+});
--- a/devtools/client/framework/source-map-url-service.js
+++ b/devtools/client/framework/source-map-url-service.js
@@ -3,31 +3,39 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 "use strict";
 
 const Services = require("Services");
 const SOURCE_MAP_PREF = "devtools.source-map.client-service.enabled";
 
 /**
  * A simple service to track source actors and keep a mapping between
- * original URLs and objects holding the source actor's ID (which is
- * used as a cookie by the devtools-source-map service) and the source
- * map URL.
+ * original URLs and objects holding the source or style actor's ID
+ * (which is used as a cookie by the devtools-source-map service) and
+ * the source map URL.
  *
  * @param {object} toolbox
  *        The toolbox.
  * @param {SourceMapService} sourceMapService
  *        The devtools-source-map functions
  */
 function SourceMapURLService(toolbox, sourceMapService) {
   this._toolbox = toolbox;
   this._target = toolbox.target;
   this._sourceMapService = sourceMapService;
+  // Map from content URLs to descriptors.  Descriptors are later
+  // passed to the source map worker.
   this._urls = new Map();
+  // Map from (stringified) locations to callbacks that are called
+  // when the service decides a location should change (say, a source
+  // map is available or the user changes the pref).
   this._subscriptions = new Map();
+  // A backward map from actor IDs to the original URL.  This is used
+  // to support pretty-printing.
+  this._idMap = new Map();
 
   this._onSourceUpdated = this._onSourceUpdated.bind(this);
   this.reset = this.reset.bind(this);
   this._prefValue = Services.prefs.getBoolPref(SOURCE_MAP_PREF);
   this._onPrefChanged = this._onPrefChanged.bind(this);
   this._onNewStyleSheet = this._onNewStyleSheet.bind(this);
 
   this._target.on("source-updated", this._onSourceUpdated);
@@ -75,32 +83,33 @@ SourceMapURLService.prototype._getLoadin
 
 /**
  * Reset the service.  This flushes the internal cache.
  */
 SourceMapURLService.prototype.reset = function () {
   this._sourceMapService.clearSourceMaps();
   this._urls.clear();
   this._subscriptions.clear();
+  this._idMap.clear();
 };
 
 /**
  * Shut down the service, unregistering its event listeners and
  * flushing the cache.  After this call the service will no longer
  * function.
  */
 SourceMapURLService.prototype.destroy = function () {
   this.reset();
   this._target.off("source-updated", this._onSourceUpdated);
   this._target.off("will-navigate", this.reset);
   if (this._stylesheetsFront) {
     this._stylesheetsFront.off("stylesheet-added", this._onNewStyleSheet);
   }
   Services.prefs.removeObserver(SOURCE_MAP_PREF, this._onPrefChanged);
-  this._target = this._urls = this._subscriptions = null;
+  this._target = this._urls = this._subscriptions = this._idMap = null;
 };
 
 /**
  * A helper function that is called when a new source is available.
  */
 SourceMapURLService.prototype._onSourceUpdated = function (_, sourceEvent) {
   // Maybe we were shut down while waiting.
   if (!this._urls) {
@@ -109,32 +118,70 @@ SourceMapURLService.prototype._onSourceU
 
   let { source } = sourceEvent;
   let { generatedUrl, url, actor: id, sourceMapURL } = source;
 
   // |generatedUrl| comes from the actor and is extracted from the
   // source code by SpiderMonkey.
   let seenUrl = generatedUrl || url;
   this._urls.set(seenUrl, { id, url: seenUrl, sourceMapURL });
+  this._idMap.set(id, seenUrl);
 };
 
 /**
  * A helper function that is called when a new style sheet is
  * available.
  * @param {StyleSheetActor} sheet
  *        The new style sheet's actor.
  */
 SourceMapURLService.prototype._onNewStyleSheet = function (sheet) {
   // Maybe we were shut down while waiting.
   if (!this._urls) {
     return;
   }
 
   let {href: url, sourceMapURL, actor: id} = sheet._form;
   this._urls.set(url, { id, url, sourceMapURL});
+  this._idMap.set(id, url);
+};
+
+/**
+ * A callback that is called from the lower-level source map service
+ * proxy (see toolbox.js) when some tool has installed a new source
+ * map.  This happens when pretty-printing a source.
+ *
+ * @param {String} id
+ *        The actor ID (used as a cookie here as elsewhere in this file)
+ * @param {String} newUrl
+ *        The URL of the pretty-printed source
+ */
+SourceMapURLService.prototype.sourceMapChanged = function (id, newUrl) {
+  if (!this._urls) {
+    return;
+  }
+
+  let urlKey = this._idMap.get(id);
+  if (urlKey) {
+    // The source map URL here doesn't actually matter.
+    this._urls.set(urlKey, { id, url: newUrl, sourceMapURL: "" });
+
+    // Walk over all the location subscribers, looking for any that
+    // are subscribed to a location coming from |urlKey|.  Then,
+    // re-notify any such subscriber by clearing the stored promise
+    // and forcing a re-evaluation.
+    for (let [, subscriptionEntry] of this._subscriptions) {
+      if (subscriptionEntry.url === urlKey) {
+        // Force an update.
+        subscriptionEntry.promise = null;
+        for (let callback of subscriptionEntry.callbacks) {
+          this._callOneCallback(subscriptionEntry, callback);
+        }
+      }
+    }
+  }
 };
 
 /**
  * Look up the original position for a given location.  This returns a
  * promise resolving to either the original location, or null if the
  * given location is not source-mapped.  If a location is returned, it
  * is of the same form as devtools-source-map's |getOriginalLocation|.
  *
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -598,16 +598,32 @@ Toolbox.prototype = {
                   // returns.
                   return {
                     text: message,
                     contentType: "text/plain",
                   };
                 });
             };
 
+          case "applySourceMap":
+            return (generatedId, url, code, mappings) => {
+              return target.applySourceMap(generatedId, url, code, mappings)
+                .then(result => {
+                  // If a tool has changed or introduced a source map
+                  // (e.g, by pretty-printing a source), tell the
+                  // source map URL service about the change, so that
+                  // subscribers to that service can be updated as
+                  // well.
+                  if (this._sourceMapURLService) {
+                    this._sourceMapURLService.sourceMapChanged(generatedId, url);
+                  }
+                  return result;
+                });
+            };
+
           default:
             return target[name];
         }
       },
     });
 
     this._sourceMapService.startSourceMapWorker(SOURCE_MAP_WORKER);
     return this._sourceMapService;