Pause Points went to nightly 3/25/2017. Since then, we updated some of the pause point locations [1] and tightened the mochitests [2]. [1]: https://github.com/devtools-html/debugger.html/pull/5777 [2]: https://github.com/devtools-html/debugger.html/pull/5808 It'd be nice to revisit a couple early assumptions: 1. pause points should be a list 2. we should resume if a pause point is not found --- ### Pause points should be a list The first version had the type signature: > type PausePoint = { > location: { line: number, column: number } > types: { breakpoint: boolean, stepOver: boolean } > } > type PausePoints = PausePoint[]; The new version will have the signature: > type PausePoints = { > line: { > column: { break: boolean, step: boolean } > } > } The new type has some advantages: 1. it is more compact 2. step does not imply step over, it could be in, out... 3. it is easier to find a pause point at a line,colum 4. It is easier to check if a line has points ### We should resume if a pause point is not found The first version assumed that if a location did not have a pause point, we did not want to stop there. This approach biased towards fewer steps. I think going forward, we should go in the other direction and bias toward pausing if we're not sure.

Comment on attachment 8964002 [details] Bug 1450323 - Debugger: Update Pause Points. https://reviewboard.mozilla.org/r/232826/#review238532 ::: devtools/client/debugger/new/parser-worker.js:21134 (Diff revision 3) > * 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/>. */ > > -const isControlFlow = node => t.isForStatement(node) || t.isWhileStatement(node) || t.isIfStatement(node) || t.isSwitchCase(node) || t.isSwitchStatement(node); > > -const isAssignment = node => t.isVariableDeclarator(node) || t.isAssignmentExpression(node); > +const isControlFlow = node => This work will be separately reviewed here: https://github.com/devtools-html/debugger.html/pull/5836 the GH pr has proper unit tests. The rationale for the changes here are: 1. the change to the data structure 2. the advent of empty pause points which note places we do *not* want to pause at

Comment on attachment 8964002 [details] Bug 1450323 - Debugger: Update Pause Points. https://reviewboard.mozilla.org/r/232826/#review240748 The server side of this looks good. I'll look at the client side on GitHub.

Comment on attachment 8964002 [details] Bug 1450323 - Debugger: Update Pause Points. https://reviewboard.mozilla.org/r/232826/#review241104 I approved the client-side changes on GitHub. Looks good.

Pushed by jlaster@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/2219f409ada9 Debugger: Update Pause Points (v37). r=jimb

I'm treating this commit as release 37 because it included the client code for pause points which might warrant a beta fix https://github.com/devtools-html/debugger.html/compare/release-36...release-37

talos https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=2cfc7ec29d0d6bb0ffcf051535d138708a1be651&newProject=try&newRevision=25a7f08c033bad4bf45b959f0564bcb48009c873&framework=1

talos job name changed: new link https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=2e0f67481b87ad46bfd74f4720cc7c0924664cee&newProject=try&newRevision=66bb535ca7f62a918842b90e1c813d55c4f460de&framework=1