Here's the reason:' Simon Fraser wrote: > > Simon Fraser wrote: > > > > For most interface IDs, there is no need to add to the code's data size > > by using NS_DEFINE_IID() macros. Instead, you should just call the static GetIID() > > method. > > It seems that none of the transaction interfaces define GetIID(). If you > want to use these with nsCOMPtr, or call the GetIID() method, they will > have to have them. OK, so I just got bit. Take a look at this: class nsIDOMEventListener : public nsISupports { static const nsIID& GetIID(); } class nsIDOMKeyListener : public nsIDOMEventListener { // no GetIID() } If you call nsIDOMKeyListener::GetIID(), you'll get the interface ID of nsIDOMEventListener. Worse still, if none of your interfaces have GetIID() methods, then you'll get the IID of nsISupports. The real danger here comes when you use nsCOMPtr. nsCOMPtr uses GetIID() internally to get the right interface when doing a do_QueryInterface(), so if you make an nsCOMPtr of, say, one of our transactions (which lack GetIID()), you are at risk of getting completely the wrong interface. Bad things can happen. We are using nsCOMPtr for transactions in a bunch of places.

I don't agree. All our transactions inherit from EditTxn. EditTxn implements nsITransaction. So our transaction classes rely soley on nsITransaction::GetIID(), and nsITransaction is the only interface they implement. nsCOMPtr can not be used for differentiating between implementations of an interface. For that, you need a class ID and need to do a specific QueryInterface yourself, without using an nsCOMPtr. So, anywhere in the code that we're using nsCOMPtr<some_transaction_class> is a bug, and adding GetIID() calls is not the answer. Simon, do you agree?

Our transactions misuse interface IDs where they should be using class IDs. We do not have separate interfaces for every transaction; each txn simply implements the transaction interface. This means that you should never use nsCOMPtr on a transaction directly, and we should rename all the tranaction IIDs to CIDs. I don't think I understood this properly when I filed this bug (actually, I don't remember filing the bug).

I cleaned this all up a while ago. Transactions now all have CID's with static accessors. Usage cleaned up throughout editor code. Simon, could you please verify?

Yep, this all looks good.