Electrolysis-enabling existing code: Approaches
aza...@mozilla.com
(InstallTrigger), it seems the standard approach is
Approach A
==========
1. Write a new IPDL protocol, which involves creating several new
files and adding some code to several existing ones.
2. Edit existing code with things like this:
#ifdef MOZ_IPC
if (we are in a child process)
send a message using the new protocol we created, which gets the
parent to do the actual work
else
#else
do the actual work
#endif
The downside here is that adding a new protocol involves a relatively
nontrivial amount of new code, especially if all we actually want is
to remote a few function calls from the child to the parent. In
addition, we must modify existing code, scattering #ifdefs in it and
making it messier.
An alternate approach, at least for some cases, might look something
like this. Note: I have no idea if this idea makes sense or not.
Approach B
==========
Assume all we want to do is remote the usage of an XPCOM component -
synchronously - so that when called on the child, it gets actually run
on the parent. Assume also that performance is not terribly important
in this case. (These conditions hold with InstallTrigger.) Optimally
we might want to just mark in the idl file that a component is 'parent-
side', and then that would automatically generate code to make that
work (which we would just call from our implementation code for that
interface). That is, the implementation code would look something like
this
TheClass::TheFunction()
{
if (call to autogenerated function returns true) // that checks if
we are in a child process, and sends a message if so, and returns true
return
... do the usual stuff ...
}
The benefit of this approach is that it requires a very small amount
of new code in order to electrolysis-enable a small amount of
functionality. However it does require changing existing code and
making it messier. Also the necessary changes to the idl code
generator might not be simple (but at least that would be a one-time
thing).
Another alternative, which I also am not sure makes sense or not, is
this:
Approach C
==========
Again, assume we want to remote the usage of an XPCOM component, and
performance is not terribly important. Assume also that the XPCOM
component happens to be written in JavaScript (that is the case with
InstallTrigger). Then we can imagine the following method:
function component() { ... } // the old component-
component.prototype = { ... } // defining code, untouched
electrolify(component);
And the electrolify() function will, if in a child process, replace
methods so that when called they send messages; and if in a parent
process, set up the code to handle answering those messages.
The benefit of this approach is that we do not touch the existing code
*at all*, and we write only a single line of new code. For the actual
message passing it might be possible to reuse the existing JavaScript
messageManager code, perhaps.
---
Thoughts?
- azakai
Benjamin Smedberg
> ==========
>
> 1. Write a new IPDL protocol, which involves creating several new
> files and adding some code to several existing ones.
> 2. Edit existing code with things like this:
>
> #ifdef MOZ_IPC
> if (we are in a child process)
> send a message using the new protocol we created, which gets the
> parent to do the actual work
> else
> #else
> do the actual work
> #endif
>
> The downside here is that adding a new protocol involves a relatively
> nontrivial amount of new code, especially if all we actually want is
> to remote a few function calls from the child to the parent. In
> addition, we must modify existing code, scattering #ifdefs in it and
> making it messier.
Approach A-2 is similar, but instead of putting the #ifdef/if in each
method, we have two implementations of the XPCOM class. At createInstance
time, we choose which implementation to use.
> Approach B
> ==========
>
> Assume all we want to do is remote the usage of an XPCOM component -
> synchronously - so that when called on the child, it gets actually run
> on the parent. Assume also that performance is not terribly important
> in this case. (These conditions hold with InstallTrigger.) Optimally
> we might want to just mark in the idl file that a component is 'parent-
> side', and then that would automatically generate code to make that
> work (which we would just call from our implementation code for that
> interface). That is, the implementation code would look something like
> this
>
> TheClass::TheFunction()
> {
> if (call to autogenerated function returns true) // that checks if
> we are in a child process, and sends a message if so, and returns true
> return
> ... do the usual stuff ...
> }
>
> The benefit of this approach is that it requires a very small amount
> of new code in order to electrolysis-enable a small amount of
> functionality. However it does require changing existing code and
> making it messier. Also the necessary changes to the idl code
> generator might not be simple (but at least that would be a one-time
> thing).
Although this sounds attractive in theory, I really don't think we want to
go down this path. In particular:
Most XPCOM APIs have callbacks. Since we do not allow synchronous
chrome->content communication those callbacks are impossible to represent in
our communication layer.
As we develop IPDL APIs we want to make sure they are a good boundary for
parameter validation (treat all content process data as untrusted). By
making IPDL files auto-generate IPDL protcols, you hide a boundary and make
it less likely that we can effectively audit it.
Part of the IPDL system is explicit resource cleanup when a child process
crashes. This usually requires very explicit modelling.
> Approach C
> ==========
>
> Again, assume we want to remote the usage of an XPCOM component, and
> performance is not terribly important. Assume also that the XPCOM
> component happens to be written in JavaScript (that is the case with
> InstallTrigger). Then we can imagine the following method:
>
> function component() { ... } // the old component-
> component.prototype = { ... } // defining code, untouched
>
> electrolify(component);
>
> And the electrolify() function will, if in a child process, replace
> methods so that when called they send messages; and if in a parent
> process, set up the code to handle answering those messages.
>
> The benefit of this approach is that we do not touch the existing code
> *at all*, and we write only a single line of new code. For the actual
> message passing it might be possible to reuse the existing JavaScript
> messageManager code, perhaps.
If you could somehow model it entirely using asynchronous messaging
chrome->content, sure. But you'd still have the same set of problems
cleaning up handles.
--BDS
aza...@mozilla.com
> > Approach A
> > ==========
>
> > 1. Write a new IPDL protocol, which involves creating several new
> > files and adding some code to several existing ones.
> > 2. Edit existing code with things like this:
>
> > #ifdef MOZ_IPC
> > if (we are in a child process)
> > send a message using the new protocol we created, which gets the
> > parent to do the actual work
> > else
> > #else
> > do the actual work
> > #endif
>
> > The downside here is that adding a new protocol involves a relatively
> > nontrivial amount of new code, especially if all we actually want is
> > to remote a few function calls from the child to the parent. In
> > addition, we must modify existing code, scattering #ifdefs in it and
> > making it messier.
>
> Approach A-2 is similar, but instead of putting the #ifdef/if in each
> method, we have two implementations of the XPCOM class. At createInstance
> time, we choose which implementation to use.
>
Interesting, is there currently an example of this in the codebase?
Got it. Thanks for the explanation.
>
>
> > Approach C
> > ==========
>
> > Again, assume we want to remote the usage of an XPCOM component, and
> > performance is not terribly important. Assume also that the XPCOM
> > component happens to be written in JavaScript (that is the case with
> > InstallTrigger). Then we can imagine the following method:
>
> > function component() { ... } // the old component-
> > component.prototype = { ... } // defining code, untouched
>
> > electrolify(component);
>
> > And the electrolify() function will, if in a child process, replace
> > methods so that when called they send messages; and if in a parent
> > process, set up the code to handle answering those messages.
>
> > The benefit of this approach is that we do not touch the existing code
> > *at all*, and we write only a single line of new code. For the actual
> > message passing it might be possible to reuse the existing JavaScript
> > messageManager code, perhaps.
>
> If you could somehow model it entirely using asynchronous messaging
> chrome->content, sure. But you'd still have the same set of problems
> cleaning up handles.
>
Well, what I was thinking of was to basically use the existing
messageManager code, which makes sending messages content<-->chrome
in JS UI code as simple as e.g.
Browser.messagePasser.addMessageListener("FennecLocationChange",
this);
[..]
receiveFennecLocationChange: function
receiveFennecLocationChange(message, location) {
[..]
sendMessage("FennecLocationChange", location);
This doesn't have problems with handles, is a very small amount of
code, etc., so it is appealing I think. It uses sync messages
from content to chrome, and async from chrome to content (which
would be enough for many purposes, in particular the ones I have
in mind, like the addon manager).
The difference is that I would like to use such functionality in
other places than where the messageManager is currently used (which
is in the UI code, browser.js and frame-content.js). I'd like to use
it from, say, an XPCOM component implemented in JavaScript - like
the addon manager.
Would it be acceptable for me to write the code to make
that work?
- azakai
Benjamin Smedberg
>> Approach A-2 is similar, but instead of putting the #ifdef/if in each
>> method, we have two implementations of the XPCOM class. At createInstance
>> time, we choose which implementation to use.
>>
>
> Interesting, is there currently an example of this in the codebase?
http://mxr.mozilla.org/projects-central/source/electrolysis/chrome/src/nsChromeFactory.cpp#64
> This doesn't have problems with handles, is a very small amount of
> code, etc., so it is appealing I think. It uses sync messages
> from content to chrome, and async from chrome to content (which
> would be enough for many purposes, in particular the ones I have
> in mind, like the addon manager).
>
> The difference is that I would like to use such functionality in
> other places than where the messageManager is currently used (which
> is in the UI code, browser.js and frame-content.js). I'd like to use
> it from, say, an XPCOM component implemented in JavaScript - like
> the addon manager.
>
> Would it be acceptable for me to write the code to make
> that work?
Yes. Let's work out the details, I'll be in MV next week and we can talk in
person!
--BDS
aza...@mozilla.com
> On 5/14/10 1:04 PM, aza...@mozilla.com wrote:
>
> >> Approach A-2 is similar, but instead of putting the #ifdef/if in each
> >> method, we have two implementations of the XPCOM class. At createInstance
> >> time, we choose which implementation to use.
>
> > Interesting, is there currently an example of this in the codebase?
>
>
> > This doesn't have problems with handles, is a very small amount of
> > code, etc., so it is appealing I think. It uses sync messages
> > from content to chrome, and async from chrome to content (which
> > would be enough for many purposes, in particular the ones I have
> > in mind, like the addon manager).
>
> > The difference is that I would like to use such functionality in
> > other places than where the messageManager is currently used (which
> > is in the UI code, browser.js and frame-content.js). I'd like to use
> > it from, say, an XPCOM component implemented in JavaScript - like
> > the addon manager.
>
> > Would it be acceptable for me to write the code to make
> > that work?
>
> Yes. Let's work out the details, I'll be in MV next week and we can talk in
> person!
>
> --BDS
Sounds good!
Meanwhile I filed a bug to organize discussion about this:
https://bugzilla.mozilla.org/show_bug.cgi?id=566024
- azakai
aza...@mozilla.com
Update: I posted a working patch to the bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=566024#c1
See the comments included with the patch - it isn't meant as a serious
solution yet, but it does show the general direction, and the
motivation. Comments and feedback in the bug would be very
appreciated.
- azakai