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 491167 f3d51256c5d728e4b324525917b481565f5690d9
parent 491166 4c162e4eda0879b7ea1761c3db4679e403273479
child 491168 0b9e4abc5911fb241ba351c07e7cc2f3e4fb2ed7
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewerstromey
bugs1500632
milestone65.0a1
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();