Bug 952127 - notify source map subscribers after pretty-printing; r?bgrins draft
authorTom Tromey <tom@tromey.com>
Fri, 15 Sep 2017 07:54:56 -0600
changeset 668459 47738a03dc190e5a372f61d211158f85ddf53daf
parent 667953 a88ee0137c104a57c747cd129c45952ccf1431ad
child 732716 86214adba22004d14960c1076e6e785f0fb6726d
push id81060
push userbmo:ttromey@mozilla.com
push dateThu, 21 Sep 2017 18:28:58 +0000
reviewersbgrins
bugs952127
milestone57.0a1
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;