787975 - Do not cross the client/server boundaries in the JSTerm $0 helper

Status: RESOLVED FIXED

(DevTools :: Console, defect, P2)

Description

[Mihai Sucan [:msucan]]

Since bug 673148 the JSTerm $0 helper crosses the process boundaries. Once bug 768096 lands it will cross the client/server boundaries. We need to fix this with some support for the remote debugging protocol in the Inspector.

Comment 1

[Mihai Sucan [:msucan]]

Patrick: I was looking into writing a patch for this bug. Did some code checks and it seems we track node selection only in the client. So when the server would try to get $0 it has no way to know which is the currently selected element.

Do we want to track the selected element in the server? Or should we send the nodeActor ID of the selected element every time when we ask for JS eval from the client? I think the latter is to be preferred.

Comment 2

[Patrick Brosset <:pbro>]

Yeah, I'd go for solution two (tracking on the client only and sending the ID to the server). I think anything that keeps the server from getting stateful is a good idea.

Comment 3

[Chris Peterson [:cpeterson]]

Mass tracking-e10s flag change. Filter bugmail on "2be0fcce-e36a-4e2c-aa80-0e3d33eb5406".

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Attachment: Do not cross the client/server boundaries in the JSTerm $0 helper. r=?

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8463604 [details] [diff] [review]
Do not cross the client/server boundaries in the JSTerm $0 helper. r=?

So I think I've got the start of this worked out - I can pass the actorID to the server without too much difficulty.

I'm a bit stuck though in getting the $0 helper in utils.js to somehow see that actorID and do something with it. I'm getting a bit lost in how the native Debugger code takes the helper bindings and runs script on them. What exactly do those helper functions have access to? Am I able to get a handle on the selectionActor and just echo it back as a node selection in the console? If so, how?

Comment 9

[Mihai Sucan [:msucan]]

Comment on attachment 8463604 [details] [diff] [review]
Do not cross the client/server boundaries in the JSTerm $0 helper. r=?

Review of attachment 8463604 [details] [diff] [review]:
-----------------------------------------------------------------

Good start. Thanks!

JSTermHelpers(obj) function is invoked with the webconsole actor instance as the first argument. You can see all new functions and getters are added to the actor.sandbox object.

In evalWithDebugger() use the selectionActor id to get the actor from the server connection, then get the real dom node from the actor, then store it temporarily on the webconsole actor object, eg. this.selectedNode, like we do for evalInput. In the $0 getter you can return aOwner.selectedNode. You will most likely need to call aOwner.makeDebuggeeValue(aOwner.selectedNode).

To get the raw node use: this.conn.getActor(nodeActorID).rawNode.

The native Debugger evalInGlobalWithBindings() adds all the properties on top of the properties of the global object, the window. So if we bind $0 during eval and your page also sets $0 in the |window| global, then during eval you have to explicitly ask for it by writing window.$0. I dont know the c++ tech details of how that is done. JSTerm helpers are executed in chrome context, not in the low privileges context of the web page - that would limit what we can do with them. From jsterm helpers you have access to the console actor, and to the whole debugger api - so you can also get stuff from the page.

Please let me know if you have more questions. Thanks!

::: browser/devtools/webconsole/webconsole.js
@@ +3274,5 @@
>      let onResult = this._executeResultCallback.bind(this, message, aCallback);
>  
> +    let options = {
> +      frame: this.SELECTED_FRAME,
> +      selection: selection,

I suggest you pass in the nodeFront actorID directly because that's what we only care about, and rename the property accordingly.

::: toolkit/devtools/server/actors/webconsole.js
@@ +725,5 @@
>      let evalOptions = {
>        bindObjectActor: aRequest.bindObjectActor,
>        frameActor: aRequest.frameActor,
>        url: aRequest.url,
> +      selectionActor: aRequest.selectionActor,

nodeActor

Comment 10

[Mike Conley (:mconley) (:⚙️)]

Attachment: Do not cross the client/server boundaries in the JSTerm $0 helper. r=?

Comment 11

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8464203 [details] [diff] [review]
Do not cross the client/server boundaries in the JSTerm $0 helper. r=?

Thanks for the info - that gave me everything I needed!

This appears to work, and I pass all of the webconsole tests.

Comment 12

[Mihai Sucan [:msucan]]

Comment on attachment 8464203 [details] [diff] [review]
Do not cross the client/server boundaries in the JSTerm $0 helper. r=?

Review of attachment 8464203 [details] [diff] [review]:
-----------------------------------------------------------------

This is good stuff. Thank you!

r+ with one request: please do s/selectedActorID/selectedNodeActor/g in the patch. 'selectedActorID' is slightly confusing (selected? what is selected?) and inconsistent with 'frameActor' (no 'ID').

Please also update the remote web console docs to mention the new evaluateJS option: https://developer.mozilla.org/en-US/docs/Tools/Web_Console/remoting

Thanks!

Comment 13

[Mike Conley (:mconley) (:⚙️)]

Thanks! Changed the name, and pushed:

remote:   https://hg.mozilla.org/integration/fx-team/rev/b55408fd3fa5

I'll update the documentation now.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b55408fd3fa5

Comment 15

[Mike Conley (:mconley) (:⚙️)]

Bug notes:

http://people.mozilla.org/~mconley2/bugnotes/bug-787975.html