1128988 - [e10s] Land runAt support for commands/converters in GCLI

Status: RESOLVED FIXED

(DevTools Graveyard :: Graphic Commandline and Toolbar, defect, P1)

Comment 1

[Jim Mathies [:jimm]]

Was this filed right Joe? Looks like this was supposed to track, but I'm not sure. Anyway, for dev tools e10s bugs we usually track to the dte-e10s meta and mark tracking-e10s:+. Does that fit this bug?

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Jim Mathies [:jimm] from comment #1)
> Was this filed right Joe? Looks like this was supposed to track, but I'm not
> sure. Anyway, for dev tools e10s bugs we usually track to the dte-e10s meta
> and mark tracking-e10s:+. Does that fit this bug?

AIUI, this work will enable GCLI commands to work with e10s.  I'll block the meta bug.

Comment 3

[Joe Walker [:jwalker]]

As jryans said, this is an e10s bug. I'm not sure what the correct meta-fu is for that, but whatever works is good for me.
This bug should fix bug 1034511 and bug 1074233.

Comment 4

[Joe Walker [:jwalker]]

Attachment: 1128988.patch

This is not complete yet, I'm working through the commands making sure they work properly, but I expect most of my changes from here to be additions, and I'd like to set the ball rolling.

I've broken out and described in detail the changes here https://github.com/joewalker/gecko-dev/pull/7

Comment 5

[Brian Grinstead [:bgrins]]

Comment on attachment 8565357 [details] [diff] [review]
1128988.patch

See comments at https://github.com/joewalker/gecko-dev/pull/7 and https://github.com/joewalker/gcli/pull/76

Comment 6

[Eddy Bruel [:ejpbruel]]

This bug blocks bug 1074233, which is M6, so I propose to track this bug for M6 as well.

Comment 7

[Jim Mathies [:jimm]]

Joe, can you fill us in on the status of this work? The e10s team is curious if anyone is working on this currently.

Comment 8

[Joe Walker [:jwalker]]

(In reply to Jim Mathies [:jimm] from comment #7)
> Joe, can you fill us in on the status of this work? The e10s team is curious
> if anyone is working on this currently.

I'm working on it. Progress here: https://github.com/joewalker/gecko-dev/pulls
Hope to be done in a few days

Comment 9

[Joe Walker [:jwalker]]

Attachment: 1128988.patch

Reviews in progress:

https://github.com/joewalker/gecko-dev/pull/8
https://github.com/joewalker/gecko-dev/pull/9
https://github.com/joewalker/gecko-dev/pull/10
https://github.com/joewalker/gecko-dev/pull/11
https://github.com/joewalker/gecko-dev/pull/12
https://github.com/joewalker/gecko-dev/pull/13

One outstanding issue: a memory leak

Comment 10

[Brian Grinstead [:bgrins]]

Attachment: gcli-leak-fix.patch

After debugging the leaks, I found this fixed browser_dbg_cmd-dbg.js

Comment 11

[Joe Walker [:jwalker]]

Attachment: 0001-Bug-1128988-runAt-support-for-commands-converters-r-.patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fc4984e39c64

Comment 12

[Joe Walker [:jwalker]]

Comment on attachment 8596075 [details] [diff] [review]
0001-Bug-1128988-runAt-support-for-commands-converters-r-.patch

Reviews:
https://github.com/joewalker/gecko-dev/pull/8
https://github.com/joewalker/gecko-dev/pull/9
https://github.com/joewalker/gecko-dev/pull/10
https://github.com/joewalker/gecko-dev/pull/11
https://github.com/joewalker/gecko-dev/pull/12
https://github.com/joewalker/gecko-dev/pull/13

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/6ecd33215340

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6ecd33215340

Comment 15

[Brian Grinstead [:bgrins]]

Somehow this has caused the Inspect Element button to disappear in the Browser Toolbox (Bug 1157906).  Looks like it may be a loader issue?

Module `definitions` is not found at resource://gre/modules/commonjs/definitions.js

"@resource:///modules/devtools/commandline/commands-index.js:82:22GcliActor<._getRequisition@resource://gre/modules/devtools/server/actors/gcli.js:245:22GcliActor<.specs<@resource://gre/modules/devtools/server/actors/gcli.js:96:12actorProto/</handler@resource://gre/modules/devtools/server/protocol.js:1006:19DSC_onPacket@resource://gre/modules/devtools/server/main.js:1456:15DebuggerTransport.prototype._onJSONObjectReady/<@resource://gre/modules/devtools/transport/transport.js:471:9makeInfallible/<@resource://gre/modules/devtools/DevToolsUtils.js:82:14makeInfallible/<@resource://gre/modules/devtools/DevToolsUtils.js:82:14"

Comment 16

[Brian Grinstead [:bgrins]]

FYI, to run the Browser Toolbox in a separate window so you can see logs, you can open up a normal BT on  so that the server starts and then run a separate command:

./mach run --profile /tmp/foobar -chrome chrome://browser/content/devtools/framework/toolbox-process-window.xul --purgecaches --jsconsole