Closed Bug 876655 Opened 7 years ago Closed 7 months ago

Watch expressions for non-toplevel frames

Categories

(DevTools :: Debugger, enhancement, P3)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Gijs, Unassigned)

References

(Blocks 2 open bugs)

Details

STR:

1. Open the browser debugger
2. Add a breakpoint somewhere that's got at least 1 stack frame above it.
3. Hit the breakpoint
4. Move up in the callstack by using the 'breadcrumbs' at the top
5. Add a watch

ER:
The watch is added and I can see its result in the current function context

AR:
The watch is added and I am transported to the top of the call stack, where the expression likely doesn't make sense. On top of that, moving back down the call stack, my watch expression disappears.

This makes exception debugging hard because I cannot easily evaluate what happened "above me" in the callstack.
What if your watch expression has side effects? Would you expect these to propagate through scopes?
I think that if a watch expression has foot-shooting side-effects, it should be the user's responsibility to move the foot away from the gun. In Chrome, clicking on other stack frames causes watch expressions to re-evaluate, but side-effects don't seem to persist. I'm not sure if this behavior is something we can or want to provide though.
(In reply to Panos Astithas [:past] from comment #2)

Somewhat related, I think it'd be a good idea to provide a better way of evaluating watch expressions [0], but ideally fully relying the server-side. Currently, whenever frames are received, the expressions are then evaluated in the topmost frame, blowing away the stack and receiving a new set of frames. This causes two roundtrips to the server, which doesn't seem like a good idea to me.

Similar to pause-on-exceptions and other persistent state on the server set by the frontend, we could have a set of attached expressions that are automatically received with the first batch of frames. How does that sound Panos? This way, we'll blow away two extra dumb things that are happening in the frontend on the client's framesadded notification: evaluating watch expressions and blowing away the stack *and/or* evaluating conditional breakpoints [1] and, as well, blowing away the stack.

[0] https://bugzilla.mozilla.org/show_bug.cgi?id=832470#c14
[1] bug 812172
Evaluating watch expressions on the server would require a new protocol request and if I remember correctly Jim felt that this should be unnecessary. It would certainly make the debugger snappier when pausing though.

It probably won't make a lot of difference for desktop, but we should try a Firefox for Android debugging session with a bunch of watch expressions and see what it currently feels like.
(In reply to Panos Astithas [:past] from comment #4)
> Evaluating watch expressions on the server would require a new protocol
> request and if I remember correctly Jim felt that this should be
> unnecessary. It would certainly make the debugger snappier when pausing
> though.
>
 
Certainly true, but at least we won't have to do this on every single frame. (at least not usually; adding an expression while paused would still yield a new stack, but that's Okay™)
Priority: -- → P3
Duplicate of this bug: 926592
Note that we eval console expressions against the currently selected frame in the debugger, and that with the browser toolbox this can be used in the browser debugger, too. This alleviates the pain here a bit because you can just eval what you need, but obviously having a watch expression which updates when you switch frames would be still better.
I haven't seen an answer to this question yet:

Should watch expressions only ever be evaluated in the frame-they-were-defined-in's scope, or should they be re-evaluated in every frame as you switch between frames and step in/out?
(In reply to Nick Fitzgerald [:fitzgen] [Ðoge:D6jomNp59N9TVfgc538HU3RswwmwZwcrd7] from comment #9)
> I haven't seen an answer to this question yet:
> 
> Should watch expressions only ever be evaluated in the
> frame-they-were-defined-in's scope, or should they be re-evaluated in every
> frame as you switch between frames and step in/out?

I would expect them all to stick around and be re-evaluated.

If the UI could be super-smart and only remove a result if the new result was sensible and not an exception ("foo is not defined"; instead, it could show some kind of indicator the result was from a different frame) then that'd be awesome, but just updating it when switching frames would be a good first step. I guess when stepping, all expression should be re-evaluated (even if they now throw errors/exceptions)

(on a separate note, it'd help if the tooltip also showed the value instead of "Edit this Watch" - right now for long expressions, I have to make the sidebar wider which is frustrating - then that'd be good, but that's orthogonal to this bug I suppose)
(In reply to :Gijs Kruitbosch from comment #10)
> (In reply to Nick Fitzgerald [:fitzgen]
> [Ðoge:D6jomNp59N9TVfgc538HU3RswwmwZwcrd7] from comment #9)
> > I haven't seen an answer to this question yet:
> > 
> > Should watch expressions only ever be evaluated in the
> > frame-they-were-defined-in's scope, or should they be re-evaluated in every
> > frame as you switch between frames and step in/out?
> 
> I would expect them all to stick around and be re-evaluated.
> 
> If the UI could be super-smart and only remove a result if the new result
> was sensible and not an exception ("foo is not defined"; instead, it could
> show some kind of indicator the result was from a different frame) then
> that'd be awesome, but just updating it when switching frames would be a
> good first step. I guess when stepping, all expression should be
> re-evaluated (even if they now throw errors/exceptions)

Visual Studio handles this by showing the expression value at a lower opacity and introduces a Refresh icon next to the expression value that can be clicked to force re-evaluation of the expression.

I can't remember if they do this only when evaluating the expression throws or for all cases of frame changes.
Why do we allow watch expressions to have side effects anyway? They are primarly useful to find out when a certain condition is matched, which can be done using side effect free expressions.
(In reply to Eddy Bruel [:ejpbruel] from comment #12)
> Why do we allow watch expressions to have side effects anyway? They are
> primarly useful to find out when a certain condition is matched, which can
> be done using side effect free expressions.

I'm not sure if the debugger APIs let us find out if an expression has a side effect or not, esp. what with getters, setters, etc. I'd expect to be able to usefully evaluate a watch expression that involved a getter, irrespective of whether that getter had side effects. I've also found actual assignment watch expressions useful at times, not least because it saves opening the console (which didn't use to be possible anyway)
Summary: Watch expressions should work in other frames than the top one → Watch expressions for non-toplevel frames
This is still reproducible in FF 43, brings debugging down to IE level in my opinion :/
Product: Firefox → DevTools

This should be fixed. Watch expressions are now re-evaluated when a frame is selected.

http://g.recordit.co/BU0LfSnMjp.gif

Status: NEW → RESOLVED
Type: defect → enhancement
Closed: 7 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.