613442 - IPDL: Offer a PFoo::Open(actor) interface to open a new IPC channel between the current process and |actor|'s remote process

Status: RESOLVED FIXED

(Core :: IPC, enhancement)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

|opens| is much simpler than |bridge|.  The syntax is

 protocol POpener {
   (parent|child) opens POpened;
 };

which says that either POpenerParent or POpenerChild may create a new POpened tree.  POpenedChild will be created by POpenerChild, and POpenedParent will be created by POpenerParent.

The current use for this is opening PVideo/Audio/Media/??? protocol(s) between the content process and chrome process.  Future uses might be
 - opening an rpc-privileged PBackwardsCompat to enable cpows for desktop extensions
 - opening a privileged debugging protocol
 - off-main-content-thread gfx processing

The backend impl of this will be almost the same as for |bridge|.

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Frontend of IPDL |opens| and tests

Produces the graph

Processes
   |jetpackContentChild|
   |jetpackContentParent|
   |contentChild|mediaChild|pluginParent|
     (contentChild)--spawns-->(pluginChild)
   |jetpackChild|
   |contentParent|jetpackParent|compositorChild|mediaParent|
     (contentParent)--spawns-->(compositorParent)
     (contentParent)--spawns-->(jetpackChild)
Bridges
   (jetpackChild)--jetpackContent bridge-->(contentChild)
Opens
   (contentChild)--opens-->(media)

Holding off on review until the backend, which should be pretty straightforward now!

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 1: Frontend support for IPDL |opens|

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 2: Frontend tests

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 3: Add AsyncChannel::Echo() to allow sending a message back to the originating endpoint

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 4: Library support of |opens| API

Comment 6

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 5: Generate C++ goop for creating |opens| channels

Comment 7

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 6: Test IPDL |opens|

Comment 8

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Probably worth taking a glance at the C++ test (part 6) to see how all these pieces fit together.

The main difference between |bridges| and |opens| is that |bridges| creates a new channel between spanning the "other sides" of actors A and B, that is, it spans two remote processes.  |opens| on the other hand creates a new channel between an actor A and its remote side.  So whereas with |bridges| we're sending channel-opened messages to A-remote and B-remote from A's process, with |opens| we're sending channel-opened messages to A and A-remote.  (Please let me know if this makes sense.)

With that in mind, the map of the implementation should be pretty straightforward --- parts 1, 4, and 5 repurpose machinery created for |bridges| to also work for |opens|.  Part 3 adds support for saying, |asyncChannel->Echo(msg)|, and having the message delivered back to |asyncChannel| in "normal" order.  This means that we can use almost the same code for sending the channel-opened messages for new |bridges| as for new |opens|; the only difference is that with |bridges|, we Send() both notifications, and with |opens| we Send() one and Echo() the other.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This is what we need to allow video decoder threads to communicate directly with the compositor process, right?

Comment 10

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

It's what we need when the compositor is the chrome process, yeah.  Bug 564086 is what we need when we have a separate compositor process, as was originally planned.

Comment 11

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Review ping.

Comment 12

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Review ping.

Comment 13

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Review ping.

Comment 14

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 4: Library support of |opens| API, v2

Needed a bit of reorg after requested changes in bug 564086.

Comment 15

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Review ping.

Comment 16

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Review ping.

Comment 17

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 528810 [details] [diff] [review]
part 4: Library support of |opens| API, v2

Looks great! Sorry for the delay :-/

Comment 18

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

http://hg.mozilla.org/mozilla-central/rev/50002ea47d76
http://hg.mozilla.org/mozilla-central/rev/2d9aaa28ddfa
http://hg.mozilla.org/mozilla-central/rev/71df5c4a5b01
http://hg.mozilla.org/mozilla-central/rev/267d4a65c25f
http://hg.mozilla.org/mozilla-central/rev/d0f5d960ac9c
http://hg.mozilla.org/mozilla-central/rev/419406a98d10