From 33f0851783d5a775ddf5cec48f7b50378b3f1387 Mon Sep 17 00:00:00 2001 From: Brian Hackett Date: Thu, 19 Sep 2019 21:18:57 +0000 Subject: [PATCH] Bug 1582557 - Fix some stepping issues, r=jlast. Differential Revision: https://phabricator.services.mozilla.com/D46518 --HG-- extra : moz-landing-system : lando --- .eslintignore | 1 + .../client/webreplay/mochitest/browser.ini | 1 + .../mochitest/browser_dbg_rr_logpoint-03.js | 2 - .../mochitest/browser_dbg_rr_stepping-05.js | 31 ++++++++++++ .../mochitest/examples/doc_minified.html | 13 +++++ .../webreplay/mochitest/examples/minified.js | 1 + devtools/client/webreplay/mochitest/head.js | 7 ++- devtools/server/actors/replay/control.js | 20 ++++++-- devtools/server/actors/replay/debugger.js | 3 -- devtools/server/actors/replay/replay.js | 48 +++++++++++-------- .../actors/replay/utils/findStepOffsets.js | 4 +- devtools/server/actors/source.js | 1 - devtools/server/actors/thread.js | 6 ++- 13 files changed, 104 insertions(+), 34 deletions(-) create mode 100644 devtools/client/webreplay/mochitest/browser_dbg_rr_stepping-05.js create mode 100644 devtools/client/webreplay/mochitest/examples/doc_minified.html create mode 100644 devtools/client/webreplay/mochitest/examples/minified.js diff --git a/.eslintignore b/.eslintignore index f474d9559b50..1d9927b4a453 100644 --- a/.eslintignore +++ b/.eslintignore @@ -90,6 +90,7 @@ devtools/client/debugger/bin/ devtools/client/debugger/packages/**/fixtures/ devtools/client/debugger/node_modules devtools/client/debugger/out +devtools/client/webreplay/mochitest/examples/ # Ignore devtools debugger files which aren't intended for linting, and also # aren't included in any .eslintignore or .prettierignore file. diff --git a/devtools/client/webreplay/mochitest/browser.ini b/devtools/client/webreplay/mochitest/browser.ini index 47bc4d0f0e20..35c37abf70e8 100644 --- a/devtools/client/webreplay/mochitest/browser.ini +++ b/devtools/client/webreplay/mochitest/browser.ini @@ -28,6 +28,7 @@ support-files = [browser_dbg_rr_stepping-02.js] [browser_dbg_rr_stepping-03.js] [browser_dbg_rr_stepping-04.js] +[browser_dbg_rr_stepping-05.js] [browser_dbg_rr_replay-01.js] [browser_dbg_rr_replay-02.js] [browser_dbg_rr_replay-03.js] diff --git a/devtools/client/webreplay/mochitest/browser_dbg_rr_logpoint-03.js b/devtools/client/webreplay/mochitest/browser_dbg_rr_logpoint-03.js index 170d18b52d65..8cf807152614 100644 --- a/devtools/client/webreplay/mochitest/browser_dbg_rr_logpoint-03.js +++ b/devtools/client/webreplay/mochitest/browser_dbg_rr_logpoint-03.js @@ -6,8 +6,6 @@ // Test event logpoints when replaying. add_task(async function() { - await pushPref("devtools.debugger.features.log-event-breakpoints", true); - const dbg = await attachRecordingDebugger("doc_events.html", { waitForRecording: true, }); diff --git a/devtools/client/webreplay/mochitest/browser_dbg_rr_stepping-05.js b/devtools/client/webreplay/mochitest/browser_dbg_rr_stepping-05.js new file mode 100644 index 000000000000..b026c1a1c00a --- /dev/null +++ b/devtools/client/webreplay/mochitest/browser_dbg_rr_stepping-05.js @@ -0,0 +1,31 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ +/* eslint-disable no-undef */ + +"use strict"; + +// Test stepping in pretty-printed code. +add_task(async function() { + const dbg = await attachRecordingDebugger("doc_minified.html", { + waitForRecording: true, + }); + + await selectSource(dbg, "minified.js"); + await prettyPrint(dbg); + + await dbg.actions.addEventListenerBreakpoints(["event.mouse.click"]); + await dbg.actions.toggleEventLogging(); + + const console = await getDebuggerSplitConsole(dbg); + const hud = console.hud; + + await warpToMessage(hud, dbg, "click", 12); + await stepInToLine(dbg, 2); + await stepOutToLine(dbg, 12); + await stepInToLine(dbg, 9); + await stepOutToLine(dbg, 13); + await stepInToLine(dbg, 5); + await stepOutToLine(dbg, 14); + + await shutdownDebugger(dbg); +}); diff --git a/devtools/client/webreplay/mochitest/examples/doc_minified.html b/devtools/client/webreplay/mochitest/examples/doc_minified.html new file mode 100644 index 000000000000..8a1e6df32183 --- /dev/null +++ b/devtools/client/webreplay/mochitest/examples/doc_minified.html @@ -0,0 +1,13 @@ +
Hello World!
+ + + diff --git a/devtools/client/webreplay/mochitest/examples/minified.js b/devtools/client/webreplay/mochitest/examples/minified.js new file mode 100644 index 000000000000..7a1a930f7e0a --- /dev/null +++ b/devtools/client/webreplay/mochitest/examples/minified.js @@ -0,0 +1 @@ +const s={getWindow:()=>window};function f(){this.getElementById("divvy").innerHTML="Done!"}const nf=f.bind(document);function DOMEvent(n,e){console.log("DOMEvent",e)}function h(n){n=new DOMEvent(n,s.getWindow()),!1===nf.call(s,n)}document.getElementById("divvy").addEventListener("click",h); diff --git a/devtools/client/webreplay/mochitest/head.js b/devtools/client/webreplay/mochitest/head.js index 54a0ac20b2e2..2fb01710db8e 100644 --- a/devtools/client/webreplay/mochitest/head.js +++ b/devtools/client/webreplay/mochitest/head.js @@ -148,7 +148,7 @@ async function waitForMessageCount(hud, text, length, selector = ".message") { return messages; } -async function warpToMessage(hud, dbg, text) { +async function warpToMessage(hud, dbg, text, maybeLine) { let messages = await waitForMessages(hud, text); ok(messages.length == 1, "Found one message"); const message = messages.pop(); @@ -168,6 +168,11 @@ async function warpToMessage(hud, dbg, text) { messages = findMessages(hud, "", ".paused"); ok(messages.length == 1, "Found one paused message"); + if (maybeLine) { + const pauseLine = getVisibleSelectedFrameLine(dbg); + ok(pauseLine == maybeLine, `Paused at line ${maybeLine} after warp`); + } + return message; async function openConsoleContextMenu(element) { diff --git a/devtools/server/actors/replay/control.js b/devtools/server/actors/replay/control.js index 9925fbbba03a..a231ab94c965 100644 --- a/devtools/server/actors/replay/control.js +++ b/devtools/server/actors/replay/control.js @@ -219,6 +219,9 @@ ChildProcess.prototype = { if (response.memoryUsage) { this.lastMemoryUsage = response.memoryUsage; } + if (response.exception) { + ThrowError(response.exception); + } } this.paused = true; this.manifest.onFinished(response); @@ -1546,7 +1549,16 @@ async function findEventFrameEntry(checkpoint, progress) { scanCheckpoint: savedCheckpoint, }); - return gEventFrameEntryPoints.get(progress); + const enterFramePoint = gEventFrameEntryPoints.get(progress); + if (!enterFramePoint) { + return null; + } + + // We want to stop at the first step in the frame, not at the EnterFrame. + const frameSteps = await findFrameSteps(enterFramePoint); + assert(pointEquals(frameSteps[0], enterFramePoint)); + + return frameSteps[1]; } async function findEventLogpointHits(checkpoint, event, callback) { @@ -1687,12 +1699,11 @@ let gLastFlushTime = Date.now(); // If necessary, synchronously flush the recording to disk. function ensureFlushed() { - assert(gActiveChild == gMainChild); gMainChild.waitUntilPaused(true); gLastFlushTime = Date.now(); - if (gLastFlushCheckpoint == gActiveChild.pauseCheckpoint()) { + if (gLastFlushCheckpoint == gMainChild.pauseCheckpoint()) { return; } @@ -1884,6 +1895,7 @@ const gControl = { // Add a breakpoint where the active child should pause while resuming. addBreakpoint(position) { + dumpv(`AddBreakpoint ${JSON.stringify(position)}`); gBreakpoints.push(position); // Start searching for breakpoint hits in the recording immediately. @@ -1904,7 +1916,9 @@ const gControl = { // Clear all installed breakpoints. clearBreakpoints() { + dumpv(`ClearBreakpoints\n`); gBreakpoints.length = 0; + if (gActiveChild == gMainChild) { // As for addBreakpoint(), update the active breakpoints in the recording // child immediately. diff --git a/devtools/server/actors/replay/debugger.js b/devtools/server/actors/replay/debugger.js index 9b098fd88c27..85ef85895575 100644 --- a/devtools/server/actors/replay/debugger.js +++ b/devtools/server/actors/replay/debugger.js @@ -268,9 +268,6 @@ ReplayDebugger.prototype = { _processResponse(request, response, divergeResponse) { dumpv(`SendRequest: ${stringify(request)} -> ${stringify(response)}`); - if (response.exception) { - ThrowError(response.exception); - } if (response.unhandledDivergence) { if (divergeResponse) { return divergeResponse; diff --git a/devtools/server/actors/replay/replay.js b/devtools/server/actors/replay/replay.js index 609e6dc06cd4..abca36039e6f 100644 --- a/devtools/server/actors/replay/replay.js +++ b/devtools/server/actors/replay/replay.js @@ -1054,7 +1054,10 @@ function ManifestStart(manifest) { dump(`Unknown manifest: ${JSON.stringify(manifest)}\n`); } } catch (e) { - printError("ManifestStart", e); + const msg = printError("ManifestStart", e); + RecordReplayControl.manifestFinished({ + exception: `ManifestStart failed: ${msg}`, + }); } } @@ -1184,7 +1187,10 @@ function HitCheckpoint(id) { try { processManifestAfterCheckpoint(point); } catch (e) { - printError("AfterCheckpoint", e); + const msg = printError("AfterCheckpoint", e); + RecordReplayControl.manifestFinished({ + exception: `AfterCheckpoint failed: ${msg}`, + }); } } @@ -1356,17 +1362,21 @@ function getObjectData(id) { rv.proxyTarget = convertValue(object.proxyTarget); rv.proxyHandler = convertValue(object.proxyHandler); } - if (object.errorMessageName) { - rv.errorMessageName = object.errorMessageName; - } - if (object.errorNotes) { - rv.errorNotes = object.errorNotes; - } - if (object.errorLineNumber) { - rv.errorLineNumber = object.errorLineNumber; - } - if (object.errorColumnNumber) { - rv.errorColumnNumber = object.errorColumnNumber; + try { + if (object.errorMessageName) { + rv.errorMessageName = object.errorMessageName; + } + if (object.errorNotes) { + rv.errorNotes = object.errorNotes; + } + if (object.errorLineNumber) { + rv.errorLineNumber = object.errorLineNumber; + } + if (object.errorColumnNumber) { + rv.errorColumnNumber = object.errorColumnNumber; + } + } catch (e) { + // Error getters can throw access denied errors. } if (CSSRule.isInstance(object.unsafeDereference())) { rv.isInstance = "CSSRule"; @@ -2048,15 +2058,10 @@ const gRequestHandlers = { }; function processRequest(request) { - try { - if (gRequestHandlers[request.type]) { - return gRequestHandlers[request.type](request); - } - return { exception: "No handler for " + request.type }; - } catch (e) { - printError("processRequest", e); - return { exception: `Request failed: ${request.type}` }; + if (gRequestHandlers[request.type]) { + return gRequestHandlers[request.type](request); } + throwError(`"No handler for ${request.type}`); } function printError(why, e) { @@ -2067,6 +2072,7 @@ function printError(why, e) { msg = "Unknown"; } dump(`Record/Replay Error: ${why}: ${msg}\n`); + return msg; } // eslint-disable-next-line no-unused-vars diff --git a/devtools/server/actors/replay/utils/findStepOffsets.js b/devtools/server/actors/replay/utils/findStepOffsets.js index 75d5749c2797..fe819e042a29 100644 --- a/devtools/server/actors/replay/utils/findStepOffsets.js +++ b/devtools/server/actors/replay/utils/findStepOffsets.js @@ -16,7 +16,7 @@ function getNeighbors(frame, offset, rewinding) { * return an array of all the step targets * that could be reached next from startLocation. */ -function findStepOffsets(frame, rewinding) { +function findStepOffsets(frame, rewinding, requireStepStart = true) { const seen = []; const result = []; let worklist = getNeighbors(frame, frame.offset, rewinding); @@ -28,7 +28,7 @@ function findStepOffsets(frame, rewinding) { } seen.push(offset); const meta = frame.script.getOffsetMetadata(offset); - if (meta.isStepStart) { + if (requireStepStart ? meta.isStepStart : meta.isBreakpoint) { if (!result.includes(offset)) { result.push(offset); } diff --git a/devtools/server/actors/source.js b/devtools/server/actors/source.js index 87da9a5f5de1..c34832175a29 100644 --- a/devtools/server/actors/source.js +++ b/devtools/server/actors/source.js @@ -223,7 +223,6 @@ const SourceActor = ActorClassWithSpec(sourceSpec, { // original recording. If we try to fetch it now it may have changed or // may no longer exist. if (this.dbg.replaying) { - assert(!this._contentType); return this.dbg.replayingContent(this.url); } diff --git a/devtools/server/actors/thread.js b/devtools/server/actors/thread.js index fb99fb6267ba..735b9bfeffec 100644 --- a/devtools/server/actors/thread.js +++ b/devtools/server/actors/thread.js @@ -897,7 +897,11 @@ const ThreadActor = ActorClassWithSpec(threadSpec, { }); if (thread.dbg.replaying) { - const offsets = findStepOffsets(parentFrame, rewinding); + const offsets = findStepOffsets( + parentFrame, + rewinding, + /* requireStepStart */ false + ); parentFrame.setReplayingOnStep(onStep, offsets); } else { parentFrame.onStep = onStep;