1168760 - Clarify the child/parent actor setup documentation

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Patrick Brosset <:pbro>]

/toolkit/devtools/server/docs/lazy-actor-modules.md is nice, I just read it now for the first time, but I spotted a few typos, and I think it could use some clarifications here and there. Also, I don't see why both lazy actor registry and child/parent setup docs are in one file, they're not really related.

Comment 1

[Patrick Brosset <:pbro>]

Attachment: MozReview Request: bz://1168760/pbrosset

/r/9427 - Bug 1168760 - Clarify the lazy-actor and parent-child-setup docs; r=ochameau

Pull down this commit:

hg pull -r a65fadb8395c1bc59d6297380108b688375186b7 https://reviewboard-hg.mozilla.org/gecko/

Comment 2

[Alexandre Poirot [:ochameau]]

Comment on attachment 8611100 [details]
MozReview Request: bz://1168760/pbrosset

https://reviewboard.mozilla.org/r/9425/#review8177

::: toolkit/devtools/server/docs/lazy-actor-modules.md:3
(Diff revision 1)
> -The **DebuggerServer** loads and creates most of the actors lazily to keep
> +The **DebuggerServer** loads and creates most of the actors lazily to keep the initial memory usage down (which is extremely important on lower end devices).

While you are at it, you could mention apps, or e10s with more than just one child process.
This is the scenarios where the memory cost is multiplied and becomes a real issue.
But that's because we spawn a debugger server in each process... which may not be obvious as we are only connecting to the main process one, the others is just for piping and for the framework to work.

::: toolkit/devtools/server/docs/lazy-actor-modules.md:5
(Diff revision 1)
> -## Register a lazy global/tab actor module
> +## How to register a lazy actor module

History...
Yes, this is lazy actors, but now all actors are registered like this (all but webbrowser.js / TabActor, which are special).

Ideally, this document would be "how to create/register an actor".
And here is how to register one, and yes btw, that's lazy.

::: toolkit/devtools/server/docs/lazy-actor-modules.md:25
(Diff revision 1)
>  ```

Again, while you are at it, it can be useful to drop a note about main.js and addBrowserActors for global actors and addChildActors for tab actors.
If you want to contribute to mozilla-central, you will have to register your actor there.
If you are working on an addon, you should call these methods from your addon code directly.

::: toolkit/devtools/server/docs/cross-process-actors.md:5
(Diff revision 1)
> +Some actors need to exchange messages between the parent and the child process.

I would add some words about why we have to do so:
Typicaly, when some components don't work correctly or are not available in child processes.

::: toolkit/devtools/server/docs/cross-process-actors.md:20
(Diff revision 1)
> +    // is not running in a child process.

Wouldn't it be clearer to word it like this:
"only if the actormodule is running in a child process"?

::: toolkit/devtools/server/docs/cross-process-actors.md:26
(Diff revision 1)
> +The above setupChildProcess helper will use the **DebuggerServer.setupInParent** to run a given setup function in the parent process Debugger Server, e.g. in the the **director-registry**:

s/above/following/ ?
s/in the the/in the/
Also, I would add "module" after **director-registry**

::: toolkit/devtools/server/docs/cross-process-actors.md:93
(Diff revision 1)
> +* the actor module then uses the `DebuggerServer.setupInParent` helper to start seting up a parent-process counterpart,

seting -> setting

::: toolkit/devtools/server/docs/cross-process-actors.md:1
(Diff revision 1)
> +# Cross-process actors

I suggested yesterday to Mike on the storage actor bug for e10s to implement cross process actors, actors that could be used locally to avoid this message manager mess.
So I'm reluctant to call this cross process actors as this isn't really that. It's more a "how to handle e10s in actors?" doc.

Comment 3

[Alexandre Poirot [:ochameau]]

While you are at improving it... here is loads of suggestions ;)

Comment 4

[Patrick Brosset <:pbro>]

Comment on attachment 8611100 [details]
MozReview Request: bz://1168760/pbrosset

/r/9427 - Bug 1168760 - Clarify the lazy-actor and parent-child-setup docs; r=ochameau

Pull down this commit:

hg pull -r 0c42ed8fd8c826a8581b980d8211379c908f21ce https://reviewboard-hg.mozilla.org/gecko/

Comment 5

[Alexandre Poirot [:ochameau]]

Comment on attachment 8611100 [details]
MozReview Request: bz://1168760/pbrosset

https://reviewboard.mozilla.org/r/9425/#review8183

Thanks a lot for all the improvements!

::: toolkit/devtools/server/docs/actor-registration.md:1
(Diff revision 2)
> +# How to register a tab actor or a global actor

nit: How to register an actor

::: toolkit/devtools/server/docs/actor-registration.md:27
(Diff revision 2)
> +If you are adding a new actor for an add-on, you should call `DebuggerServer.registerModule` directly from your add-on code.

for -> from ?

::: toolkit/devtools/server/docs/actor-registration.md:5
(Diff revision 2)
> +To register a global actor:

If I could abuse of your time, I'd drop a note about global vs tab. But I could understand you stop the contribution here ;)

Starting with tab actors is simplier as it is easy to describe. It's for actors targeting a document: a tab in firefox, an app on b2g or a remote document in Firefox Mobile/Safari/Chrome for android (via Valence).

Global actors are for the other stuff, not related to any particular document, but global to the whole Firefox/FxOS/Chrome/Safari instance we are connecting to (preferences actor is a good example).

Comment 6

[Patrick Brosset <:pbro>]

Thanks Alex. I will upload a new patch in a second that should take these last recommendations into consideration.

Comment 7

[Patrick Brosset <:pbro>]

Comment on attachment 8611100 [details]
MozReview Request: bz://1168760/pbrosset

https://reviewboard.mozilla.org/r/9425/#review8185

Ship It!

Comment 8

[Patrick Brosset <:pbro>]

Comment on attachment 8611100 [details]
MozReview Request: bz://1168760/pbrosset

/r/9427 - Bug 1168760 - Clarify the lazy-actor and parent-child-setup docs; r=ochameau

Pull down this commit:

hg pull -r dde11461142d1d81b82550ae0f31d0bafe45c7d3 https://reviewboard-hg.mozilla.org/gecko/

Comment 9

[Patrick Brosset <:pbro>]

No try build because this is only a doc change, no code changes were done.

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/9905af238c6c

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9905af238c6c

Comment 13

[Patrick Brosset <:pbro>]

Attachment: MozReview Request: Bug 1168760 - Clarify the lazy-actor and parent-child-setup docs; r=ochameau