872091 - Rearrange toolkit/devtools a bit

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Dave Camp (:dcamp)]

* Move files around so that things look like:

devtools/
- server/
-- dbg-server.jsm
-- main.js (was dbg-server.js)
-- transport.js (was dbg-transport.js)
-- actors/
--- webbrowser.js (was dbg-browser-actors.js, the rest of these files follow that pattern).
--- script.js
--- profiler.js
--- styleeditor.js
--- gcli.js
- client/
-- dbg-client.jsm

Rebase-o-rama, sorry.

Comment 1

[Dave Camp (:dcamp)]

(Oops, submitted early):

* Install .js into resource://gre/modules/devtools/ with directory intact, for example: resource://gre/modules/devtools/server/actors/script.js

* A few "make sure errors are printed on the console" fixes snuck in to the patch.

Comment 2

[Dave Camp (:dcamp)]

Attachment: v1

Comment 3

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

Comment on attachment 749359 [details] [diff] [review]
v1

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

Just one typo.

::: b2g/chrome/content/shell.js
@@ +996,5 @@
>    start: function debugger_start() {
>      if (!DebuggerServer.initialized) {
>        // Ask for remote connections.
>        DebuggerServer.init(this.prompt.bind(this));
> +      DebuggerServer.addActors("resource://gre/modules/devtools/server/actors/webbrowser");

s/webbrowser/webbrowser.js/

Comment 4

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

A build with the patch applied gives me this when entering "tools srcdir /foo/bar":

console.error: 
  Message: ReferenceError: gDevTools is not defined
  Stack:
    @resource://gre/modules/devtools/Loader.jsm:230
.exec/<@resource:///modules/devtools/BuiltinCommands.jsm:1605
resolve@resource://gre/modules/commonjs/sdk/core/promise.js:120
then@resource://gre/modules/commonjs/sdk/core/promise.js:45
resolve@resource://gre/modules/commonjs/sdk/core/promise.js:187
resolve@resource://gre/modules/commonjs/sdk/core/promise.js:120
then@resource://gre/modules/commonjs/sdk/core/promise.js:45
resolve@resource://gre/modules/commonjs/sdk/core/promise.js:187
onmessage@resource://gre/modules/osfile/_PromiseWorker.jsm:134

Comment 5

[Jim Blandy :jimb]

While we're at it, could dbg-client.jsm become client/debugger.jsm, or client/dbg.jsm, or something less repetitive?

Comment 6

[Jim Blandy :jimb]

Comment on attachment 749359 [details] [diff] [review]
v1

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

This looks great, but as I reviewed it, something did occur to me that we perhaps ought to think about:

I'm not sure gathering the actors together in their own directory really makes sense. The protocol is designed to let different tools use it independently, so having allthe tools' actors in one place hopefully isn't as valuable as having each actor in the same directory with its related code. So, for example, the console actor should be with the other console stuff. One could still remove redundancy:

devtools
- dbg-server.jsm
- main.js
- transport.js
- webconsole
  - actors.js
  - WebConsoleUtils.jsm
- dbg
  - client.jsm
  - root-actor.js
  - browser-actors.js
  - script-actors.js
- styleeditor-actors.js

Things like styleeditor that are currently simple can be top-level files for now, and become directories if they grow.

::: b2g/chrome/content/shell.js
@@ +1004,3 @@
>  #endif
>        if ("nsIProfiler" in Ci)
> +        DebuggerServer.addActors("resource://gre/modules/devtools/server/actors/profiler.js");

Indentation?

::: toolkit/devtools/server/dbg-server.jsm
@@ -31,5 @@
>    "    throw e;\n" +
>    "  }\n" +
>    "}";
>  
> -Cu.import("resource://gre/modules/devtools/dbg-client.jsm");

... what in the world was that doing there?

Comment 7

[Jim Blandy :jimb]

> ::: b2g/chrome/content/shell.js
> @@ +1004,3 @@
> >  #endif
> >        if ("nsIProfiler" in Ci)
> > +        DebuggerServer.addActors("resource://gre/modules/devtools/server/actors/profiler.js");
> 
> Indentation?

I misread this. Sorry. It's completely fine...

Comment 8

[Dave Camp (:dcamp)]

https://hg.mozilla.org/integration/fx-team/rev/1c2ca70f459e
https://hg.mozilla.org/integration/fx-team/rev/1904eab9bcfb
https://hg.mozilla.org/integration/fx-team/rev/a0789668bcf2

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/1c2ca70f459e
https://hg.mozilla.org/mozilla-central/rev/1904eab9bcfb
https://hg.mozilla.org/mozilla-central/rev/a0789668bcf2