Bug 161721 – Web Inspector: Stepping all the way through program should not cause a pause on the next program that executes

• Component: Web Inspector
• Status: RESOLVED DUPLICATE

[Joseph Pecoraro]

Summary:
Stepping all the way through program should not cause a pause on the next program that executes 

Test:
1. <script>
2. setTimeout(() => {
3.     console.log("here");
4. }, 3000);
5. debugger;
6. </script>

Steps to Reproduce:
1. Inspect test page
2. Reload
  => Pause on debugger statement
3. Step in until the program completes
4. Wait a few seconds
  => Unexpected pause on timer firing, or other code executing in the page

Notes:
- Stepping through a program should not affect the next program that runs
- Chrome does pause at the next program execution
- Firefox does not pause at the next program execution

[Radar WebKit Bug Importer]

<rdar://problem/28198547>

[Joseph Pecoraro]

One possible location where this would be useful is stepping across microtasks:

For example, "step-into" a line with setTimeout would eventually pause when the timeout fires:

    debugger;
 >  setTimeout(() => {
       alert("here");
    });
    ...

Or a Promise constructor to step into the function:

    debugger;
 >  new Promise((resolve, reject) => {
       alert("here");
    });
    ...

This would work right now with current step-into behavior if you step-into all the way out of the first program.

But if there is anything in the "..." location (or parent call frames), then this is super bad to rely on "step-into" to trigger this. I'd rather this be a separate command, "step-follow" or "step-through" which recognizes that there is a callback and pauses inside it.

I'm going to drop this behavior from step-into. It is too arbitrary as is.

[Joseph Pecoraro]

Created attachment 288224 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Created attachment 288225 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Comment on attachment 288225 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=288225&action=review

> LayoutTests/inspector/debugger/stepInto.html:54
> +    // Always step-in when call frames change.

This comment should be "step-into".

> LayoutTests/inspector/debugger/stepOver.html:54
> +    // Always step-in when call frames change.

This comment should be "step-over".

[Matt Baker]

Comment on attachment 288225 [details]
[PATCH] Proposed Fix

r=me.

Maybe there should be a change log comment making it more clear that the change to Debugger.cpp alone isn't enough to fix the issue. On first read it wasn't clear that InspectorDOMDebuggerAgent::stepInfo was causing the next call to pauseOnNativeEventIfNeeded to break in the debugger.

[Blaze Burg]

View in context: https://bugs.webkit.org/attachment.cgi?id=288225&action=review

r=me too

> LayoutTests/inspector/debugger/stepInto.html:19
> +    testAlert("log 1");

Please update this to "log from inside entry2" so that the expected output is more comprehensible. I thought it was code accidentally left in from debugging.

> LayoutTests/inspector/debugger/stepOver-expected.txt:6
> +-- Running test case: Debugger.stepOver.statements

This test is very nice.

[Joseph Pecoraro]

Fixed as part of bug 155325:
https://trac.webkit.org/changeset/206652

*** This bug has been marked as a duplicate of bug 155325 ***