Bug 1371852 - make Frame component listen for source-map pref changes; r=jryans

MozReview-Commit-ID: 51DTsRGogCh

--HG--
extra : rebase_source : fc40cb117e4d504066e11170c6cae546e419ab4f

devtools/client/framework/source-map-url-service.js

@@ -3,6 +3,9 @@
  * 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
@@ -20,13 +23,18 @@ function SourceMapURLService(target, threadClient, sourceMapService) {
   this._target = target;
   this._sourceMapService = sourceMapService;
   this._urls = new Map();
+  this._subscriptions = 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);
 
   target.on("source-updated", this._onSourceUpdated);
   target.on("will-navigate", this.reset);
 
+  Services.prefs.addObserver(SOURCE_MAP_PREF, this._onPrefChanged);
+
   // Start fetching the sources now.
   this._loadingPromise = new Promise(resolve => {
     threadClient.getSources(({sources}) => {
@@ -42,6 +50,7 @@ function SourceMapURLService(target, threadClient, sourceMapService) {
 SourceMapURLService.prototype.reset = function () {
   this._sourceMapService.clearSourceMaps();
   this._urls.clear();
+  this._subscriptions.clear();
 };
 
 /**
@@ -53,7 +62,8 @@ SourceMapURLService.prototype.destroy = function () {
   this.reset();
   this._target.off("source-updated", this._onSourceUpdated);
   this._target.off("will-navigate", this.reset);
-  this._target = this._urls = null;
+  Services.prefs.removeObserver(SOURCE_MAP_PREF, this._onPrefChanged);
+  this._target = this._urls = this._subscriptions = null;
 };
 
 /**
@@ -111,4 +121,129 @@ SourceMapURLService.prototype.originalPositionFor = async function (url, line, c
   return resolvedLocation;
 };
 
+/**
+ * Helper function to call a single callback for a given subscription
+ * entry.
+ * @param {Object} subscriptionEntry
+ *                 An entry in the _subscriptions map.
+ * @param {Function} callback
+ *                 The callback to call; @see subscribe
+ */
+SourceMapURLService.prototype._callOneCallback = async function (subscriptionEntry,
+                                                                 callback) {
+  // If source maps are disabled, immediately call with just "false".
+  if (!this._prefValue) {
+    callback(false);
+    return;
+  }
+
+  if (!subscriptionEntry.promise) {
+    const {url, line, column} = subscriptionEntry;
+    subscriptionEntry.promise = this.originalPositionFor(url, line, column);
+  }
+
+  let resolvedLocation = await subscriptionEntry.promise;
+  if (resolvedLocation) {
+    const {line, column, sourceUrl} = resolvedLocation;
+    // In case we're racing a pref change, pass the current value
+    // here, not plain "true".
+    callback(this._prefValue, sourceUrl, line, column);
+  }
+};
+
+/**
+ * Subscribe to changes to a given location.  This will arrange to
+ * call a callback when an original location is determined (if source
+ * maps are enabled), or when the source map pref changes.
+ *
+ * @param {String} url
+ *                 The URL of the generated location.
+ * @param {Number} line
+ *                 The line number of the generated location.
+ * @param {Number} column
+ *                 The column number of the generated location (can be undefined).
+ * @param {Function} callback
+ *                 The callback to call.  This may be called zero or
+ *                 more times -- it may not be called if the location
+ *                 is not source mapped; and it may be called multiple
+ *                 times if the source map pref changes.  It is called
+ *                 as callback(enabled, url, line, column).  |enabled|
+ *                 is a boolean.  If true then source maps are enabled
+ *                 and the remaining arguments are the original
+ *                 location.  If false, then source maps are disabled
+ *                 and the generated location should be used; in this
+ *                 case the remaining arguments should be ignored.
+ */
+SourceMapURLService.prototype.subscribe = function (url, line, column, callback) {
+  if (!this._subscriptions) {
+    return;
+  }
+
+  let key = JSON.stringify([url, line, column]);
+  let subscriptionEntry = this._subscriptions.get(key);
+  if (!subscriptionEntry) {
+    subscriptionEntry = {
+      url,
+      line,
+      column,
+      promise: null,
+      callbacks: [],
+    };
+    this._subscriptions.set(key, subscriptionEntry);
+  }
+  subscriptionEntry.callbacks.push(callback);
+
+  // Only notify upon subscription if source maps are actually in use.
+  if (this._prefValue) {
+    this._callOneCallback(subscriptionEntry, callback);
+  }
+};
+
+/**
+ * Unsubscribe from changes to a given location.
+ *
+ * @param {String} url
+ *                 The URL of the generated location.
+ * @param {Number} line
+ *                 The line number of the generated location.
+ * @param {Number} column
+ *                 The column number of the generated location (can be undefined).
+ * @param {Function} callback
+ *                 The callback.
+ */
+SourceMapURLService.prototype.unsubscribe = function (url, line, column, callback) {
+  if (!this._subscriptions) {
+    return;
+  }
+  let key = JSON.stringify([url, line, column]);
+  let subscriptionEntry = this._subscriptions.get(key);
+  if (subscriptionEntry) {
+    let index = subscriptionEntry.callbacks.indexOf(callback);
+    if (index !== -1) {
+      subscriptionEntry.callbacks.splice(index, 1);
+      // Remove the whole entry when the last subscriber is removed.
+      if (subscriptionEntry.callbacks.length === 0) {
+        this._subscriptions.delete(key);
+      }
+    }
+  }
+};
+
+/**
+ * A helper function that is called when the source map pref changes.
+ * This function notifies all subscribers of the state change.
+ */
+SourceMapURLService.prototype._onPrefChanged = function () {
+  if (!this._subscriptions) {
+    return;
+  }
+
+  this._prefValue = Services.prefs.getBoolPref(SOURCE_MAP_PREF);
+  for (let [, subscriptionEntry] of this._subscriptions) {
+    for (let callback of subscriptionEntry.callbacks) {
+      this._callOneCallback(subscriptionEntry, callback);
+    }
+  }
+};
+
 exports.SourceMapURLService = SourceMapURLService;

devtools/client/framework/toolbox.js

@@ -536,14 +536,9 @@ Toolbox.prototype = {
   },
 
   /**
-   * A common access point for the client-side mapping service for source maps that
-   * any panel can use.  This is a "low-level" API that connects to
-   * the source map worker.
+   * Unconditionally create and get the source map service.
    */
-  get sourceMapService() {
-    if (!Services.prefs.getBoolPref("devtools.source-map.client-service.enabled")) {
-      return null;
-    }
+  _createSourceMapService: function () {
     if (this._sourceMapService) {
       return this._sourceMapService;
     }
@@ -554,6 +549,18 @@ Toolbox.prototype = {
     return this._sourceMapService;
   },
 
+  /**
+   * A common access point for the client-side mapping service for source maps that
+   * any panel can use.  This is a "low-level" API that connects to
+   * the source map worker.
+   */
+  get sourceMapService() {
+    if (!Services.prefs.getBoolPref("devtools.source-map.client-service.enabled")) {
+      return null;
+    }
+    return this._createSourceMapService();
+  },
+
   /**
    * Clients wishing to use source maps but that want the toolbox to
    * track the source actor mapping can use this source map service.
@@ -565,10 +572,7 @@ Toolbox.prototype = {
     if (this._sourceMapURLService) {
       return this._sourceMapURLService;
     }
-    let sourceMaps = this.sourceMapService;
-    if (!sourceMaps) {
-      return null;
-    }
+    let sourceMaps = this._createSourceMapService();
     this._sourceMapURLService = new SourceMapURLService(this._target, this.threadClient,
                                                         sourceMaps);
     return this._sourceMapURLService;

devtools/client/shared/components/frame.js

@@ -50,35 +50,35 @@ module.exports = createClass({
   },
 
   componentWillMount() {
-    const sourceMapService = this.props.sourceMapService;
-    if (sourceMapService) {
+    if (this.props.sourceMapService) {
       const { source, line, column } = this.props.frame;
-      sourceMapService.originalPositionFor(source, line, column)
-        .then(resolvedLocation => {
-          if (resolvedLocation) {
-            this.onSourceUpdated(resolvedLocation);
-          }
-        });
+      this.props.sourceMapService.subscribe(source, line, column,
+                                            this._locationChanged);
     }
   },
 
-  /**
-   * Component method to update the FrameView when a resolved location is available
-   * @param {Location} resolvedLocation
-   *        the resolved location as found via a source map
-   */
-  onSourceUpdated(resolvedLocation) {
-    const { sourceUrl, line, column } = resolvedLocation;
-    const frame = {
-      source: sourceUrl,
-      line,
-      column,
-      functionDisplayName: this.props.frame.functionDisplayName,
+  componentWillUnmount() {
+    if (this.props.sourceMapService) {
+      const { source, line, column } = this.props.frame;
+      this.props.sourceMapService.unsubscribe(source, line, column,
+                                              this._locationChanged);
+    }
+  },
+
+  _locationChanged(isSourceMapped, url, line, column) {
+    let newState = {
+      isSourceMapped,
     };
-    this.setState({
-      frame,
-      isSourceMapped: true,
-    });
+    if (isSourceMapped) {
+      newState.frame = {
+        source: url,
+        line,
+        column,
+        functionDisplayName: this.props.frame.functionDisplayName,
+      };
+    }
+
+    this.setState(newState);
   },
 
   /**
@@ -108,7 +108,7 @@ module.exports = createClass({
       showFullSourceUrl
     } = this.props;
 
-    if (this.state && this.state.isSourceMapped) {
+    if (this.state && this.state.isSourceMapped && this.state.frame) {
       frame = this.state.frame;
       isSourceMapped = this.state.isSourceMapped;
     } else {

devtools/client/shared/components/test/mochitest/chrome.ini

@@ -3,6 +3,7 @@ support-files =
   head.js
 
 [test_frame_01.html]
+[test_frame_02.html]
 [test_HSplitBox_01.html]
 [test_notification_box_01.html]
 [test_notification_box_02.html]

devtools/client/shared/components/test/mochitest/test_frame_01.html

@@ -293,14 +293,11 @@ window.onload = Task.async(function* () {
       sourceUrl: "https://bugzilla.mozilla.org/original.js",
     };
     let mockSourceMapService = {
-      originalPositionFor: function () {
-	// Return a phony promise-like thing that resolves
-	// immediately.
-	return {
-	  then: function (consequence) {
-	    consequence(resolvedLocation);
-	  },
-	};
+      subscribe: function (url, line, column, callback) {
+	// Resolve immediately.
+        callback(true, resolvedLocation.sourceUrl, resolvedLocation.line);
+      },
+      unsubscribe: function (url, line, column, callback) {
       },
     };
     yield checkFrameComponent({

devtools/client/shared/components/test/mochitest/test_frame_02.html

@@ -0,0 +1,97 @@
+<!-- This Source Code Form is subject to the terms of the Mozilla Public
+   - License, v. 2.0. If a copy of the MPL was not distributed with this
+   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
+<!DOCTYPE HTML>
+<html>
+<!--
+Test that the frame component reacts to source-map pref changse.
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Frame component source-map test</title>
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css">
+</head>
+<body>
+<pre id="test">
+<script src="head.js" type="application/javascript"></script>
+<script type="application/javascript">
+window.onload = Task.async(function* () {
+  try {
+    const Services = browserRequire("Services");
+    let ReactDOM = browserRequire("devtools/client/shared/vendor/react-dom");
+    let React = browserRequire("devtools/client/shared/vendor/react");
+    let Frame = React.createFactory(browserRequire("devtools/client/shared/components/frame"));
+
+    const resolvedLocation = {
+      sourceId: "whatever",
+      line: 23,
+      sourceUrl: "https://bugzilla.mozilla.org/original.js",
+    };
+    let mockSourceMapService = {
+      _update: function () {
+        this._callback(Services.prefs.getBoolPref(PREF),
+                       resolvedLocation.sourceUrl,
+                       resolvedLocation.line);
+      },
+      subscribe: function (url, line, column, callback) {
+        this._callback = callback;
+	// Resolve immediately.
+        this._update();
+      },
+      unsubscribe: function (url, line, column, callback) {
+      },
+    };
+
+    let props = {
+      onClick: () => {},
+      frame: {
+	line: 97,
+	source: "https://bugzilla.mozilla.org/bundle.js",
+      },
+      sourceMapService: mockSourceMapService,
+    };
+
+    const PREF = "devtools.source-map.client-service.enabled";
+    Services.prefs.setBoolPref(PREF, false);
+
+    let frame = ReactDOM.render(Frame(props), window.document.body);
+    let el = ReactDOM.findDOMNode(frame);
+    let { source } = props.frame;
+
+    let expectedOriginal = {
+      file: "original.js",
+      line: resolvedLocation.line,
+      shouldLink: true,
+      tooltip: "View source in Debugger → https://bugzilla.mozilla.org/original.js:23",
+      source: "https://bugzilla.mozilla.org/original.js",
+    };
+    let expectedGenerated = {
+      file: "bundle.js",
+      line: 97,
+      shouldLink: true,
+      tooltip: "View source in Debugger → https://bugzilla.mozilla.org/bundle.js:97",
+      source: "https://bugzilla.mozilla.org/bundle.js",
+    };
+
+    checkFrameString(Object.assign({ el, source }, expectedGenerated));
+
+    Services.prefs.setBoolPref(PREF, true);
+    mockSourceMapService._update();
+    checkFrameString(Object.assign({ el, source }, expectedOriginal));
+
+    Services.prefs.setBoolPref(PREF, false);
+    mockSourceMapService._update();
+    checkFrameString(Object.assign({ el, source }, expectedGenerated));
+
+    Services.prefs.clearUserPref(PREF);
+  } catch (e) {
+    ok(false, "Got an error: " + DevToolsUtils.safeErrorString(e));
+  } finally {
+    SimpleTest.finish();
+  }
+});
+</script>
+</pre>
+</body>
+</html>

devtools/client/shared/components/test/mochitest/test_stack-trace-source-maps.html

@@ -47,21 +47,14 @@ window.onload = function () {
       onViewSourceInScratchpad: () => {},
       // A mock source map service.
       sourceMapService: {
-	originalPositionFor: function (url, line, column) {
+	subscribe: function (url, line, column, callback) {
 	  let newLine = line === 99 ? 1 : 7;
-	  // Return a phony promise-like thing that resolves
-	  // immediately.
-	  return {
-	    then: function (consequence) {
-	      consequence({
-		sourceId: "whatever",
-		sourceUrl: "https://bugzilla.mozilla.org/original.js",
-		line: newLine,
-		column,
-	      });
-	    },
-	  };
-	}
+	  // Resolve immediately.
+	  callback(true, "https://bugzilla.mozilla.org/original.js",
+		   newLine, column);
+	},
+        unsubscribe: function (url, line, column, callback) {
+        },
       },
     };
 

devtools/client/shared/widgets/tooltip/EventTooltipHelper.js

@@ -44,6 +44,7 @@ function EventTooltip(tooltip, eventListenerInfos, toolbox) {
   this._headerClicked = this._headerClicked.bind(this);
   this._debugClicked = this._debugClicked.bind(this);
   this.destroy = this.destroy.bind(this);
+  this._subscriptions = [];
 }
 
 EventTooltip.prototype = {
@@ -106,26 +107,33 @@ EventTooltip.prototype = {
       if (listener.hide.filename) {
         text = L10N.getStr("eventsTooltip.unknownLocation");
         title = L10N.getStr("eventsTooltip.unknownLocationExplanation");
-      } else if (sourceMapService) {
+      } else {
         const location = this._parseLocation(text);
         if (location) {
-          sourceMapService.originalPositionFor(location.url, location.line)
-            .then((originalLocation) => {
-              // Do nothing if the tooltip was destroyed while we were
-              // waiting for a response.
-              if (this._tooltip) {
-                if (originalLocation) {
-                  const { sourceUrl, line } = originalLocation;
-                  let newURI = sourceUrl + ":" + line;
-                  filename.textContent = newURI;
-                  filename.setAttribute("title", newURI);
-                  let eventEditor = this._eventEditors.get(content);
-                  eventEditor.uri = newURI;
-                }
-                // This is emitted for testing.
-                this._tooltip.emit("event-tooltip-source-map-ready");
-              }
-            });
+          let callback = (enabled, url, line, column) => {
+            // Do nothing if the tooltip was destroyed while we were
+            // waiting for a response.
+            if (this._tooltip) {
+              const newUrl = enabled ? url : location.url;
+              const newLine = enabled ? line : location.line;
+
+              let newURI = newUrl + ":" + newLine;
+              filename.textContent = newURI;
+              filename.setAttribute("title", newURI);
+              let eventEditor = this._eventEditors.get(content);
+              eventEditor.uri = newURI;
+
+              // This is emitted for testing.
+              this._tooltip.emit("event-tooltip-source-map-ready");
+            }
+          };
+
+          sourceMapService.subscribe(location.url, location.line, null, callback);
+          this._subscriptions.push({
+            url: location.url,
+            line: location.line,
+            callback
+          });
         }
       }
 
@@ -316,6 +324,12 @@ EventTooltip.prototype = {
       node.removeEventListener("click", this._debugClicked);
     }
 
+    const sourceMapService = this._toolbox.sourceMapURLService;
+    for (let subscription of this._subscriptions) {
+      sourceMapService.unsubscribe(subscription.url, subscription.line, null,
+                                   subscription.callback);
+    }
+
     this._eventListenerInfos = this._toolbox = this._tooltip = null;
   }
 };