818385 - Support chrome debugging in Firefox OS

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P2)

Description

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

As far as the remote debugging protocol is concerned, chrome debugging is mostly a global thread actor that has visibility to all JS running in the application process. We should add such capability to Firefox OS in order to help debugging Firefox itself. The patch in bug 817580 adds a special "tab" target that corresponds to the shell.xul window, but having the client connect to the Remote Process target would be more natural and less of a hack.

Comment 1

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

Filter on BLACKEAGLE2.

Comment 2

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

Bug 817580 won't be adding the chrome window as a debuggee after all, so this bug should offer it from the chromeDebuger actor, as we do in desktop Firefox.

Comment 3

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

Attachment: Patch v1

This proved almost trivial to implement and works fine with b2g-desktop.

The main benefit of this change is having a sane remote debugging experience for the Firefox OS Simulator and B2G desktop. Currently, when selecting the "Connect..." menu item, the list of available targets includes shell.xul and Main Process. If the user selects the latter, he will get a working web console and profiler, but a broken debugger. Selecting shell.xul results in a broken web console, due to a different bug that has not been fixed in the v1 branch (which we are also going to fix).

This patch allows debugging all JavaScript code that runs in the B2G process, since every global is added as a debuggee. Obviously this is not something that we want to have enabled on devices, which is why it is under the existing #ifndef check.

Comment 4

[[:fabrice] Fabrice Desré]

Comment on attachment 716075 [details] [diff] [review]
Patch v1

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

r=me with comment addressed

::: b2g/chrome/content/dbg-browser-actors.js
@@ +66,1 @@
>    let actor = this._tabActors.get(this.browser);

Remove the let here.

Comment 5

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

Attachment: Patch v2

(In reply to Fabrice Desré [:fabrice] from comment #4)
> r=me with comment addressed
> 
> ::: b2g/chrome/content/dbg-browser-actors.js
> @@ +66,1 @@
> >    let actor = this._tabActors.get(this.browser);
> 
> Remove the let here.

Good catch, thanks.

Comment 6

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/a2e41cb1e33e

Comment 7

[Ryan VanderMeulen [:RyanVM]]

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

Comment 8

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

Fabrice, can we uplift this to the mozilla-b2g18 branch, so that it can be picked up by the simulator? If so, what flags should I set?

Comment 9

[[:fabrice] Fabrice Desré]

You need to ask for approval‑mozilla‑b2g18 in the patch details.

Comment 10

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

Comment on attachment 716149 [details] [diff] [review]
Patch v2

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): new feature that we want to get into the Firefox OS Simulator
User impact if declined: developers using the Firefox OS Simulator or b2g-desktop will not be able to connect to the "Main Process" target and have a working debugger. Currently their only choice for a working debugger is the "shell.xul" target, which unfortunately results in a broken web console due to a different bug.
Testing completed: in m-i and m-c.
Risk to taking this patch (and alternatives if risky): very low risk, the patch is only compiled for desktop b2g builds (not for devices) is a trivial port of the relevant desktop functionality, and will be used/triggered by developers only
String or UUID changes made by this patch: none

Comment 11

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

Comment on attachment 716149 [details] [diff] [review]
Patch v2

D'oh, apparently chrome debugging for desktop Firefox landed in Firefox 19, so there is nothing to do for 18.