Bug 1500632 - source-map-url-service should trigger subscribers when files matching them are detected. r=tromey
authorLogan Smyth <loganfsmyth@gmail.com>
Wed, 24 Oct 2018 12:55:01 +0000
changeset 442803 f3d51256c5d728e4b324525917b481565f5690d9
parent 442802 4c162e4eda0879b7ea1761c3db4679e403273479
child 442804 0b9e4abc5911fb241ba351c07e7cc2f3e4fb2ed7
push id71655
push userlsmyth@mozilla.com
push dateWed, 24 Oct 2018 16:28:15 +0000
treeherderautoland@f3d51256c5d7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstromey
bugs1500632
milestone65.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 1500632 - source-map-url-service should trigger subscribers when files matching them are detected. r=tromey Differential Revision: https://phabricator.services.mozilla.com/D9433
devtools/client/framework/source-map-url-service.js
devtools/client/framework/test/browser.ini
devtools/client/framework/test/browser_source_map-late-script.js
devtools/client/framework/test/code_bundle_late_script.js
devtools/client/framework/test/code_bundle_late_script.js.map
devtools/client/framework/test/code_late_script.js
--- a/devtools/client/framework/source-map-url-service.js
+++ b/devtools/client/framework/source-map-url-service.js
@@ -56,27 +56,27 @@ SourceMapURLService.prototype._getLoadin
     this._loadingPromise = (async () => {
       if (this._target.isWorkerTarget) {
         return;
       }
       this._stylesheetsFront = await this._target.getFront("stylesheets");
       this._stylesheetsFront.on("stylesheet-added", this._onNewStyleSheet);
       const styleSheetsLoadingPromise =
           this._stylesheetsFront.getStyleSheets().then(sheets => {
-            sheets.forEach(this._onNewStyleSheet);
+            sheets.forEach(this._registerNewStyleSheet, this);
           }, () => {
             // Ignore any protocol-based errors.
           });
 
       // Start fetching the sources now.
       const loadingPromise = this._toolbox.threadClient.getSources().then(({sources}) => {
         // Ignore errors.  Register the sources we got; we can't rely on
         // an event to arrive if the source actor already existed.
         for (const source of sources) {
-          this._onSourceUpdated({source});
+          this._registerNewSource(source);
         }
       }, e => {
         // Also ignore any protocol-based errors.
       });
 
       await styleSheetsLoadingPromise;
       await loadingPromise;
     })();
@@ -87,16 +87,17 @@ 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();
+  this._loadingPromise = null;
 };
 
 /**
  * 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() {
@@ -109,47 +110,84 @@ SourceMapURLService.prototype.destroy = 
   Services.prefs.removeObserver(SOURCE_MAP_PREF, this._onPrefChanged);
   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) {
+  const url = this._registerNewSource(sourceEvent.source);
+
+  if (url) {
+    // Subscribers might have been added for this file before the
+    // "source-updated" event was fired.
+    this._dispatchSubscribersForURL(url);
+  }
+};
+
+/**
+ * A helper function that registers a new source file with the service.
+ *
+ * @param {SourceActor} source The new source's actor.
+ * @returns {string | undefined} A URL for the registered file,
+ *        if registered successfully.
+ */
+SourceMapURLService.prototype._registerNewSource = function(source) {
   // Maybe we were shut down while waiting.
   if (!this._urls) {
     return;
   }
 
-  const { source } = sourceEvent;
   const { generatedUrl, url, actor: id, sourceMapURL } = source;
 
   // |generatedUrl| comes from the actor and is extracted from the
   // source code by SpiderMonkey.
   const seenUrl = generatedUrl || url;
   this._urls.set(seenUrl, { id, url: seenUrl, sourceMapURL });
   this._idMap.set(id, seenUrl);
+
+  return 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) {
+  const url = this._registerNewStyleSheet(sheet);
+
+  if (url) {
+    // Subscribers might have been added for this file before the
+    // "stylesheet-added" event was fired.
+    this._dispatchSubscribersForURL(url);
+  }
+};
+
+/**
+ * A helper function that registers a new stylesheet with the service.
+ * @param {StyleSheetActor} sheet
+ *        The new style sheet's actor.
+ * @returns {string | undefined} A URL for the registered file,
+ *        if registered successfully.
+ */
+SourceMapURLService.prototype._registerNewStyleSheet = function(sheet) {
   // Maybe we were shut down while waiting.
   if (!this._urls) {
     return;
   }
 
   const {href, nodeHref, sourceMapURL, actorID: id} = sheet;
   const url = href || nodeHref;
   this._urls.set(url, { id, url, sourceMapURL});
   this._idMap.set(id, url);
+
+  return 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
@@ -162,27 +200,36 @@ SourceMapURLService.prototype.sourceMapC
     return;
   }
 
   const 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 (const [, subscriptionEntry] of this._subscriptions) {
-      if (subscriptionEntry.url === urlKey) {
-        // Force an update.
-        subscriptionEntry.promise = null;
-        for (const callback of subscriptionEntry.callbacks) {
-          this._callOneCallback(subscriptionEntry, callback);
-        }
+    this._dispatchSubscribersForURL(urlKey);
+  }
+};
+
+/**
+ * A helper function that dispatches subscribers for a specific URL.
+ * @param {string} urlKey
+ *        The url to trigger subscribers for.
+ */
+SourceMapURLService.prototype._dispatchSubscribersForURL = function(urlKey) {
+  // 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 (const [, subscriptionEntry] of this._subscriptions) {
+    if (subscriptionEntry.url === urlKey) {
+      // Force an update.
+      subscriptionEntry.promise = null;
+      for (const 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
--- a/devtools/client/framework/test/browser.ini
+++ b/devtools/client/framework/test/browser.ini
@@ -6,16 +6,18 @@ support-files =
   browser_toolbox_options_disable_js_iframe.html
   browser_toolbox_options_disable_cache.sjs
   browser_toolbox_sidebar_existing_tabs.xul
   browser_toolbox_sidebar_events.xul
   browser_toolbox_sidebar_tool.xul
   browser_toolbox_sidebar_toolURL.xul
   browser_toolbox_window_title_changes_page.html
   browser_toolbox_window_title_frame_select_page.html
+  code_bundle_late_script.js
+  code_bundle_late_script.js.map
   code_binary_search.coffee
   code_binary_search.js
   code_binary_search.map
   code_binary_search_absolute.js
   code_binary_search_absolute.map
   code_bundle_cross_domain.js
   code_bundle_cross_domain.js.map
   code_bundle_no_race.js
@@ -66,16 +68,17 @@ skip-if = os == 'win' || debug # Bug 128
 [browser_new_activation_workflow.js]
 [browser_source_map-01.js]
 [browser_source_map-absolute.js]
 [browser_source_map-cross-domain.js]
 [browser_source_map-init.js]
 [browser_source_map-inline.js]
 [browser_source_map-no-race.js]
 [browser_source_map-reload.js]
+[browser_source_map-late-script.js]
 [browser_target_from_url.js]
 [browser_target_events.js]
 [browser_target_remote.js]
 [browser_target_support.js]
 [browser_toolbox_custom_host.js]
 [browser_toolbox_dynamic_registration.js]
 [browser_toolbox_getpanelwhenready.js]
 [browser_toolbox_highlight.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/framework/test/browser_source_map-late-script.js
@@ -0,0 +1,51 @@
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test that you can subscribe to notifications on a source before it has loaded.
+
+"use strict";
+
+const PAGE_URL = `${URL_ROOT}doc_empty-tab-01.html`;
+const JS_URL = URL_ROOT + "code_bundle_late_script.js";
+
+const ORIGINAL_URL = "webpack:///code_late_script.js";
+
+const GENERATED_LINE = 107;
+const ORIGINAL_LINE = 11;
+
+add_task(async function() {
+  // Start with the empty page, then navigate, so that we can properly
+  // listen for new sources arriving.
+  const toolbox = await openNewTabAndToolbox(PAGE_URL, "webconsole");
+  const service = toolbox.sourceMapURLService;
+
+  const scriptMapped = new Promise(resolve => {
+    let count = 0;
+    service.subscribe(
+      JS_URL,
+      GENERATED_LINE,
+      undefined,
+      (...args) => {
+        if (count === 0) {
+          resolve(args);
+        }
+        count += 1;
+      }
+    );
+  });
+
+  // Inject JS script
+  const sourceSeen = waitForSourceLoad(toolbox, JS_URL);
+  await createScript(JS_URL);
+  await sourceSeen;
+
+  // Ensure that the URL service fired an event about the location loading.
+  const [isSourceMapped, url, line] = await scriptMapped;
+  is(isSourceMapped, true, "check is mapped");
+  is(url, ORIGINAL_URL, "check mapped URL");
+  is(line, ORIGINAL_LINE, "check mapped line number");
+
+  await toolbox.destroy();
+  gBrowser.removeCurrentTab();
+  finish();
+});
new file mode 100644
--- /dev/null
+++ b/devtools/client/framework/test/code_bundle_late_script.js
@@ -0,0 +1,116 @@
+/******/ (function(modules) { // webpackBootstrap
+/******/ 	// The module cache
+/******/ 	var installedModules = {};
+/******/
+/******/ 	// The require function
+/******/ 	function __webpack_require__(moduleId) {
+/******/
+/******/ 		// Check if module is in cache
+/******/ 		if(installedModules[moduleId]) {
+/******/ 			return installedModules[moduleId].exports;
+/******/ 		}
+/******/ 		// Create a new module (and put it into the cache)
+/******/ 		var module = installedModules[moduleId] = {
+/******/ 			i: moduleId,
+/******/ 			l: false,
+/******/ 			exports: {}
+/******/ 		};
+/******/
+/******/ 		// Execute the module function
+/******/ 		modules[moduleId].call(module.exports, module, module.exports, __webpack_require__);
+/******/
+/******/ 		// Flag the module as loaded
+/******/ 		module.l = true;
+/******/
+/******/ 		// Return the exports of the module
+/******/ 		return module.exports;
+/******/ 	}
+/******/
+/******/
+/******/ 	// expose the modules object (__webpack_modules__)
+/******/ 	__webpack_require__.m = modules;
+/******/
+/******/ 	// expose the module cache
+/******/ 	__webpack_require__.c = installedModules;
+/******/
+/******/ 	// define getter function for harmony exports
+/******/ 	__webpack_require__.d = function(exports, name, getter) {
+/******/ 		if(!__webpack_require__.o(exports, name)) {
+/******/ 			Object.defineProperty(exports, name, { enumerable: true, get: getter });
+/******/ 		}
+/******/ 	};
+/******/
+/******/ 	// define __esModule on exports
+/******/ 	__webpack_require__.r = function(exports) {
+/******/ 		if(typeof Symbol !== 'undefined' && Symbol.toStringTag) {
+/******/ 			Object.defineProperty(exports, Symbol.toStringTag, { value: 'Module' });
+/******/ 		}
+/******/ 		Object.defineProperty(exports, '__esModule', { value: true });
+/******/ 	};
+/******/
+/******/ 	// create a fake namespace object
+/******/ 	// mode & 1: value is a module id, require it
+/******/ 	// mode & 2: merge all properties of value into the ns
+/******/ 	// mode & 4: return value when already ns object
+/******/ 	// mode & 8|1: behave like require
+/******/ 	__webpack_require__.t = function(value, mode) {
+/******/ 		if(mode & 1) value = __webpack_require__(value);
+/******/ 		if(mode & 8) return value;
+/******/ 		if((mode & 4) && typeof value === 'object' && value && value.__esModule) return value;
+/******/ 		var ns = Object.create(null);
+/******/ 		__webpack_require__.r(ns);
+/******/ 		Object.defineProperty(ns, 'default', { enumerable: true, value: value });
+/******/ 		if(mode & 2 && typeof value != 'string') for(var key in value) __webpack_require__.d(ns, key, function(key) { return value[key]; }.bind(null, key));
+/******/ 		return ns;
+/******/ 	};
+/******/
+/******/ 	// getDefaultExport function for compatibility with non-harmony modules
+/******/ 	__webpack_require__.n = function(module) {
+/******/ 		var getter = module && module.__esModule ?
+/******/ 			function getDefault() { return module['default']; } :
+/******/ 			function getModuleExports() { return module; };
+/******/ 		__webpack_require__.d(getter, 'a', getter);
+/******/ 		return getter;
+/******/ 	};
+/******/
+/******/ 	// Object.prototype.hasOwnProperty.call
+/******/ 	__webpack_require__.o = function(object, property) { return Object.prototype.hasOwnProperty.call(object, property); };
+/******/
+/******/ 	// __webpack_public_path__
+/******/ 	__webpack_require__.p = "";
+/******/
+/******/
+/******/ 	// Load entry module and return exports
+/******/ 	return __webpack_require__(__webpack_require__.s = "./code_late_script.js");
+/******/ })
+/************************************************************************/
+/******/ ({
+
+/***/ "./code_late_script.js":
+/*!*****************************!*\
+  !*** ./code_late_script.js ***!
+  \*****************************/
+/*! no static exports found */
+/***/ (function(module, exports, __webpack_require__) {
+
+"use strict";
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Original source code for the inline source map test.
+// The generated file was made with
+//    webpack --devtool source-map code_late_script.js code_bundle_late_script.js
+
+
+
+function f() {
+  console.log("The first version of the script");
+}
+
+f();
+
+
+/***/ })
+
+/******/ });
+//# sourceMappingURL=code_bundle_late_script.js.map
\ No newline at end of file
new file mode 100644
--- /dev/null
+++ b/devtools/client/framework/test/code_bundle_late_script.js.map
@@ -0,0 +1,1 @@
+{"version":3,"sources":["webpack:///webpack/bootstrap","webpack:///./code_late_script.js"],"names":[],"mappings":";AAAA;AACA;;AAEA;AACA;;AAEA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;;AAEA;AACA;;AAEA;AACA;;AAEA;AACA;AACA;;;AAGA;AACA;;AAEA;AACA;;AAEA;AACA;AACA;AACA,kDAA0C,gCAAgC;AAC1E;AACA;;AAEA;AACA;AACA;AACA,gEAAwD,kBAAkB;AAC1E;AACA,yDAAiD,cAAc;AAC/D;;AAEA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA,iDAAyC,iCAAiC;AAC1E,wHAAgH,mBAAmB,EAAE;AACrI;AACA;;AAEA;AACA;AACA;AACA,mCAA2B,0BAA0B,EAAE;AACvD,yCAAiC,eAAe;AAChD;AACA;AACA;;AAEA;AACA,8DAAsD,+DAA+D;;AAErH;AACA;;;AAGA;AACA;;;;;;;;;;;;;AClFA;AACA;;AAEA;AACA;AACA;;AAEa;;AAEb;AACA;AACA;;AAEA","file":"code_bundle_late_script.js","sourcesContent":[" \t// The module cache\n \tvar installedModules = {};\n\n \t// The require function\n \tfunction __webpack_require__(moduleId) {\n\n \t\t// Check if module is in cache\n \t\tif(installedModules[moduleId]) {\n \t\t\treturn installedModules[moduleId].exports;\n \t\t}\n \t\t// Create a new module (and put it into the cache)\n \t\tvar module = installedModules[moduleId] = {\n \t\t\ti: moduleId,\n \t\t\tl: false,\n \t\t\texports: {}\n \t\t};\n\n \t\t// Execute the module function\n \t\tmodules[moduleId].call(module.exports, module, module.exports, __webpack_require__);\n\n \t\t// Flag the module as loaded\n \t\tmodule.l = true;\n\n \t\t// Return the exports of the module\n \t\treturn module.exports;\n \t}\n\n\n \t// expose the modules object (__webpack_modules__)\n \t__webpack_require__.m = modules;\n\n \t// expose the module cache\n \t__webpack_require__.c = installedModules;\n\n \t// define getter function for harmony exports\n \t__webpack_require__.d = function(exports, name, getter) {\n \t\tif(!__webpack_require__.o(exports, name)) {\n \t\t\tObject.defineProperty(exports, name, { enumerable: true, get: getter });\n \t\t}\n \t};\n\n \t// define __esModule on exports\n \t__webpack_require__.r = function(exports) {\n \t\tif(typeof Symbol !== 'undefined' && Symbol.toStringTag) {\n \t\t\tObject.defineProperty(exports, Symbol.toStringTag, { value: 'Module' });\n \t\t}\n \t\tObject.defineProperty(exports, '__esModule', { value: true });\n \t};\n\n \t// create a fake namespace object\n \t// mode & 1: value is a module id, require it\n \t// mode & 2: merge all properties of value into the ns\n \t// mode & 4: return value when already ns object\n \t// mode & 8|1: behave like require\n \t__webpack_require__.t = function(value, mode) {\n \t\tif(mode & 1) value = __webpack_require__(value);\n \t\tif(mode & 8) return value;\n \t\tif((mode & 4) && typeof value === 'object' && value && value.__esModule) return value;\n \t\tvar ns = Object.create(null);\n \t\t__webpack_require__.r(ns);\n \t\tObject.defineProperty(ns, 'default', { enumerable: true, value: value });\n \t\tif(mode & 2 && typeof value != 'string') for(var key in value) __webpack_require__.d(ns, key, function(key) { return value[key]; }.bind(null, key));\n \t\treturn ns;\n \t};\n\n \t// getDefaultExport function for compatibility with non-harmony modules\n \t__webpack_require__.n = function(module) {\n \t\tvar getter = module && module.__esModule ?\n \t\t\tfunction getDefault() { return module['default']; } :\n \t\t\tfunction getModuleExports() { return module; };\n \t\t__webpack_require__.d(getter, 'a', getter);\n \t\treturn getter;\n \t};\n\n \t// Object.prototype.hasOwnProperty.call\n \t__webpack_require__.o = function(object, property) { return Object.prototype.hasOwnProperty.call(object, property); };\n\n \t// __webpack_public_path__\n \t__webpack_require__.p = \"\";\n\n\n \t// Load entry module and return exports\n \treturn __webpack_require__(__webpack_require__.s = \"./code_late_script.js\");\n","/* Any copyright is dedicated to the Public Domain.\n http://creativecommons.org/publicdomain/zero/1.0/ */\n\n// Original source code for the inline source map test.\n// The generated file was made with\n//    webpack --devtool source-map code_late_script.js code_bundle_late_script.js\n\n\"use strict\";\n\nfunction f() {\n  console.log(\"The first version of the script\");\n}\n\nf();\n"],"sourceRoot":""}
\ No newline at end of file
new file mode 100644
--- /dev/null
+++ b/devtools/client/framework/test/code_late_script.js
@@ -0,0 +1,14 @@
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Original source code for the inline source map test.
+// The generated file was made with
+//    webpack --devtool source-map code_late_script.js --output code_bundle_late_script.js --mode development
+
+"use strict";
+
+function f() {
+  console.log("The first version of the script");
+}
+
+f();