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

RESOLVED FIXED in mozilla7

Status

()

Core
IPC
--
enhancement
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: cjones, Assigned: cjones)

Tracking

unspecified
mozilla7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments, 2 obsolete attachments)

15.62 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
Details | Diff | Splinter Review
2.12 KB, patch
Details | Diff | Splinter Review
3.40 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
Details | Diff | Splinter Review
10.19 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
Details | Diff | Splinter Review
9.39 KB, patch
Details | Diff | Splinter Review
3.23 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
Details | Diff | Splinter Review
|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|.
Created attachment 491780 [details] [diff] [review]
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!
Created attachment 492894 [details] [diff] [review]
part 1: Frontend support for IPDL |opens|
Attachment #491780 - Attachment is obsolete: true
Attachment #492894 - Flags: review?(bent.mozilla)
Created attachment 492895 [details] [diff] [review]
part 2: Frontend tests
Created attachment 492896 [details] [diff] [review]
part 3: Add AsyncChannel::Echo() to allow sending a message back to the originating endpoint
Attachment #492896 - Flags: review?(bent.mozilla)
Created attachment 492897 [details] [diff] [review]
part 4: Library support of |opens| API
Attachment #492897 - Flags: review?(bent.mozilla)
Created attachment 492898 [details] [diff] [review]
part 5: Generate C++ goop for creating |opens| channels
Attachment #492898 - Flags: review?(bent.mozilla)
Created attachment 492900 [details] [diff] [review]
part 6: Test IPDL |opens|
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.
This is what we need to allow video decoder threads to communicate directly with the compositor process, right?
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.
Blocks: 598869
Review ping.
Review ping.
Review ping.
Created attachment 528810 [details] [diff] [review]
part 4: Library support of |opens| API, v2

Needed a bit of reorg after requested changes in bug 564086.
Attachment #492897 - Attachment is obsolete: true
Attachment #528810 - Flags: review?(bent.mozilla)
Attachment #492897 - Flags: review?(bent.mozilla)
Review ping.
Review ping.
Blocks: 657261
Attachment #492894 - Flags: review?(bent.mozilla) → review+
Attachment #492896 - Flags: review?(bent.mozilla) → review+
Attachment #492898 - Flags: review?(bent.mozilla) → review+
Comment on attachment 528810 [details] [diff] [review]
part 4: Library support of |opens| API, v2

Looks great! Sorry for the delay :-/
Attachment #528810 - Flags: review?(bent.mozilla) → review+
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
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Updated

6 years ago
Target Milestone: --- → mozilla7
You need to log in before you can comment on or make changes to this bug.