Bug 143225 - Web Inspector: debugger pause reason not updated when paused at breakpoint after resuming from debugger statement
Summary: Web Inspector: debugger pause reason not updated when paused at breakpoint af...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-03-30 09:29 PDT by Brian Burg
Modified: 2016-12-13 15:39 PST (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Brian Burg 2015-03-30 09:29:02 PDT
Steps to reproduce:

1. Enable "pause on exceptions"
2. Cause an exception to be thrown
3. Press "step-over"/"step-out" several times

The pause reason is still listed as "Exception with thrown value: ...". However, the message may no longer relevant given that we may have walked to a different call frame.

So, maybe we should add a new pause reason ("Manual Stepping" or similar), or make it possible to click on the initial pause reason message to reveal a marker at the initial pause source location.
Comment 1 Radar WebKit Bug Importer 2015-03-30 09:29:23 PDT
<rdar://problem/20347033>
Comment 2 Joseph Pecoraro 2015-03-30 15:08:23 PDT
I deliberately wanted the current behavior. When stepping, I want to know the original pause reason. For a few reasons:

  - if you step around a bit, you will always be able to jump back to the original pause location
  - if the original pause was on a breakpoint, you can edit/disable that breakpoint right from the pause reason section no matter where you are in source code

There does exist a small problem in the implementation right now though. If you pause on a breakpoint, hit continue, and quickly pause on another breakpoint, it won't update to the new breakpoint. That is because of the way our Paused / Resumed events work in the frontend. So there is room to improve this here. That said, we could probably work around this in the frontend if the user definitely did a "continue" and paused soon after.
Comment 3 Brian Burg 2015-03-30 15:12:04 PDT
(In reply to comment #2)
> I deliberately wanted the current behavior. When stepping, I want to know
> the original pause reason. For a few reasons:
> 
>   - if you step around a bit, you will always be able to jump back to the
> original pause location

It's weird that there's a jump button but the message can't be clicked on. Showing an inline marker (like for errors) would be more consistent, as would linkifying the error message.

>   - if the original pause was on a breakpoint, you can edit/disable that
> breakpoint right from the pause reason section no matter where you are in
> source code

I think the current behavior is good in this respect.

> There does exist a small problem in the implementation right now though. If
> you pause on a breakpoint, hit continue, and quickly pause on another
> breakpoint, it won't update to the new breakpoint. That is because of the
> way our Paused / Resumed events work in the frontend. So there is room to
> improve this here. That said, we could probably work around this in the
> frontend if the user definitely did a "continue" and paused soon after.

Maybe keep a timeout flag in DebuggerManager and reset it if 'continue' is requested from UI.

PS. Go back on vacation! :-)
Comment 4 Brian Burg 2016-06-05 15:36:42 PDT
I hit this again yesterday. It's really awkward if you continue from a debugger statement in one file, then you hit a breakpoint deep in some other code and the Pause Reason still says "Debugger Statement".

I think the current behavior w.r.t. what should be shown after stepping is fine, and I disagree with my initial bug report here. However, the behavior when hitting a breakpoint after resuming from another breakpoint/debugger statement/immediate pause is jarring and hard to argue in favor of, IMO. We probably need to distinguish these different types of pauses in the frontend, since IIRC it counts everything as being part of the same "paused session" if the debugger re-pauses within 50ms or some other arbitrary time limit.

New STEPS TO REPRODUCE:

1. Inspect the attached test case, enable breakpoints
2. Set a breakpoint on `window.dispatchEvent(e)`
3. Reload the page
4. Hit continue when the debugger pauses at the `debugger` statement

EXPECTED:

Pause Reason is updated to cite the breakpoint from step (2)

ACTUAL:

Pause Reason still cites the debugger statement
Comment 5 Brian Burg 2016-06-05 15:39:29 PDT
(In reply to comment #4)
> We probably need to distinguish these different types of pauses in the
> frontend, since IIRC it counts everything as being part of the same "paused
> session" if the debugger re-pauses within 50ms or some other arbitrary time
> limit.

I misspoke, we need to save the last debugger action (resume / step over / step in / step out). Right now we start a timer for any of these things since the backend only tells us that we paused at a line. If the last action was resume, then we should reset the pause reason.