Bug 1366853 - SourceMapURLService must wait for sources to be available; r=bgrins
☠☠ backed out by b609d94a575a ☠ ☠
authorTom Tromey <tom@tromey.com>
Mon, 22 May 2017 13:47:42 -0600
changeset 410178 30c2b38c4a0f3f26eeccc8eaadab09e4fd95028f
parent 410177 dfb811dc7408195c905b41dcf7a926837649864a
child 410179 50545ad26ef4d4eaf00a73d62c2abc7f12a2dffa
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins
bugs1366853
milestone55.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 1366853 - SourceMapURLService must wait for sources to be available; r=bgrins MozReview-Commit-ID: CVbYQCzJQTu
devtools/client/framework/source-map-url-service.js
devtools/client/framework/test/browser.ini
devtools/client/framework/test/browser_source_map-no-race.js
devtools/client/framework/test/code_bundle_no_race.js
devtools/client/framework/test/code_bundle_no_race.js.map
devtools/client/framework/test/code_no_race.js
devtools/client/framework/toolbox.js
--- a/devtools/client/framework/source-map-url-service.js
+++ b/devtools/client/framework/source-map-url-service.js
@@ -6,29 +6,39 @@
 /**
  * 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.
  *
  * @param {object} target
  *        The object the toolbox is debugging.
+ * @param {object} threadClient
+ *        The toolbox's thread client
  * @param {SourceMapService} sourceMapService
  *        The devtools-source-map functions
  */
-function SourceMapURLService(target, sourceMapService) {
+function SourceMapURLService(target, threadClient, sourceMapService) {
   this._target = target;
   this._sourceMapService = sourceMapService;
   this._urls = new Map();
 
   this._onSourceUpdated = this._onSourceUpdated.bind(this);
   this.reset = this.reset.bind(this);
 
   target.on("source-updated", this._onSourceUpdated);
   target.on("will-navigate", this.reset);
+
+  // Start fetching the sources now.
+  this._loadingPromise = new Promise(resolve => {
+    threadClient.getSources(({sources}) => {
+      // Just ignore errors.
+      resolve(sources);
+    });
+  });
 }
 
 /**
  * Reset the service.  This flushes the internal cache.
  */
 SourceMapURLService.prototype.reset = function () {
   this._sourceMapService.clearSourceMaps();
   this._urls.clear();
@@ -70,16 +80,24 @@ SourceMapURLService.prototype._onSourceU
  * @param {number} line
  *        The line number to map.
  * @param {number} column
  *        The column number to map.
  * @return Promise
  *        A promise resolving either to the original location, or null.
  */
 SourceMapURLService.prototype.originalPositionFor = async function (url, line, column) {
+  // Ensure the sources are loaded before replying.
+  await this._loadingPromise;
+
+  // Maybe we were shut down while waiting.
+  if (!this._urls) {
+    return null;
+  }
+
   const urlInfo = this._urls.get(url);
   if (!urlInfo) {
     return null;
   }
   // Call getOriginalURLs to make sure the source map has been
   // fetched.  We don't actually need the result of this though.
   await this._sourceMapService.getOriginalURLs(urlInfo);
   const location = { sourceId: urlInfo.id, line, column, sourceUrl: url };
--- a/devtools/client/framework/test/browser.ini
+++ b/devtools/client/framework/test/browser.ini
@@ -8,23 +8,26 @@ support-files =
   browser_toolbox_sidebar_tool.xul
   browser_toolbox_window_title_changes_page.html
   browser_toolbox_window_title_frame_select_page.html
   code_binary_search.coffee
   code_binary_search.js
   code_binary_search.map
   code_binary_search_absolute.js
   code_binary_search_absolute.map
+  code_bundle_no_race.js
+  code_bundle_no_race.js.map
   code_bundle_reload_1.js
   code_bundle_reload_1.js.map
   code_bundle_reload_2.js
   code_bundle_reload_2.js.map
   code_inline_bundle.js
   code_inline_original.js
   code_math.js
+  code_no_race.js
   code_reload_1.js
   code_reload_2.js
   doc_empty-tab-01.html
   doc_reload.html
   head.js
   shared-head.js
   shared-redux-head.js
   helper_disable_cache.js
@@ -47,16 +50,17 @@ support-files =
 [browser_keybindings_01.js]
 [browser_keybindings_02.js]
 [browser_keybindings_03.js]
 [browser_menu_api.js]
 [browser_new_activation_workflow.js]
 [browser_source_map-01.js]
 [browser_source_map-absolute.js]
 [browser_source_map-inline.js]
+[browser_source_map-no-race.js]
 [browser_source_map-reload.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]
new file mode 100644
--- /dev/null
+++ b/devtools/client/framework/test/browser_source_map-no-race.js
@@ -0,0 +1,41 @@
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test that the source map service doesn't race against source
+// reporting.
+
+"use strict";
+
+const JS_URL = URL_ROOT + "code_bundle_no_race.js";
+
+const PAGE_URL = `data:text/html,
+<!doctype html>
+
+<html>
+  <script src="${JS_URL}"></script>
+  <head>
+    <meta charset="utf-8"/>
+    <title>Empty test page to test race case</title>
+  </head>
+
+  <body>
+  </body>
+
+</html>`;
+
+const ORIGINAL_URL = "webpack:///code_no_race.js";
+
+const GENERATED_LINE = 84;
+const ORIGINAL_LINE = 11;
+
+add_task(function* () {
+  // Start with the empty page, then navigate, so that we can properly
+  // listen for new sources arriving.
+  const toolbox = yield openNewTabAndToolbox(PAGE_URL, "webconsole");
+  const service = toolbox.sourceMapURLService;
+
+  info(`checking original location for ${JS_URL}:${GENERATED_LINE}`);
+  let newLoc = yield service.originalPositionFor(JS_URL, GENERATED_LINE);
+  is(newLoc.sourceUrl, ORIGINAL_URL, "check mapped URL");
+  is(newLoc.line, ORIGINAL_LINE, "check mapped line number");
+});
new file mode 100644
--- /dev/null
+++ b/devtools/client/framework/test/code_bundle_no_race.js
@@ -0,0 +1,92 @@
+/******/ (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;
+/******/
+/******/ 	// identity function for calling harmony imports with the correct context
+/******/ 	__webpack_require__.i = function(value) { return value; };
+/******/
+/******/ 	// define getter function for harmony exports
+/******/ 	__webpack_require__.d = function(exports, name, getter) {
+/******/ 		if(!__webpack_require__.o(exports, name)) {
+/******/ 			Object.defineProperty(exports, name, {
+/******/ 				configurable: false,
+/******/ 				enumerable: true,
+/******/ 				get: getter
+/******/ 			});
+/******/ 		}
+/******/ 	};
+/******/
+/******/ 	// 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 = 0);
+/******/ })
+/************************************************************************/
+/******/ ([
+/* 0 */
+/***/ (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_no_race.js code_bundle_no_race.js
+
+
+
+function f() {
+  console.log("anything will do");
+}
+
+f();
+
+
+/***/ })
+/******/ ]);
+//# sourceMappingURL=code_bundle_no_race.js.map
\ No newline at end of file
new file mode 100644
--- /dev/null
+++ b/devtools/client/framework/test/code_bundle_no_race.js.map
@@ -0,0 +1,1 @@
+{"version":3,"sources":["webpack:///webpack/bootstrap 40c4319d19d88c63024f","webpack:///./code_no_race.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,mDAA2C,cAAc;;AAEzD;AACA;AACA;AACA;AACA;AACA;AACA;AACA,aAAK;AACL;AACA;;AAEA;AACA;AACA;AACA,mCAA2B,0BAA0B,EAAE;AACvD,yCAAiC,eAAe;AAChD;AACA;AACA;;AAEA;AACA,8DAAsD,+DAA+D;;AAErH;AACA;;AAEA;AACA;;;;;;;;AChEA;AACA;;AAEA;AACA;AACA;;AAEA;;AAEA;AACA;AACA;;AAEA","file":"code_bundle_no_race.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// identity function for calling harmony imports with the correct context\n \t__webpack_require__.i = function(value) { return value; };\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, {\n \t\t\t\tconfigurable: false,\n \t\t\t\tenumerable: true,\n \t\t\t\tget: getter\n \t\t\t});\n \t\t}\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 \t// Load entry module and return exports\n \treturn __webpack_require__(__webpack_require__.s = 0);\n\n\n\n// WEBPACK FOOTER //\n// webpack/bootstrap 40c4319d19d88c63024f","/* 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_no_race.js code_bundle_no_race.js\n\n\"use strict\";\n\nfunction f() {\n  console.log(\"anything will do\");\n}\n\nf();\n\n\n\n//////////////////\n// WEBPACK FOOTER\n// ./code_no_race.js\n// module id = 0\n// module chunks = 0"],"sourceRoot":""}
\ No newline at end of file
new file mode 100644
--- /dev/null
+++ b/devtools/client/framework/test/code_no_race.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_no_race.js code_bundle_no_race.js
+
+"use strict";
+
+function f() {
+  console.log("anything will do");
+}
+
+f();
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -555,17 +555,18 @@ Toolbox.prototype = {
   get sourceMapURLService() {
     if (this._sourceMapURLService) {
       return this._sourceMapURLService;
     }
     let sourceMaps = this.sourceMapService;
     if (!sourceMaps) {
       return null;
     }
-    this._sourceMapURLService = new SourceMapURLService(this._target, sourceMaps);
+    this._sourceMapURLService = new SourceMapURLService(this._target, this.threadClient,
+                                                        sourceMaps);
     return this._sourceMapURLService;
   },
 
   // Return HostType id for telemetry
   _getTelemetryHostId: function () {
     switch (this.hostType) {
       case Toolbox.HostType.BOTTOM: return 0;
       case Toolbox.HostType.SIDE: return 1;