I wasted an hour last night trying to figure out how to implement nsIClassInfo. This should be much simpler to figure out.
It doesn't help that the MCD page  is out of date. For instance, it references NS_DECL_CLASSINFO, which no longer exists.
I propose we remove all C++-specific instructions to comments in nsClassInfoImpl.h -- comments in the code have a much higher probability of being updated when the code changes. (Also, speaking for myself, I'm much more likely to find a code comment than an MDC page.) We can put a link to the comments in the wiki page.
I'll make an attempt at writing up this comment, but there's a lot about ClassInfo I don't understand. I'll post some questions in a bit.
cc'ing the usual suspects who can help review
Created attachment 533294 [details] [diff] [review]
There are a few XXX's in the patch which I need help with. bsmedberg, do you know the answers?
Comment on attachment 533294 [details] [diff] [review]
>diff --git a/xpcom/components/nsIClassInfo.idl b/xpcom/components/nsIClassInfo.idl
>- * Provides information about a specific implementation class
>+ * Provides information about a specific implementation class. If you want
>+ * your class to implement nsIClassInfo, see nsIClassInfo.h for instructions --
>+ * you don't (XXX necessarily?) need to inherit from nsIClassInfo.
I'd be even stronger: implementation classes should not inherit from nsIClassInfo.
>diff --git a/xpcom/glue/nsIClassInfoImpl.h b/xpcom/glue/nsIClassInfoImpl.h
>+ * XXX What about the NULL param, the get language helper callbacks?
I really don't know what this does, I think it's used for specialized DOM stuff. jst/mrbkap would know.
jst/mrbkap, thoughts on comment 4?
(In reply to comment #4)
> >+ * you don't (XXX necessarily?) need to inherit from nsIClassInfo.
> I'd be even stronger: implementation classes should not inherit from
I don't known that I'd be that strong :), there's nothing inherently wrong with implementing nsIClassInfo on a class itself, but it's often better to do it on a separate object since you can save yourself a vtable per object, but you do pay a slight performance penalty due to additional malloc overhead if you don't hand back a singleton.
> >+ * XXX What about the NULL param, the get language helper callbacks?
> I really don't know what this does, I think it's used for specialized DOM
> stuff. jst/mrbkap would know.
Yup, this is all for getting from XPCOM object to a scriptable helper, i.e. nsIXPCScriptable in the case of JS (which is the only relevant language here, really). So unless you're doing some very specialized JS stuff, you can ignore this. The only user of this data that I know of is XPConnect.
(In reply to comment #6)
> there's nothing inherently wrong
> with implementing nsIClassInfo on a class itself,
Except that we do treat nsIClassInfo as a singleton, for example in XPConnect, and if nsIClassInfo is implemented in the component itself we often end up holding one instance of that component alive for the duration of the app. See also bug 658632. I'd make the comment say that the component shouldn't implement nsIClassInfo itself unless there is a very specific need for classinfo to be unique per instance. It's probably very rare, XTF does it currently but I'm not convinced that it really needs to.
Created attachment 544825 [details] [diff] [review]
Comment on attachment 544825 [details] [diff] [review]
Review of attachment 544825 [details] [diff] [review]:
@@ +46,5 @@
> + * nsIClassInfo. Implementing nsIClassInfo is particularly helpful if you have
> + * a C++ class which implements multiple interfaces and which you access from
> + * won't have to call QueryInterface on instances of the class; all methods
> + * from all interfaces will be available automagically.
from all interfaces returned from GetInterfaces
Thanks for the review, Peter!
Updated the wiki page, marking dev-doc-complete.