Last Comment Bug 613442 - IPDL: Offer a PFoo::Open(actor) interface to open a new IPC channel between the current process and |actor|'s remote process
: IPDL: Offer a PFoo::Open(actor) interface to open a new IPC channel between t...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: IPC (show other bugs)
: unspecified
: All All
: -- enhancement (vote)
: mozilla7
Assigned To: Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
:
:
Mentors:
Depends on: 564086
Blocks: 562770 598868 598869 657261
  Show dependency treegraph
 
Reported: 2010-11-18 23:19 PST by Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
Modified: 2011-06-06 00:44 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Frontend of IPDL |opens| and tests (16.15 KB, patch)
2010-11-18 23:23 PST, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Splinter Review
part 1: Frontend support for IPDL |opens| (15.62 KB, patch)
2010-11-23 19:22 PST, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
bent.mozilla: review+
Details | Diff | Splinter Review
part 2: Frontend tests (2.12 KB, patch)
2010-11-23 19:23 PST, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Splinter Review
part 3: Add AsyncChannel::Echo() to allow sending a message back to the originating endpoint (3.40 KB, patch)
2010-11-23 19:23 PST, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
bent.mozilla: review+
Details | Diff | Splinter Review
part 4: Library support of |opens| API (4.56 KB, patch)
2010-11-23 19:23 PST, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Splinter Review
part 5: Generate C++ goop for creating |opens| channels (10.19 KB, patch)
2010-11-23 19:23 PST, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
bent.mozilla: review+
Details | Diff | Splinter Review
part 6: Test IPDL |opens| (9.39 KB, patch)
2010-11-23 19:24 PST, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Splinter Review
part 4: Library support of |opens| API, v2 (3.23 KB, patch)
2011-04-28 01:41 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
bent.mozilla: review+
Details | Diff | Splinter Review

Description Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-11-18 23:19:43 PST
|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 2010-11-18 23:23:17 PST
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!
Comment 2 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-11-23 19:22:54 PST
Created attachment 492894 [details] [diff] [review]
part 1: Frontend support for IPDL |opens|
Comment 3 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-11-23 19:23:08 PST
Created attachment 492895 [details] [diff] [review]
part 2: Frontend tests
Comment 4 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-11-23 19:23:23 PST
Created attachment 492896 [details] [diff] [review]
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 2010-11-23 19:23:38 PST
Created attachment 492897 [details] [diff] [review]
part 4: Library support of |opens| API
Comment 6 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-11-23 19:23:54 PST
Created attachment 492898 [details] [diff] [review]
part 5: Generate C++ goop for creating |opens| channels
Comment 7 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-11-23 19:24:08 PST
Created attachment 492900 [details] [diff] [review]
part 6: Test IPDL |opens|
Comment 8 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-11-23 19:33:04 PST
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) 2010-11-23 20:32:09 PST
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 2010-11-23 20:58:36 PST
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 2011-03-23 12:12:12 PDT
Review ping.
Comment 12 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-04-20 18:13:44 PDT
Review ping.
Comment 13 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-04-27 22:32:43 PDT
Review ping.
Comment 14 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-04-28 01:41:46 PDT
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.
Comment 15 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-05-04 15:27:09 PDT
Review ping.
Comment 16 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-05-10 10:06:09 PDT
Review ping.
Comment 17 Ben Turner (not reading bugmail, use the needinfo flag!) 2011-05-15 21:04:22 PDT
Comment on attachment 528810 [details] [diff] [review]
part 4: Library support of |opens| API, v2

Looks great! Sorry for the delay :-/

Note You need to log in before you can comment on or make changes to this bug.