Closed
Bug 885653
Opened 11 years ago
Closed 11 years ago
Add support for IPDL bridges that connect two of the same protocol
Categories
(Core :: IPC, defect)
Core
IPC
Tracking
()
RESOLVED
FIXED
mozilla26
People
(Reporter: dzbarsky, Assigned: dzbarsky)
References
Details
Attachments
(2 files, 1 obsolete file)
2.16 KB,
patch
|
justin.lebar+bug
:
review+
|
Details | Diff | Splinter Review |
11.96 KB,
patch
|
justin.lebar+bug
:
feedback+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•11 years ago
|
||
The issue with this patch is that we end up generating code like virtual PBrowserParent* AllocPBrowser( Transport* transport, ProcessId otherProcess) = 0; virtual PBrowserChild* AllocPBrowser( Transport* transport, ProcessId otherProcess) = 0; which isn't legal C++. How would you feel about renaming the (De)Alloc functions to contain Child/Parent in the name?
Assignee: nobody → dzbarsky
Status: NEW → ASSIGNED
Attachment #765802 -
Flags: review?(justin.lebar+bug)
Assignee | ||
Comment 2•11 years ago
|
||
We need this because we want both PContentParent and PContentChild to be able to open PBrowser, but don't want two PBrowserChild* AllocPBrowser declarations in PContentChild.
Attachment #765807 -
Flags: review?(justin.lebar+bug)
Assignee | ||
Comment 3•11 years ago
|
||
Comment on attachment 765807 [details] [diff] [review] Part 2: Use sets for channelOpenedActors I don't think we need this.
Attachment #765807 -
Attachment is obsolete: true
Attachment #765807 -
Flags: review?(justin.lebar+bug)
Assignee | ||
Comment 4•11 years ago
|
||
This is basically the approach we want. A ContentChild (TestRecursiveBridgeMainChild) asks the ContentParent (TestRecursiveBridgeMainParent) to create a new ContentParent and bridge them with a ContentBridge (TestRecursiveBridgeSub). The two ContentChild's allocate the two halves of the bridge. We can then use the bridge singleton to create and manage PBrowsers.
Attachment #766530 -
Flags: feedback?(justin.lebar+bug)
Assignee | ||
Updated•11 years ago
|
Summary: Add support for recursive IPDL bridges → Add support for IPDL bridges that connect two of the same protocol
Comment 5•11 years ago
|
||
Comment on attachment 765802 [details] [diff] [review] Part 1: Teach the compiler that protocols can bridge two of the same protocol diff --git a/ipc/ipdl/ipdl/type.py b/ipc/ipdl/ipdl/type.py index 07a6abd..fff695a 100644 --- a/ipc/ipdl/ipdl/type.py +++ b/ipc/ipdl/ipdl/type.py @@ -1776,11 +1776,12 @@ class BuildProcessGraph(TcheckVisitor): if pproc == cproc: if parentSideActor is not None: - self.error(bridges.loc, - "ambiguous bridge `%s' between `%s' and `%s'", - bridgeProto.name(), - parentSideProto.name(), - childSideProto.name()) + if parentSideProto != childSideProto: + self.error(bridges.loc, + "ambiguous bridge `%s' between `%s' and `%s'", + bridgeProto.name(), + parentSideProto.name(), + childSideProto.name()) if parentSideActor is not None and parentSideProto != childSideProto
Attachment #765802 -
Flags: review?(justin.lebar+bug) → review+
Comment 6•11 years ago
|
||
Comment on attachment 766530 [details] [diff] [review] Part 2: Add an IPDL test for recursive bridges I don't pretend to know what's going on here, but I presume you copy-pasted some existing test and changed it so as to cover recursive bridging. That seems fine...
Attachment #766530 -
Flags: feedback?(justin.lebar+bug) → feedback+
Assignee | ||
Comment 7•11 years ago
|
||
(In reply to Justin Lebar [:jlebar] from comment #5) > Comment on attachment 765802 [details] [diff] [review] > Part 1: Teach the compiler that protocols can bridge two of the same protocol > > diff --git a/ipc/ipdl/ipdl/type.py b/ipc/ipdl/ipdl/type.py > index 07a6abd..fff695a 100644 > --- a/ipc/ipdl/ipdl/type.py > +++ b/ipc/ipdl/ipdl/type.py > > @@ -1776,11 +1776,12 @@ class BuildProcessGraph(TcheckVisitor): > if pproc == cproc: > if parentSideActor is not None: > - self.error(bridges.loc, > - "ambiguous bridge `%s' between `%s' and > `%s'", > - bridgeProto.name(), > - parentSideProto.name(), > - childSideProto.name()) > + if parentSideProto != childSideProto: > + self.error(bridges.loc, > + "ambiguous bridge `%s' between `%s' and > `%s'", > + bridgeProto.name(), > + parentSideProto.name(), > + childSideProto.name()) > > if parentSideActor is not None and parentSideProto != childSideProto That's not the same as what I did, and in fact causes us to not generate the AllocPContentBridge functions because we go through the pproc == cproc case twice and overwrite the results of the first one the second time.
Assignee | ||
Comment 9•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/395ba08436fd
Comment 10•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/395ba08436fd
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Updated•10 years ago
|
Blocks: nested-oop
You need to log in
before you can comment on or make changes to this bug.
Description
•