RESOLVED DUPLICATE of bug 155325 161721
Web Inspector: Stepping all the way through program should not cause a pause on the next program that executes
https://bugs.webkit.org/show_bug.cgi?id=161721
Summary Web Inspector: Stepping all the way through program should not cause a pause ...
Joseph Pecoraro
Reported Thursday, September 8, 2016 1:23:11 AM UTC
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
Attachments
[PATCH] Proposed Fix (14.33 KB, patch)
2016-09-07 19:07 PDT, Joseph Pecoraro
no flags
[PATCH] Proposed Fix (14.98 KB, patch)
2016-09-07 19:21 PDT, Joseph Pecoraro
mattbaker: review+
joepeck: commit-queue-
Radar WebKit Bug Importer
Comment 1 Thursday, September 8, 2016 1:24:00 AM UTC
Joseph Pecoraro
Comment 2 Thursday, September 8, 2016 2:51:17 AM UTC
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
Comment 3 Thursday, September 8, 2016 3:07:25 AM UTC
Created attachment 288224 [details] [PATCH] Proposed Fix
Joseph Pecoraro
Comment 4 Thursday, September 8, 2016 3:21:55 AM UTC
Created attachment 288225 [details] [PATCH] Proposed Fix
Joseph Pecoraro
Comment 5 Thursday, September 8, 2016 5:23:08 AM UTC
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 6 Thursday, September 8, 2016 7:48:47 PM UTC
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
Comment 7 Thursday, September 8, 2016 7:59:19 PM UTC
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
Comment 8 Friday, September 30, 2016 8:36:40 PM UTC
Fixed as part of bug 155325: https://trac.webkit.org/changeset/206652 *** This bug has been marked as a duplicate of bug 155325 ***
Note You need to log in before you can comment on or make changes to this bug.