1477988 - Introduce an API to create an actor in the parent process, from the actors running in the content processes.

Status: RESOLVED FIXED

(DevTools :: Framework, enhancement, P2)

Description

[Alexandre Poirot [:ochameau]]

A prerequisite to bug 1444132 would be to introduce an API to allow instanciating an actor in the parent process from the web console actors that typically runs in the content process.
The goal of this API is to only instanciate the actor and retrieve its actor ID.
Then the actor running in the content process will only hand over to the client this actor ID. Then the client is going to request this newly instanciated actor directly.

The proposal from bug 1444132 was to expose a new function on DebuggerServerConnection, called "spawnActorInParentProcess".
This method would require as argument:
* the actor ID of the actor requesting to create this new actor.
  It will be used to create the new actor ID and help following actors dependencies by looking at actor's IDs.
* require path to the actor module
  To know where to load the actor from.
* constructor name
  To know what is the symbol to import from this module.
* args
  An object of structure-clonable objects that will be piped over process boundary and passed as arguments to the actor constructor.

The actor constructor, i.e. per protocol.js convention, the "initialize" method, will receive the following arguments:
 * the DebuggerServerConnection it is attached to
 * all arguments passed into `args` argument of `spawnActorInParentProcess`
 * the message manager that can be used to communicate over the process boundary in case you have to communicate with the actor running in the content process.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1477988 - Implements DebuggerServer.spawnActorInParentProcess.

This new API allows to instanciate an actor in the parent process from actors running in the parent process.
The created actors are returned to the client from the actors running in content,
but after that, the client communicates directly with the created actors running in the parent process.

Review commit: https://reviewboard.mozilla.org/r/259038/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/259038/

Comment 2

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

Comment on attachment 8994484 [details]
Bug 1477988 - Implements DebuggerServer.spawnActorInParentProcess.

https://reviewboard.mozilla.org/r/259038/#review266106

Thanks, looks resonable overall! :)

::: devtools/server/main.js:967
(Diff revision 1)
> +
> +          // Manually set the actor ID in order to insert parent actorID as prefix
> +          // in order to help identifying actor hiearchy via actor IDs.
> +          // Remove `/` as it may confuse message forwarding between processes.
> +          const contentPrefix = spawnedByActorID.replace(connection.prefix, "")
> +                                              .replace("/", "-");

Maybe adjust indentation?

::: devtools/server/main.js:1869
(Diff revision 1)
> +
> +  // Instanciates a protocol.js actor in the parent process, from the content process
> +  // module is the absolute path to protocol.js actor module
> +  // `constructor` is the symbol exported by this module that implements Actor
> +  // `args` are the arguments to pass to its contructor
> +  spawnActorInParentProcess(spawnedByActorID, { module, constructor, args }) {

Maybe use `/** */` style comment for function?

::: devtools/server/tests/browser/browser_spawn_actor_in_parent.js:41
(Diff revision 1)
> +
> +  initDebuggerServer();
> +  const client = new DebuggerClient(DebuggerServer.connectPipe());
> +  const form = await connectDebuggerClient(client);
> +  const inContentFront = InContentFront(client, form);
> +  const formSpawn = await inContentFront.spawnInParent(ACTOR_URL);

It's a bit strange to check `isInContent == true` from the result of something called `spawnInParent`, but I think it's just how you bundled values for this test...  Maybe checking `isInContent` as as separate step would be more clear?

Comment 3

[Alexandre Poirot [:ochameau]]

Comment on attachment 8994484 [details]
Bug 1477988 - Implements DebuggerServer.spawnActorInParentProcess.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/259038/diff/1-2/

Comment 4

[Alexandre Poirot [:ochameau]]

Comment on attachment 8994484 [details]
Bug 1477988 - Implements DebuggerServer.spawnActorInParentProcess.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/259038/diff/2-3/

Comment 5

[Alexandre Poirot [:ochameau]]

Rebased on top of bug 1478244 to share the code registering actors in the content process.

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #2)
> ::: devtools/server/tests/browser/browser_spawn_actor_in_parent.js:41
> (Diff revision 1)
> > +
> > +  initDebuggerServer();
> > +  const client = new DebuggerClient(DebuggerServer.connectPipe());
> > +  const form = await connectDebuggerClient(client);
> > +  const inContentFront = InContentFront(client, form);
> > +  const formSpawn = await inContentFront.spawnInParent(ACTOR_URL);
> 
> It's a bit strange to check `isInContent == true` from the result of
> something called `spawnInParent`, but I think it's just how you bundled
> values for this test...  Maybe checking `isInContent` as as separate step
> would be more clear?

I added a dedicated "isInContent" request to clarify that.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/46e1c24f6842
Implements DebuggerServer.spawnActorInParentProcess. r=jryans

Comment 7

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/46e1c24f6842