Bug 1491354 - Extends top-level await mapping from debugger to toolbox; r=bgrins,jlast.
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Mon, 24 Sep 2018 08:17:30 +0000
changeset 493595 3eff766fa7c2146b53cfd273532b3d61f2d79ee6
parent 493594 5bd44d836cbfea1826511e91736a89c198c3123a
child 493596 787f9095543f9b60d22301d6c3ef06c4b47b1173
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins, jlast
bugs1491354, 1410820
milestone64.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 1491354 - Extends top-level await mapping from debugger to toolbox; r=bgrins,jlast. This patch makes the parser-worker available at the toolbox level. This way, the console does not have to rely on the debugger being open to map top-level await expression. In order to make the worker works in the toolbox, some changes are required (passing a window object, checking inToolbox differently). We take this as an opportunity to *not* display the async iife result, a promise, in the console. This is made by checking if the input was mapped, and if so, ignoring the result we get from the server. A couple tests are added to ensure the basic usage works as expected. This patch should be considered as a v0 for top-level await evaluation as there are things that are not perfect here. Since we rely on console.log the result are treated differently from other evaluation results: - the style is different - the result gets added to the log cache (when restarting the console, the results will still be displayed, but not the commands). - the results can be filtered, although evaluation results should not - `$_` after a top-level await evaluation returns the Promise created by the async iife, not the result that was displayed in the console. All those should be addressed in Bug 1410820. Differential Revision: https://phabricator.services.mozilla.com/D6038
devtools/client/debugger/new/dist/parser-worker.js
devtools/client/debugger/new/src/actions/expressions.js
devtools/client/debugger/new/src/actions/preview.js
devtools/client/debugger/panel.js
devtools/client/framework/toolbox.js
devtools/client/webconsole/components/JSTerm.js
devtools/client/webconsole/test/mochitest/browser.ini
devtools/client/webconsole/test/mochitest/browser_jsterm_await.js
devtools/client/webconsole/test/mochitest/browser_jsterm_await_paused.js
devtools/client/webconsole/webconsole.js
--- a/devtools/client/debugger/new/dist/parser-worker.js
+++ b/devtools/client/debugger/new/dist/parser-worker.js
@@ -25906,33 +25906,48 @@ var _mapBindings2 = _interopRequireDefau
 
 var _mapAwaitExpression = __webpack_require__(3778);
 
 var _mapAwaitExpression2 = _interopRequireDefault(_mapAwaitExpression);
 
 function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }
 
 function mapExpression(expression, mappings, bindings, shouldMapBindings = true, shouldMapAwait = true) {
+  const mapped = {
+    originalExpression: false,
+    bindings: false,
+    await: false,
+  };
+
   try {
     if (mappings) {
+      const beforeOriginalExpression = expression;
       expression = (0, _mapOriginalExpression2.default)(expression, mappings);
+      mapped.originalExpression = beforeOriginalExpression !== expression;
     }
 
     if (shouldMapBindings) {
+      const beforeBindings = expression;
       expression = (0, _mapBindings2.default)(expression, bindings);
+      mapped.bindings = beforeBindings !== expression;
     }
 
     if (shouldMapAwait) {
+      const beforeAwait = expression;
       expression = (0, _mapAwaitExpression2.default)(expression);
+      mapped.await = beforeAwait !== expression;
     }
   } catch (e) {
     console.log(e);
   }
 
-  return expression;
+  return {
+    expression,
+    mapped,
+  };
 } /* 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/>. */
 
 /***/ }),
 
 /***/ 3756:
 /***/ (function(module, exports, __webpack_require__) {
@@ -46747,17 +46762,17 @@ function wrapExpression(ast) {
   const newAst = t.arrowFunctionExpression([], t.blockStatement(body), true);
   return (0, _generator2.default)(newAst).code;
 }
 
 function handleTopLevelAwait(expression) {
   const ast = hasTopLevelAwait(expression);
   if (ast) {
     const func = wrapExpression(ast);
-    return (0, _generator2.default)(_template2.default.ast(`(${func})().then(r => console.log(r));`)).code;
+    return (0, _generator2.default)(_template2.default.ast(`(${func})().then(console.log).catch(console.error)`)).code;
   }
 
   return expression;
 }
 
 /***/ }),
 
 /***/ 3779:
--- a/devtools/client/debugger/new/src/actions/expressions.js
+++ b/devtools/client/debugger/new/src/actions/expressions.js
@@ -186,17 +186,20 @@ function evaluateExpression(expression) 
       const {
         location
       } = frame;
       const source = (0, _selectors.getSourceFromId)(getState(), location.sourceId);
       const sourceId = source.id;
       const selectedSource = (0, _selectors.getSelectedSource)(getState());
 
       if (selectedSource && !(0, _devtoolsSourceMap.isGeneratedId)(sourceId) && !(0, _devtoolsSourceMap.isGeneratedId)(selectedSource.id)) {
-        input = await dispatch(getMappedExpression(input));
+        const mapResult = await dispatch(getMappedExpression(input));
+        if (mapResult !== null) {
+          input = mapResult.expression;
+        }
       }
     }
 
     const frameId = (0, _selectors.getSelectedFrameId)(getState());
     return dispatch({
       type: "EVALUATE_EXPRESSION",
       input: expression.input,
       [_promise.PROMISE]: client.evaluateInFrame((0, _expressions.wrapExpression)(input), frameId)
@@ -221,14 +224,14 @@ function getMappedExpression(expression)
     // because mapping an expression can be slow if the parser worker is
     // busy doing other work.
     //
     // 1. there are no mappings - we do not need to map original expressions
     // 2. does not contain `await` - we do not need to map top level awaits
     // 3. does not contain `=` - we do not need to map assignments
 
     if (!mappings && !expression.match(/(await|=)/)) {
-      return expression;
+      return null;
     }
 
     return parser.mapExpression(expression, mappings, bindings || [], _prefs.features.mapExpressionBindings, _prefs.features.mapAwaitExpression);
   };
 }
\ No newline at end of file
--- a/devtools/client/debugger/new/src/actions/preview.js
+++ b/devtools/client/debugger/new/src/actions/preview.js
@@ -92,17 +92,21 @@ function setPreview(expression, location
         if (!source) {
           return;
         }
 
         const sourceId = source.id;
         const selectedFrame = (0, _selectors.getSelectedFrame)(getState());
 
         if (location && !(0, _devtoolsSourceMap.isGeneratedId)(sourceId)) {
-          expression = await dispatch((0, _expressions.getMappedExpression)(expression));
+          const mapResult =
+            await dispatch((0, _expressions.getMappedExpression)(expression));
+          if (mapResult !== null) {
+            expression = mapResult.expression;
+          }
         }
 
         if (!selectedFrame) {
           return;
         }
 
         const {
           result
--- a/devtools/client/debugger/panel.js
+++ b/devtools/client/debugger/panel.js
@@ -129,17 +129,17 @@ DebuggerPanel.prototype = {
     });
   },
 
   // DebuggerPanel API
 
   getMappedExpression(expression) {
     // No-op implementation since this feature doesn't exist in the older
     // debugger implementation.
-    return expression;
+    return null;
   },
 
   isPaused() {
     let framesController = this.panelWin.DebuggerController.StackFrames;
     let thread = framesController.activeThread;
     return thread && thread.paused;
   },
 
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -676,16 +676,31 @@ Toolbox.prototype = {
   get sourceMapService() {
     if (!Services.prefs.getBoolPref("devtools.source-map.client-service.enabled")) {
       return null;
     }
     return this._createSourceMapService();
   },
 
   /**
+   * A common access point for the client-side parser service that any panel can use.
+   */
+  get parserService() {
+    if (this._parserService) {
+      return this._parserService;
+    }
+
+    this._parserService =
+      this.browserRequire("devtools/client/debugger/new/src/workers/parser/index");
+    this._parserService
+      .start("resource://devtools/client/debugger/new/dist/parser-worker.js", this.win);
+    return this._parserService;
+  },
+
+  /**
    * Clients wishing to use source maps but that want the toolbox to
    * track the source and style sheet actor mapping can use this
    * source map service.  This is a higher-level service than the one
    * returned by |sourceMapService|, in that it automatically tracks
    * source and style sheet actor IDs.
    */
   get sourceMapURLService() {
     if (this._sourceMapURLService) {
@@ -2807,16 +2822,21 @@ Toolbox.prototype = {
       this._sourceMapURLService.destroy();
       this._sourceMapURLService = null;
     }
     if (this._sourceMapService) {
       this._sourceMapService.stopSourceMapWorker();
       this._sourceMapService = null;
     }
 
+    if (this._parserService) {
+      this._parserService.stop();
+      this._parserService = null;
+    }
+
     if (this.webconsolePanel) {
       this._saveSplitConsoleHeight();
       this.webconsolePanel.removeEventListener("resize",
         this._saveSplitConsoleHeight);
       this.webconsolePanel = null;
     }
     if (this.textBoxContextMenuPopup) {
       this.textBoxContextMenuPopup.removeEventListener("popupshowing",
--- a/devtools/client/webconsole/components/JSTerm.js
+++ b/devtools/client/webconsole/components/JSTerm.js
@@ -423,23 +423,36 @@ class JSTerm extends Component {
       this.inputNode.focus();
     }
   }
 
   /**
    * The JavaScript evaluation response handler.
    *
    * @private
-   * @param object response
+   * @param {Object} response
    *        The message received from the server.
+   * @param {Object} options
+   *        On options object that can contain the following properties:
+   *          - {Object} mapped: An object indicating if the input was modified by the
+   *                             parser worker.
    */
-  async _executeResultCallback(response) {
+  async _executeResultCallback(response, options = {}) {
     if (!this.hud) {
       return null;
     }
+
+    // If the expression was a top-level await that the parser-worker transformed, we
+    // don't want to show the result returned by the server as it's a Promise that was
+    // created on our end by wrapping the input in an instantly called async function
+    // (e.g. `await 42` -> `(async () => {return await 42})()`).
+    if (options && options.mapped && options.mapped.await) {
+      return null;
+    }
+
     if (response.error) {
       console.error("Evaluation error " + response.error + ": " + response.message);
       return null;
     }
     let errorMessage = response.exceptionMessage;
 
     // Wrap thrown strings in Error objects, so `throw "foo"` outputs "Error: foo"
     if (typeof response.exception === "string") {
@@ -541,28 +554,38 @@ class JSTerm extends Component {
     }
 
     const { ConsoleCommand } = require("devtools/client/webconsole/types");
     const cmdMessage = new ConsoleCommand({
       messageText: executeString,
     });
     this.hud.proxy.dispatchMessageAdd(cmdMessage);
 
+    let mappedExpressionRes = null;
+    try {
+      mappedExpressionRes = await this.hud.owner.getMappedExpression(executeString);
+    } catch (e) {
+      console.warn("Error when calling getMappedExpression", e);
+    }
+
+    executeString = mappedExpressionRes ? mappedExpressionRes.expression : executeString;
+
     const options = {
       frame: this.SELECTED_FRAME,
       selectedNodeActor,
     };
 
-    const mappedString = await this.hud.owner.getMappedExpression(executeString);
     // Even if requestEvaluation rejects (because of webConsoleClient.evaluateJSAsync),
     // we still need to pass the error response to executeResultCallback.
-    const onEvaluated = this.requestEvaluation(mappedString, options)
+    const onEvaluated = this.requestEvaluation(executeString, options)
       .then(res => res, res => res);
     const response = await onEvaluated;
-    return this._executeResultCallback(response);
+    return this._executeResultCallback(response, {
+      mapped: mappedExpressionRes ? mappedExpressionRes.mapped : null
+    });
   }
 
   /**
    * Request a JavaScript string evaluation from the server.
    *
    * @param string str
    *        String to execute.
    * @param object [options]
--- a/devtools/client/webconsole/test/mochitest/browser.ini
+++ b/devtools/client/webconsole/test/mochitest/browser.ini
@@ -198,16 +198,18 @@ skip-if = verify
 [browser_jsterm_autocomplete_inside_text.js]
 [browser_jsterm_autocomplete_native_getters.js]
 [browser_jsterm_autocomplete_nav_and_tab_key.js]
 [browser_jsterm_autocomplete_paste_undo.js]
 [browser_jsterm_autocomplete_return_key_no_selection.js]
 [browser_jsterm_autocomplete_return_key.js]
 [browser_jsterm_autocomplete_width.js]
 [browser_jsterm_autocomplete-properties-with-non-alphanumeric-names.js]
+[browser_jsterm_await_paused.js]
+[browser_jsterm_await.js]
 [browser_jsterm_completion_case_sensitivity.js]
 [browser_jsterm_completion.js]
 [browser_jsterm_content_defined_helpers.js]
 [browser_jsterm_copy_command.js]
 [browser_jsterm_ctrl_a_select_all.js]
 [browser_jsterm_ctrl_key_nav.js]
 skip-if = os != 'mac' # The tested ctrl+key shortcuts are OSX only
 [browser_jsterm_document_no_xray.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/webconsole/test/mochitest/browser_jsterm_await.js
@@ -0,0 +1,85 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test that top-level await expression work as expected.
+
+"use strict";
+
+const TEST_URI = "data:text/html;charset=utf-8,Web Console test top-level await";
+
+add_task(async function() {
+  // Enable await mapping.
+  await pushPref("devtools.debugger.features.map-await-expression", true);
+
+  // Run test with legacy JsTerm
+  await pushPref("devtools.webconsole.jsterm.codeMirror", false);
+  await performTests();
+  // And then run it with the CodeMirror-powered one.
+  await pushPref("devtools.webconsole.jsterm.codeMirror", true);
+  await performTests();
+});
+
+async function performTests() {
+  const hud = await openNewTabAndConsole(TEST_URI);
+  const {jsterm} = hud;
+
+  info("Evaluate a top-level await expression");
+  let onMessage = waitForMessage(hud, "await1");
+  // We use "await" + 1 to not match the evaluated command message.
+  const simpleAwait = `await new Promise(r => setTimeout(() => r("await" + 1), 1000))`;
+  jsterm.execute(simpleAwait);
+  await onMessage;
+  // Check that the resulting promise of the async iife is not displayed.
+  let messages = hud.ui.outputNode.querySelectorAll(".message .message-body");
+  let messagesText = Array.from(messages).map(n => n.textContent).join(" - ");
+  is(messagesText, `${simpleAwait} - await1`,
+    "The output contains the the expected messages");
+
+  info("Check that assigning the result of a top-level await expression works");
+  onMessage = waitForMessage(hud, "await2");
+  jsterm.execute(`x = await new Promise(r => setTimeout(() => r("await" + 2), 1000))`);
+  await onMessage;
+
+  onMessage = waitForMessage(hud, `"-await2-"`);
+  jsterm.execute(`"-" + x + "-"`);
+  let message = await onMessage;
+  ok(message.node, "`x` was assigned as expected");
+
+  info("Check that awaiting for a rejecting promise displays an error");
+  onMessage = waitForMessage(hud, "await-rej", ".message.error");
+  jsterm.execute(`x = await new Promise((resolve,reject) =>
+    setTimeout(() => reject("await-" + "rej"), 1000))`);
+  message = await onMessage;
+  ok(message.node, "awaiting for a rejecting promise displays an error message");
+
+  info("Check that concurrent await expression work fine");
+  hud.ui.clearOutput();
+  const delays = [2000, 500, 1000, 1500];
+  const inputs = delays.map(delay => `await new Promise(
+    r => setTimeout(() => r("await-concurrent-" + ${delay}), ${delay}))`);
+
+  // Let's wait for the message that sould be displayed last.
+  onMessage = waitForMessage(hud, "await-concurrent-2000");
+  for (const input of inputs) {
+    jsterm.execute(input);
+  }
+  await onMessage;
+
+  messages = hud.ui.outputNode.querySelectorAll(".message .message-body");
+  messagesText = Array.from(messages).map(n => n.textContent);
+  const expectedMessages = [
+    ...inputs,
+    "await-concurrent-500",
+    "await-concurrent-1000",
+    "await-concurrent-1500",
+    "await-concurrent-2000",
+  ];
+  is(JSON.stringify(messagesText, null, 2), JSON.stringify(expectedMessages, null, 2),
+    "The output contains the the expected messages, in the expected order");
+
+  info("Check that a logged promise is still displayed as a promise");
+  onMessage = waitForMessage(hud, "Promise {");
+  jsterm.execute(`new Promise(r => setTimeout(() => r(1), 1000))`);
+  message = await onMessage;
+  ok(message, "Promise are displayed as expected");
+}
new file mode 100644
--- /dev/null
+++ b/devtools/client/webconsole/test/mochitest/browser_jsterm_await_paused.js
@@ -0,0 +1,62 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test that top-level await expression work as expected when debugger is paused.
+
+"use strict";
+
+const TEST_URI =
+  `data:text/html;charset=utf-8,Web Console test top-level await when debugger paused`;
+
+add_task(async function() {
+  // Enable await mapping.
+  await pushPref("devtools.debugger.features.map-await-expression", true);
+
+  // Run test with legacy JsTerm
+  await pushPref("devtools.webconsole.jsterm.codeMirror", false);
+  await performTests();
+  // And then run it with the CodeMirror-powered one.
+  await pushPref("devtools.webconsole.jsterm.codeMirror", true);
+  await performTests();
+});
+
+async function performTests() {
+  const hud = await openNewTabAndConsole(TEST_URI);
+  const {jsterm} = hud;
+
+  const pauseExpression = `(() => {
+    var foo = "bar";
+    /* Will pause the script and open the debugger panel */
+    debugger;
+  })()`;
+  jsterm.execute(pauseExpression);
+
+  // wait for the debugger to be opened and paused.
+  const target = TargetFactory.forTab(gBrowser.selectedTab);
+  const toolbox = gDevTools.getToolbox(target);
+  const dbg = await waitFor(() => toolbox.getPanel("jsdebugger"));
+  await waitFor(() => dbg._selectors.isPaused(dbg._getState()));
+  await toolbox.toggleSplitConsole();
+
+  const onMessage = waitForMessage(hud, "res: bar");
+  const awaitExpression = `await new Promise(res => {
+    setTimeout(() => res("res: " + foo), 1000);
+  })`;
+  await jsterm.execute(awaitExpression);
+
+  // Click on the resume button to not be paused anymore.
+  dbg.panelWin.document.querySelector("button.resume").click();
+
+  await onMessage;
+  const messages = hud.ui.outputNode.querySelectorAll(".message .message-body");
+  const messagesText = Array.from(messages).map(n => n.textContent);
+  const expectedMessages = [
+    pauseExpression,
+    awaitExpression,
+    // The result of pauseExpression
+    "undefined",
+    "res: bar",
+  ];
+  is(JSON.stringify(messagesText, null, 2), JSON.stringify(expectedMessages, null, 2),
+    "The output contains the the expected messages, in the expected order");
+}
--- a/devtools/client/webconsole/webconsole.js
+++ b/devtools/client/webconsole/webconsole.js
@@ -221,28 +221,47 @@ WebConsole.prototype = {
 
     if (!panel) {
       return null;
     }
 
     return panel.getFrames();
   },
 
-  async getMappedExpression(expression) {
+  /**
+   * Given an expression, returns an object containing a new expression, mapped by the
+   * parser worker to provide additional feature for the user (top-level await,
+   * original languages mapping, …).
+   *
+   * @param {String} expression: The input to maybe map.
+   * @returns {Object|null}
+   *          Returns null if the input can't be mapped.
+   *          If it can, returns an object containing the following:
+   *            - {String} expression: The mapped expression
+   *            - {Object} mapped: An object containing the different mapping that could
+   *                               be done and if they were applied on the input.
+   *                               At the moment, contains `await`, `bindings` and
+   *                               `originalExpression`.
+   */
+  getMappedExpression(expression) {
     const toolbox = gDevTools.getToolbox(this.target);
     if (!toolbox) {
-      return expression;
-    }
-    const panel = toolbox.getPanel("jsdebugger");
-
-    if (!panel) {
-      return expression;
+      return null;
     }
 
-    return panel.getMappedExpression(expression);
+    const panel = toolbox.getPanel("jsdebugger");
+    if (panel) {
+      return panel.getMappedExpression(expression);
+    }
+
+    if (toolbox.parserService && expression.includes("await ")) {
+      return toolbox.parserService.mapExpression(expression);
+    }
+
+    return null;
   },
 
   /**
    * Retrieves the current selection from the Inspector, if such a selection
    * exists. This is used to pass the ID of the selected actor to the Web
    * Console server for the $0 helper.
    *
    * @return object|null