815537 - Nested pauses triggered by watch expressions and conditional breakpoints are broken

Status: RESOLVED WONTFIX

(DevTools :: Debugger, defect, P2)

Description

[Panos Astithas (he/him) [:past] (please ni?)]

STR:

1) Got to http://htmlpad.org/debugger
2) 'Click me'
3) Add breakpoints at lines 11 and 35
4) Resume
5) Add the watch expression: a == test()
6) The following error is displayed in the console:

DBG-CLIENT: TypeError: this.currentEvaluation is null: SF__onFrames@chrome://browser/content/debugger-controller.js:538
EV_notify@resource://gre/modules/devtools/dbg-client.jsm:145
TC_fillFrames/<@resource://gre/modules/devtools/dbg-client.jsm:946
DC_onPacket@resource://gre/modules/devtools/dbg-client.jsm:493
@chrome://global/content/devtools/dbg-transport.js:212

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

Maybe the safest thing to do is to ignore breakpoint hits in the server when a client eval frame is on the stack.

Comment 2

[Victor Porof [:vporof][:vp]]

(In reply to Panos Astithas [:past] from comment #1)
> Maybe the safest thing to do is to ignore breakpoint hits in the server when
> a client eval frame is on the stack.

This is definitely the right thing to do, if you don't want to end up in a potentially infinitely nested stack.

Comment 3

[Panos Astithas (he/him) [:past] (please ni?)]

Filter on BLACKEAGLE2.

Comment 5

[Panos Astithas (he/him) [:past] (please ni?)]

As bug 908959 points out, we should be ignoring debugger statements as well. Also, presumably other forms of execution pause, like pause-on-exceptions, pause-on-DOM-events, etc.

Comment 6

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Should we consider bumping up priority on this? Results in a completely broken debugger.

Comment 7

[Rob Campbell [:rc] (:robcee)]

seems prudent, thanks for the bump.

(In reply to Panos Astithas [:past] from comment #5)
> As bug 908959 points out, we should be ignoring debugger statements as well.
> Also, presumably other forms of execution pause, like pause-on-exceptions,
> pause-on-DOM-events, etc.

What about in the case when we're evaling something from the console or scratchpad in a Debugger frame?

see bug 813087.

Comment 8

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Rob Campbell [:rc] (:robcee) from comment #7)
> What about in the case when we're evaling something from the console or
> scratchpad in a Debugger frame?

I think we should be able to differentiate between debugger watch expressions and console/scratchpad evals, because they use different packet handlers in different actors. We may have to inspect the stack at the breakpoint hook, I haven't looked closely yet.

Comment 10

[Panos Astithas (he/him) [:past] (please ni?)]

Not going to work on this in the next couple of weeks at least.

Comment 11

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Note that "while (1) ;" and then stopping the script via the slow script dialog also results in sad. Same with conditional breakpoints.

Adding "debugger" inside a conditional breakpoint pauses twice: once presumably for the debugger statement (no environment is displayed at all) and then if you resume you immediately pause again, this time at the breakpoint's location (with environment and all that).

Comment 12

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Jim has ideas on UI in this comment: https://bugzilla.mozilla.org/show_bug.cgi?id=812172#c8

> Ideally, all of these would cause execution to pause, with a note explaining
> what's going on, a clear mark on the stack for the debugger's eval, and a
> button to disable the breakpoint, or remove the condition.

Comment 13

[Eddy Bruel [:ejpbruel]]

Is this about us not handling nested pauses correctly in the debugger?

http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/script.js#1804

Comment 14

[Jason Laster [:jlast]]

I don't think we're currently trying to support nested pauses so i'm going to close this.