Introduction

This does not generally address possible issues with how we use XPCOM and the interfaces, but instead focuses on how we declare interfaces. A much deeper analysis would be required to uncover problems with how interfaces are used (and where XPCOM rules might be being broken; e.g. refcounting errors, casting instead of using QueryInterface, Implementing objects that break XPCOM identity rules, etc).

• C++ default params

  A number of interfaces declare C++ default param values in their method declarations. This can not be done in xpidl and makes no sense outside of in-process direct C++ to C++ calling. It is simply not XPCOM interface compliant.

• Methods that are not declared NS_IMETHOD

  Many methods are accessors methods. The proper way to implement this in XPCOM is to use an out parameter. Admittedly, this makes for an awkward method usage. But this is how XPCOM works. Many interface authors have chosen to cut this corner. They write:

  NS_IMETHOD_(int) GetFoo() = 0;

  or worse...

  virtual int GetFoo() = 0;

  where they should be writing...

  NS_IMETHOD GetFoo(/*out*/ int* foo) = 0;

  The return type of nsresult (hidden in NS_IMETHOD) is critical for XPConnect and future proxy use. It should be used in all public interface unless there is a very strong overriding reason.

  In the (very) few cases where it is not appropriate to return nsresult, it is important that the NS_IMETHOD_() macro be used. Some code, as shown above, does things like virtual int. The NS_IMETHOD_() macro does more than expand to virtual, it also declares the correct calling convention on some platforms. It should be used. I don't think it is ever appropriate for the keyword virtual to appear in an XPCOM interface header.

  Some code tries to be tricky by returning semantically meaningful results in the nsresult...

  NS_IMETHOD IsEnabled() = 0;

  Users are expected to interpret a return value of NS_OK to mean one thing and any failure code to mean another. This pattern also will break down when used in XPConnect or across a proxy. The proxy uses the failure code for its own purposes and the intended result may not be passed through as expected. Don't do this. Use out params.

• Non-XPCOM interface listeners

  There are a few pieces of code that declare listener interfaces where the listener interfaces are not XPCOM interfaces. If these listeners were declared as XPCOM interfaces then listeners could be implemented using XPConnect.

• C++ templates in XPCOM interface declarations

  nsIFrameReflow does this. I don't know why.

• C++ operator overloading in XPCOM interface declarations

  nsITokenObserver does this. I don't know why.

• varargs

  One or two interfaces use '...' params. This is not valid XPCOM. We've discussed possible workarounds. I don't know what replacement for this will be decided upon.

• Marking a method const

  This is a normal C++ idiom to indicate that calling the given method will not modify the object. XPCOM has no way to check for or enforce this.

• Marking a parameter const

  This is not directly expressable in xpidl. The xpidl compiler may well mark pointer/reference type in params as const in the generated headers.

• Use of C arrays

  Currently we have no way to represent arrays in xpidl or map them to XPConnect. This needs to be addressed as some point. It is recommended that XPCOM wrapper interfaces be declared and implemented to manipulate them in the mean time.

• Use of C structs and non-XPCOM classes (and void*)

  Currently we have no way to represent structs or non-XPCOM classes in xpidl or map them to XPConnect. This needs to be addressed as some point. It is recommended that XPCOM wrapper interfaces be declared and implemented to manipulate them in the mean time.

  Mike Shaver plans to implement a native modifier in xpidl to at least let pointers to non-XPCOM types appear in idl and the generated headers. However, interfaces using these native params will not be reflectable into XPConnect and we would have no way to build proxies for them in the future without also declaring the struct layout and details in some future incarnation of xpidl.

• Failure to use the shared allocator

  XPCOM has some serious rules about transfer of ownership of pointer type objects when used in out and in/out params. Following these rules is inconvenient, but not following them puts one on the road to memory leaks and crashes. Given:

  NS_IMETHOD GetName(/*out*/ char** name) = 0;

  By the rules, the implementor of this method should be using the shared allocator (nsIAllocator in our system) to make a copy of name to hand to the caller. The caller then takes ownership of the string and uses the shared allocator to free it when it chooses. Any other scheme is not generally safe. The obvious (unsafe) implementation of the method is:

  NS_IMETHODIMP clazz::GetName(/*out*/ char** name) {
      *name = mName;
      return NS_OK;
  }
  

  This supposes both that the caller will not use the returned pointer past the lifetime of the callee object and that the callee is implemented such that it should retain ownership of the string returned to all callers; i.e. what if the semantics of the implementation where to have the callee return a string that was different on each call ("string1" then "string2"...), would we expect the callee to manage and free all of those strings upon its destruction?

  Passing a pointer type as an out (or in/out) param without transferring ownership implies a contract that is not expressed in the XPCOM interface. This makes it dangerous. However, while it may be that the contract is understood by the users of the interface, we currently have no way to communicate that contract to XPConnect. Given that this pattern is so prevalent and I can't realistically anticipate that the pattern will go away, I intend to propose an xpidl keyword to be used to annotate such params so that XPConnect can know which ones do not follow the standard transfer of ownership rules. Brendan suggests the keyword: 'weak'. I'm thinking 'shared'.

• STRINGS

  The entire string issue remains a big mess in my eyes. We have a number of ways in the code to refer to strings: char* and nsString in a bunch of variations. We may well have an interface coming along too. Strings are the biggest example of the Failure to use the shared allocator problem mentioned above. We need to agree on a safe set of string representations that will be used for communication of this data across XPCOM interfaces. We are just not there yet.

• C++ references

  (e.g. char*&) This C++ism is not representable in xpidl. The XPCOM typelib spec allows for representing this in the sense that it is implemented in binary as a pointer with the added guarantee that it can not hold a NULL. Mappings like XPConnect can impose (and honor) this guarantee as it pertains to in params, but I can't see that it is fully safe for out params. This is a really C++ only idiom and I don't think it really belongs in XPCOM interfaces.

• Passing 'big' things by value

  C++ allows for passing structs by value. This is generally a mistake, though some structs really fit in 32 or 64 bit 'slots'. Still this is not 'good' XPCOM interface behavior.

• Use of #define for constants

  Converting these to enum will allow the xpidl compiler to capture them, reflect them into the generated headers, and, importantly, reflect them into XPConnect so that these name/value pairs will be accessible to JavaScript code using the interface.

• Global enums

  We do not have a way to reflect top level enums into XPConnect. Making use of the interface declaration as a namespace will both allow us to reflect them into XPConnect and also reduce global namespace pollution. I encourage people to do this wherever possible.

• Mixing XPCOM interface declarations with other code

  A lot of header files declare XPCOM interfaces in the same file where 'other' stuff is declared. As we move to xpidl it will make more sense to separate these.