Skip to content

Commit

Permalink
fix: restart frame not working
Browse files Browse the repository at this point in the history
Previously the paused state wasn't set and we weren't in the function
after hitting "restart frame". Both the previous adapter[1] and the
Chrome devtools themselves[2] just discard the response of restart frame
and immediately stepInto(). Doing the same here appears to work well
and fixes #142.

 1. https://github.com/microsoft/vscode-chrome-debug-core/blob/d4bd01568b2bff1109bb75d80d487b5fbbc8427e/src/chrome/chromeDebugAdapter.ts#L1603-L1605
 2. https://github.com/ChromeDevTools/devtools-frontend/blob/c96ccd99f0e299d9cce23a8667eb1d35e057f7e0/front_end/sdk/DebuggerModel.js#L1382-L1385
  • Loading branch information
connor4312 authored Dec 5, 2019
1 parent 0a41897 commit fa92b16
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 16 deletions.
33 changes: 19 additions & 14 deletions src/adapter/threads.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export class Thread implements VariableStoreDelegate {
private _scriptSources = new Map<string, Map<string, Source>>();
private _sourceMapLoads = new Map<string, Promise<{ remainPaused: boolean }>>();
private _smartStepper: SmartStepper;
private _expectedPauseReason: PausedReason | undefined;
private _expectedPauseReason?: { reason: PausedReason, description?: string };

constructor(
sourceContainer: SourceContainer,
Expand Down Expand Up @@ -161,23 +161,23 @@ export class Thread implements VariableStoreDelegate {

async pause(): Promise<Dap.PauseResult | Dap.Error> {
if (await this._cdp.Debugger.pause({}))
this._expectedPauseReason = 'pause';
this._expectedPauseReason = { reason: 'pause' };
else
return errors.createSilentError(localize('error.pauseDidFail', 'Unable to pause'));
return {};
}

async stepOver(): Promise<Dap.NextResult | Dap.Error> {
if (await this._cdp.Debugger.stepOver({}))
this._expectedPauseReason = 'step';
this._expectedPauseReason = { reason: 'step' };
else
return errors.createSilentError(localize('error.stepOverDidFail', 'Unable to step next'));
return {};
}

async stepInto(): Promise<Dap.StepInResult | Dap.Error> {
if (await this._cdp.Debugger.stepInto({breakOnAsyncCall: true}))
this._expectedPauseReason = 'step';
this._expectedPauseReason = { reason: 'step' };
else
return errors.createSilentError(localize('error.stepInDidFail', 'Unable to step in'));
return {};
Expand All @@ -198,17 +198,22 @@ export class Thread implements VariableStoreDelegate {
}

async restartFrame(params: Dap.RestartFrameParams): Promise<Dap.RestartFrameResult | Dap.Error> {
const stackFrame = this._pausedDetails ? this._pausedDetails.stackTrace.frame(params.frameId) : undefined;
if (!stackFrame)
const stackFrame = this._pausedDetails?.stackTrace.frame(params.frameId);
if (!stackFrame) {
return this._stackFrameNotFoundError();
}

const callFrameId = stackFrame.callFrameId();
if (!callFrameId)
if (!callFrameId) {
return errors.createUserError(localize('error.restartFrameAsync', 'Cannot restart asynchronous frame'));
const response = await this._cdp.Debugger.restartFrame({ callFrameId });
if (response && this._pausedDetails)
this._pausedDetails.stackTrace = this.launchConfig.showAsyncStacks
? StackTrace.fromDebugger(this, response.callFrames, response.asyncStackTrace, response.asyncStackTraceId)
: StackTrace.fromDebugger(this, response.callFrames);
}

await this._cdp.Debugger.restartFrame({ callFrameId });
this._expectedPauseReason = {
reason: 'frame_entry' as PausedReason,
description: localize('reason.description.restart', 'Paused on frame entry'),
};
await this._cdp.Debugger.stepInto({});
return {};
}

Expand Down Expand Up @@ -738,8 +743,8 @@ export class Thread implements VariableStoreDelegate {
return {
thread: this,
stackTrace,
reason: this._expectedPauseReason,
description: localize('pause.default', 'Paused')
description: localize('pause.default', 'Paused'),
...this._expectedPauseReason,
};
}
return {
Expand Down
2 changes: 1 addition & 1 deletion src/build/generate-contributions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ const baseConfigurationAttributes: ConfigurationAttributes<IBaseConfiguration> =
skipFiles: {
type: 'array',
description: refString('chrome.skipFiles.description'),
default: [],
default: ["<node_internals>/**"],
},
smartStep: {
type: 'boolean',
Expand Down
16 changes: 16 additions & 0 deletions src/test/breakpoints/breakpoints-restart-frame.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
allThreadsStopped : false
description : Paused on breakpoint
reason : breakpoint
threadId : <number>
}
<anonymous> @ ${workspaceFolder}/web/restart.js:6:3
<anonymous> @ ${workspaceFolder}/web/restart.js:7:3
{
allThreadsStopped : false
description : Paused on frame entry
reason : frame_entry
threadId : <number>
}
<anonymous> @ ${workspaceFolder}/web/restart.js:4:3
<anonymous> @ ${workspaceFolder}/web/restart.js:7:3
17 changes: 16 additions & 1 deletion src/test/breakpoints/breakpointsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ describe('breakpoints', () => {
p.assertLog();
});

// See #109
// See #109
itIntegrates('source map set compiled', async ({ r }) => {
// Breakpoint in compiled script which has a source map should resolve
// to the compiled script.
Expand Down Expand Up @@ -329,4 +329,19 @@ describe('breakpoints', () => {
p.assertLog();
});
});

itIntegrates('restart frame', async ({ r }) => {
const p = await r.launchUrl('restart.html');
const source: Dap.Source = {
path: p.workspacePath('web/restart.js'),
};
await p.dap.setBreakpoints({ source, breakpoints: [{ line: 6, column: 0 }] });
p.load();
const { threadId } = p.log(await p.dap.once('stopped'));
const stack = await p.logger.logStackTrace(threadId);
p.dap.restartFrame({ frameId: stack[0].id });

await waitForPause(p);
p.assertLog();
});
});
2 changes: 2 additions & 0 deletions src/test/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ export class Logger {
}, {}, ' ');
}
}

return stack;
}

evaluateAndLog(expression: string, options?: LogOptions, context?: 'watch' | 'repl' | 'hover'): Promise<Dap.Variable>;
Expand Down
5 changes: 5 additions & 0 deletions testWorkspace/web/restart.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<head>
<script src='restart.js'></script>
</head>
<body>
</body>
8 changes: 8 additions & 0 deletions testWorkspace/web/restart.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
let a = 0;

(() => {
a++;
a++;
a++;
})();

0 comments on commit fa92b16

Please sign in to comment.