1235887 - [jsdbg2] Implement Debugger.Frame.prototype.pop

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect)

Description

[Lin Clark [:linclark]]

There is a TODO in devtools/server/actors/script.js that depends on this:

https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/script.js#3208

Some questions:
- What do we need to do to make this work? Are there any remaining platform changes that need to happen?
- What client code would use this, and for what use case? Is any client code currently using it?

This TODO has been around for 4 years, so it is also possible that we don't need  this anymore. If we aren't going to implement this, can we remove the pop method that contains this TODO? When it is removed, none of the tests break.

Comment 1

[Lin Clark [:linclark]]

Needinfo-ing Jim because Eddy suggested that he'd be the best person to ask about this.

Comment 2

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

Just a friendly heads up: things related to SpiderMonkey's Debugger API tend to go in Core: JavaScript.

Comment 3

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

(In reply to Lin Clark from comment #0)
> - What do we need to do to make this work? Are there any remaining platform
> changes that need to happen?

The Debugger API is implemented in C++ inside of SpiderMonkey, so extending the API is itself platform work.

I'm not 100% sure if there is much other infra needed inside SM for this method, though. Possibly a good-first-bug for SM but this is really a part of SM that I'm not super familiar with.

> - What client code would use this, and for what use case? Is any client code
> currently using it?

Currently, no. The user-facing feature that would be built on top of this would be "force return 5 from this function instead of continuing to execute it and return whatever would otherwise be returned."

> This TODO has been around for 4 years, so it is also possible that we don't
> need  this anymore. If we aren't going to implement this, can we remove the
> pop method that contains this TODO? When it is removed, none of the tests
> break.

I think it would be good to remove this RDP method since it has always just returned "not yet implemented", but I do think this method is a logical and nice-to-have method for the Debugger API.

Comment 4

[Jim Blandy :jimb]

(In reply to Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8] from comment #3)
> (In reply to Lin Clark from comment #0)
> > - What client code would use this, and for what use case? Is any client code
> > currently using it?
> 
> Currently, no. The user-facing feature that would be built on top of this
> would be "force return 5 from this function instead of continuing to execute
> it and return whatever would otherwise be returned."

This sounds like just returning a { return: 5 } completion value, not D.F.p.pop, though.

D.F.p.pop applied to an arbitrary frame sounds kind of impossible to implement. What happens to the C++ frames in between the youngest frame and the one being popped? Since the frame being popped is necessarily a parent of the frame calling D.F.p.pop, where is that call's continuation afterwards?

Unless I'm confused, D.F.p.pop was an impossibility the moment it was designed (by me *cough*), and completion values are the actual primitive to use to implement the same behavior.

Oh, hey. D.F.p.pop was deleted from the Debugger API spec! Yay!


> I think it would be good to remove this RDP method since it has always just
> returned "not yet implemented", but I do think this method is a logical and
> nice-to-have method for the Debugger API.

+1 to removing the RDP method.

Comment 5

[Lin Clark [:linclark]]

Since everyone +1-ed it, I removed the RDP method in bug 1235901.

From what Jim said, it sounds like we can close this issue out. Nick, Jim, is that right?

Comment 6

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

Yeah I think I was mistaken.