> The use of NS_IMETHOD is funky since on win32 this implies
> __stdcall. But that can't be since you use varargs. Does this
> compile on Win32? At least give a warning? Maybe "virtual
> nsresult" would be better?

Oh, thus far I've only compiled it on linux.. but thanks for the heads up, I'll change 
it to virtual nsresult, and see how the other
platforms fare.


> This is really an evil method as far
> as xpcom interfaces are concerned. But, I suppose it needs to
> look like an interface to allow for factory access to it without
> adding linkage dependencies.
>

Right.. that's specifically why I wrote it in C++ and not in IDL. I didn't want there 
to be any confusion that this was XPCOM-safe :)

>
> For "&window_opener_args->child" did you mean "&wargs->child" ?
>

oops, yeah... fixed.

>
> I'm not clear on why the prototype for JSCallerCallback does not
> include the argc, yet the place you seem to be implementing one
> does include argc. You want that in the prototype too, no?
>

oops, broken example.. fixed in my tree now.

I didn't specifically didn't include an argc because there's no way to construct an 
varargs style argument list at runtime, which means
that argc will never change from compile time.

I didn't want to have nsJSCaller try to compute it somehow, and then have people 
depending on it's value, in case it was somehow computed
incorrectly. The consumer of this ought to know argc at compile time, otherwise 
something is very wrong :)

>
> I'm not too concerned about where it goes. Is appshell a general
> enough place? Putting it in xpconnect is ok with some cleanup and
> changes to conform to the local traditions.
>

Sure :)
I started with appshell because this was originally just going to call 
window->OpenDialog() - but then I realized I could make it much
more general so I thought xpconnect might be a good place.

Any other conventions you'd like to see, other than your comments?

                                Alec

Reply via email to