Bug 161716 - Web Inspector: Stepping into a function / program should not require stepping to the first statement
Summary: Web Inspector: Stepping into a function / program should not require stepping...
Status: RESOLVED DUPLICATE of bug 155325
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-09-07 15:52 PDT by Joseph Pecoraro
Modified: 2016-09-30 12:36 PDT (History)
9 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (6.65 KB, patch)
2016-09-07 21:14 PDT, Joseph Pecoraro
bburg: review+
joepeck: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2016-09-07 15:52:16 PDT
Summary:
Stepping into a function should not require stepping to the first statement

Test:
1. <script>
2. function foo() {
3.     console.log("foo");
4. }
5. foo();
6. </script>

Steps to Reproduce:
1. Inspect test page
2. Set a breakpoint on line 5
3. Reload
  => Hits breakpoint, pauses on line 5
4. Step-in
  => Pauses on line 2 for the function definition, you have to step to get to line 3
  => Expected to pause on line 3 which is the first statement in foo

Notes:
- lldb pause on line 3 before the first statement in foo
- Chrome pauses on line 3 before the first statement in foo
- Firefox pauses on line 3 before the first statement in foo
- I think this is us pausing at Debugger::callEvent. Seems we can just not pause there. I'm not sure if there is any reason to pause there.
Comment 1 Radar WebKit Bug Importer 2016-09-07 15:52:33 PDT
<rdar://problem/28196590>
Comment 2 Joseph Pecoraro 2016-09-07 19:09:53 PDT
This is the same for stepping into a program.

Test:
1. <script>
2. var before = 1;
3. eval("1 + 1");
4. var after = 1;
5. </script>

There are 3 pauses on line 3 because when we step-into we pause at 3 locations:

    PausedAtStartOfProgram
    PausedAtEndOfProgram
    PausedAtStatement
Comment 3 Joseph Pecoraro 2016-09-07 20:57:47 PDT
After a long discussion with coworkers I think we all agree that there is very little value in pausing at the start of a program. Since there is nothing you can do that is different from pausing at the first statement within it. However there is value in pausing at the end of a program / end of a function. Since that is the last opportunity  to view the state of things within the function (the value of the last statement for instance).
Comment 4 Joseph Pecoraro 2016-09-07 21:14:58 PDT
Created attachment 288240 [details]
[PATCH] Proposed Fix
Comment 5 Joseph Pecoraro 2016-09-07 21:15:16 PDT
Comment on attachment 288240 [details]
[PATCH] Proposed Fix

cq- this depends on an earlier patch so it won't apply yet.
Comment 6 BJ Burg 2016-09-08 12:12:27 PDT
(In reply to comment #2)
> This is the same for stepping into a program.
> 
> Test:
> 1. <script>
> 2. var before = 1;
> 3. eval("1 + 1");
> 4. var after = 1;
> 5. </script>
> 
> There are 3 pauses on line 3 because when we step-into we pause at 3
> locations:
> 
>     PausedAtStartOfProgram
>     PausedAtEndOfProgram
>     PausedAtStatement

>:(
Comment 7 BJ Burg 2016-09-08 12:16:19 PDT
Comment on attachment 288240 [details]
[PATCH] Proposed Fix

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

> LayoutTests/inspector/debugger/stepInto-expected.txt:-19
> -PAUSE AT testAlert:6:19

Nice nice.

> Source/JavaScriptCore/debugger/Debugger.h:-95
> -        PausedAfterCall,

Is this completely redundant with PauseBeforeReturn? It doesn't seem like its removal caused any fewer pauses to happen in the test expectations.
Comment 8 BJ Burg 2016-09-08 12:21:33 PDT
Comment on attachment 288240 [details]
[PATCH] Proposed Fix

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

r=me

>> Source/JavaScriptCore/debugger/Debugger.h:-95
>> -        PausedAfterCall,
> 
> Is this completely redundant with PauseBeforeReturn? It doesn't seem like its removal caused any fewer pauses to happen in the test expectations.

EDIT: this is for pausing when calling a function but before running the first statement. It's redundant with PausedAtStatement for the first statement.
Comment 9 Joseph Pecoraro 2016-09-12 12:07:04 PDT
I rewrote everything here in an effort to fix other debugger stepping issues. I'm going to obsolete this patch. And likely duplicate this bug to a more general bug that addresses the larger stepping issues.
Comment 10 Joseph Pecoraro 2016-09-30 12:36:57 PDT
Fixed as part of bug 155325:
https://trac.webkit.org/changeset/206652

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