diff --git a/flow-typed/debugger-html.js b/flow-typed/debugger-html.js index 95854d25ca..6f1e094570 100644 --- a/flow-typed/debugger-html.js +++ b/flow-typed/debugger-html.js @@ -155,9 +155,13 @@ declare module "debugger-html" { declare type Why = | {| exception: string | Grip, - type: "exception" + type: "exception", + frameFinished?: Object |} - | { type: string }; + | { + type: string, + frameFinished?: Object + }; /** * Why is the Debugger Paused? @@ -185,6 +189,7 @@ declare module "debugger-html" { * @static */ declare type Pause = { + frame: Frame, frames: Frame[], why: Why, loadedObjects?: LoadedObject[] diff --git a/src/test/mochitest/browser.ini b/src/test/mochitest/browser.ini index a33e69f86b..c453016cf2 100644 --- a/src/test/mochitest/browser.ini +++ b/src/test/mochitest/browser.ini @@ -92,6 +92,7 @@ skip-if = os == "linux" # bug 1351952 [browser_dbg-pretty-print-console.js] [browser_dbg-pretty-print-paused.js] [browser_dbg-preview.js] +[browser_dbg-returnvalues.js] [browser_dbg-scopes-mutations.js] [browser_dbg-search-file.js] skip-if = os == "win" # Bug 1393121 diff --git a/src/test/mochitest/browser_dbg-returnvalues.js b/src/test/mochitest/browser_dbg-returnvalues.js new file mode 100644 index 0000000000..aacb8da3cf --- /dev/null +++ b/src/test/mochitest/browser_dbg-returnvalues.js @@ -0,0 +1,69 @@ +function getLabel(dbg, index) { + return findElement(dbg, "scopeNode", index).innerText; +} + +function getValue(dbg, index) { + return findElement(dbg, "scopeValue", index).innerText; +} + +function toggleScopes(dbg) { + return findElement(dbg, "scopesHeader").click(); +} + +async function testReturnValue(dbg, val) { + invokeInTab("return_something", val); + await waitForPaused(dbg); + + // "Step in" 3 times to get to the point where the debugger can + // see the return value. + await stepIn(dbg); + await stepIn(dbg); + await stepIn(dbg); + + is(getLabel(dbg, 1), "return_something", "check for return_something"); + + // We don't show "undefined" but we do show other falsy values. + let label = getLabel(dbg, 2); + if (val === "undefined") { + ok(label !== "", "do not show for undefined"); + } else { + is(label, "", "check for "); + // The "uneval" call here gives us the way one would write `val` as + // a JavaScript expression, similar to the way the debugger + // displays values, so this test works when `val` is a string. + is(getValue(dbg, 2), uneval(val), `check value is ${uneval(val)}`); + } + + await resume(dbg); + assertNotPaused(dbg); +} + +async function testThrowValue(dbg, val) { + invokeInTab("throw_something", val); + await waitForPaused(dbg); + + // "Step in" once to get to the point where the debugger can see the + // exception. + await stepIn(dbg); + + is(getLabel(dbg, 1), "callee", "check for callee"); + is(getLabel(dbg, 2), "", "check for "); + // The "uneval" call here gives us the way one would write `val` as + // a JavaScript expression, similar to the way the debugger + // displays values, so this test works when `val` is a string. + is(getValue(dbg, 2), uneval(val), `check exception is ${uneval(val)}`); + + await resume(dbg); + await waitForPaused(dbg); + await resume(dbg); + assertNotPaused(dbg); +} + +add_task(async function() { + const dbg = await initDebugger("doc-return-values.html"); + toggleScopes(dbg); + await togglePauseOnExceptions(dbg, true, false); + + await testReturnValue(dbg, "to sender"); + await testThrowValue(dbg, "a fit"); +}); diff --git a/src/test/mochitest/head.js b/src/test/mochitest/head.js index 049cb93160..ef23594210 100644 --- a/src/test/mochitest/head.js +++ b/src/test/mochitest/head.js @@ -256,6 +256,15 @@ function waitForSelectedSource(dbg, url) { ); } +/** + * Assert that the debugger is not currently paused. + * @memberof mochitest/asserts + * @static + */ +function assertNotPaused(dbg) { + ok(!isPaused(dbg), "client is not paused"); +} + /** * Assert that the debugger is paused at the correct location. * @@ -726,14 +735,20 @@ function waitForActive(dbg) { * Invokes a global function in the debuggee tab. * * @memberof mochitest/helpers - * @param {String} fnc + * @param {String} fnc The name of a global function on the content window to + * call. This is applied to structured clones of the + * remaining arguments to invokeInTab. + * @param {Any} ...args Remaining args to serialize and pass to fnc. * @return {Promise} * @static */ -function invokeInTab(fnc) { - info(`Invoking function ${fnc} in tab`); - return ContentTask.spawn(gBrowser.selectedBrowser, fnc, function*(fnc) { - content.wrappedJSObject[fnc](); // eslint-disable-line mozilla/no-cpows-in-tests, max-len +function invokeInTab(fnc, ...args) { + info(`Invoking in tab: ${fnc}(${args.map(uneval).join(",")})`); + return ContentTask.spawn(gBrowser.selectedBrowser, { fnc, args }, function*({ + fnc, + args + }) { + content.wrappedJSObject[fnc](...args); // eslint-disable-line mozilla/no-cpows-in-tests, max-len }); } diff --git a/src/utils/scopes.js b/src/utils/scopes.js index 6b1dd0de5e..c6f317207d 100644 --- a/src/utils/scopes.js +++ b/src/utils/scopes.js @@ -5,7 +5,6 @@ // @flow import { toPairs } from "lodash"; -import { get } from "lodash"; import { simplifyDisplayName } from "./frame"; import type { Frame, Pause, Scope, BindingContents } from "debugger-html"; @@ -74,30 +73,34 @@ function getSourceBindingVariables( return bound.concat(unused); } -export function getSpecialVariables(pauseInfo: Pause, path: string) { - const thrown = get(pauseInfo, "why.frameFinished.throw", undefined); - - const returned = get(pauseInfo, "why.frameFinished.return", undefined); - +export function getFramePopVariables(pauseInfo: Pause, path: string) { const vars = []; - if (thrown !== undefined) { - vars.push({ - name: "", - path: `${path}/`, - contents: { value: thrown } - }); - } + if (pauseInfo.why && pauseInfo.why.frameFinished) { + const frameFinished = pauseInfo.why.frameFinished; - if (returned !== undefined) { - // Do not display a return value of "undefined", - if (!returned || !returned.type || returned.type !== "undefined") { + // Always display a `throw` property if present, even if it is falsy. + if (Object.prototype.hasOwnProperty.call(frameFinished, "throw")) { vars.push({ - name: "", - path: `${path}/`, - contents: { value: returned } + name: "", + path: `${path}/`, + contents: { value: frameFinished.throw } }); } + + if (Object.prototype.hasOwnProperty.call(frameFinished, "return")) { + const returned = frameFinished.return; + + // Do not display undefined. Do display falsy values like 0 and false. The + // protocol grip for undefined is a JSON object: { type: "undefined" }. + if (typeof returned !== "object" || returned.type !== "undefined") { + vars.push({ + name: "", + path: `${path}/`, + contents: { value: returned } + }); + } + } } return vars; @@ -137,7 +140,6 @@ export function getScopes( const scopes = []; let scope = selectedScope; - const pausedScopeActor = get(pauseInfo, "frame.scope.actor"); let scopeIndex = 1; do { @@ -159,9 +161,13 @@ export function getScopes( ? getSourceBindingVariables(bindings, sourceBindings, key) : getBindingVariables(bindings, key); - // show exception, return, and this variables in innermost scope - if (scope.actor === pausedScopeActor) { - vars = vars.concat(getSpecialVariables(pauseInfo, key)); + // On the innermost scope of a frame that is just about to be popped, show + // the return value or the exception being thrown as special variables. + if ( + scope.actor === selectedScope.actor && + selectedFrame.id === pauseInfo.frame.id + ) { + vars = vars.concat(getFramePopVariables(pauseInfo, key)); } if (scope.actor === selectedScope.actor) { diff --git a/src/utils/tests/scopes.spec.js b/src/utils/tests/scopes.spec.js index c9b07327e0..dc451f3651 100644 --- a/src/utils/tests/scopes.spec.js +++ b/src/utils/tests/scopes.spec.js @@ -1,4 +1,4 @@ -import { getSpecialVariables, getScopes } from "../scopes"; +import { getFramePopVariables, getScopes } from "../scopes"; const errorGrip = { type: "object", @@ -43,7 +43,7 @@ function throwWhy(grip) { } describe("scopes", () => { - describe("getSpecialVariables", () => { + describe("getFramePopVariables", () => { describe("falsey values", () => { // NOTE: null and undefined are treated like objects and given a type const falsey = { false: false, "0": 0, null: { type: "null" } }; @@ -51,7 +51,7 @@ describe("scopes", () => { const value = falsey[test]; it(`shows ${test} returns`, () => { const pauseData = returnWhy(value); - const vars = getSpecialVariables(pauseData, ""); + const vars = getFramePopVariables(pauseData, ""); expect(vars[0].name).toEqual(""); expect(vars[0].name).toEqual(""); expect(vars[0].contents.value).toEqual(value); @@ -59,7 +59,7 @@ describe("scopes", () => { it(`shows ${test} throws`, () => { const pauseData = throwWhy(value); - const vars = getSpecialVariables(pauseData, ""); + const vars = getFramePopVariables(pauseData, ""); expect(vars[0].name).toEqual(""); expect(vars[0].name).toEqual(""); expect(vars[0].contents.value).toEqual(value); @@ -70,7 +70,7 @@ describe("scopes", () => { describe("Error / Objects", () => { it("shows Error returns", () => { const pauseData = returnWhy(errorGrip); - const vars = getSpecialVariables(pauseData, ""); + const vars = getFramePopVariables(pauseData, ""); expect(vars[0].name).toEqual(""); expect(vars[0].name).toEqual(""); expect(vars[0].contents.value.class).toEqual("Error"); @@ -78,7 +78,7 @@ describe("scopes", () => { it("shows error throws", () => { const pauseData = throwWhy(errorGrip); - const vars = getSpecialVariables(pauseData, ""); + const vars = getFramePopVariables(pauseData, ""); expect(vars[0].name).toEqual(""); expect(vars[0].name).toEqual(""); expect(vars[0].contents.value.class).toEqual("Error"); @@ -88,13 +88,13 @@ describe("scopes", () => { describe("undefined", () => { it("does not show undefined returns", () => { const pauseData = returnWhy({ type: "undefined" }); - const vars = getSpecialVariables(pauseData, ""); + const vars = getFramePopVariables(pauseData, ""); expect(vars.length).toEqual(0); }); it("shows undefined throws", () => { const pauseData = throwWhy({ type: "undefined" }); - const vars = getSpecialVariables(pauseData, ""); + const vars = getFramePopVariables(pauseData, ""); expect(vars[0].name).toEqual(""); expect(vars[0].name).toEqual(""); expect(vars[0].contents.value).toEqual({ type: "undefined" }); @@ -106,9 +106,6 @@ describe("scopes", () => { it("single scope", () => { const pauseData = { frame: { - scope: { - actor: "actor1" - }, this: {} } }; @@ -138,9 +135,6 @@ describe("scopes", () => { it("second scope", () => { const pauseData = { frame: { - scope: { - actor: "actor1" - }, this: {} } }; @@ -175,5 +169,103 @@ describe("scopes", () => { contents: {} }); }); + + it("returning scope", () => { + const pauseData = { + frame: { + this: {} + }, + why: { + frameFinished: { + return: "to sender" + } + } + }; + + const selectedFrame = { + scope: { + actor: "actor1", + type: "block", + bindings: { + arguments: [], + variables: {} + }, + parent: null + }, + this: {} + }; + + const scopes = getScopes(pauseData, selectedFrame); + expect(scopes).toMatchObject([ + { + path: "actor1-1", + contents: [ + { + name: "", + path: "actor1-1/", + contents: { + value: "to sender" + } + }, + { + name: "", + path: "actor1-1/", + contents: { + value: {} + } + } + ] + } + ]); + }); + + it("throwing scope", () => { + const pauseData = { + frame: { + this: {} + }, + why: { + frameFinished: { + throw: "a party" + } + } + }; + + const selectedFrame = { + scope: { + actor: "actor1", + type: "block", + bindings: { + arguments: [], + variables: {} + }, + parent: null + }, + this: {} + }; + + const scopes = getScopes(pauseData, selectedFrame); + expect(scopes).toMatchObject([ + { + path: "actor1-1", + contents: [ + { + name: "", + path: "actor1-1/", + contents: { + value: "a party" + } + }, + { + name: "", + path: "actor1-1/", + contents: { + value: {} + } + } + ] + } + ]); + }); }); });