Say you have a binding <binding id="somewidget" extends="lalala"> <implementation implements="nsIFoo, nsIBar"> <property name="foo"> <getter> /* Do some stuff */ </getter> ... Once you QI a content node to nsIFoo, you can no longer QI the same content node to nsIBar, and vice versa. You can do one or the other, but not both.

I tested the patch, and it works. Basically, in this part of the code supportsInterface is always true, and the existence of a wrappedJS object for the content node does not necessarily mean we have wrapped all of the possible interfaces, although it's being treated that way. We still need to fall through if wrappedJS->AggregatedQueryInterface(aIID, aResult) fails, to add another entry to the wrappedJS table, since there at this point we're looking to bind another interface to the same content node. If there's potential performance degredation, I'm not seeing it. The fall through doesn't happen in normal execution afaict. It would only happen in places where multiple interfaces are supported - so far that only happens in code proposed for bug 82207.

Cc'ing jband. /be

This looks to me like it *might* cause problems in the binding manager. Though from the xpconnect perspective it looks OK. The issue is that a given wrappedJS only lives as long as someone holds a reference on it (in the C++ AddRef sense). By making a second wrapper for the same aContent and calling SetWrappedJS you displace the original entry in mWrapperTable and that wrapper is released. The comment before the declaration of mWrapperTable makes that sound like it may have bad side effects. But the comment might not be *exactly* correct. Still, this may not really be a problem. I believe the diagram below represents the relationships correctly... DOM element ^ | WrappedNative <-- JSObject of WrappedNative ^ ^ | | WrappedJS-Foo WrappedJS-Bar I *think* the key here is the lifetime of the "JSObject of WrappedNative" this is the JSObject that xbl has hacked up. Its lifetime is assured by *any* WrappedJS having a reference to it. So, it *may* be that the only cost of letting WrappedJS-Bar displace WrappedJS-Foo in mWrapperTable is the fact that we'll have to go to the trouble of creating a replacement WrappedJS-Foo next time someone asks (thus displacing WrappedJS-Bar in mWrapperTable). Or it may be that something scarier happens. In that case you'd need a better mechanism for controling the lifetimes of these WrappedJS's. Time for hyatt to render an opinion...

Seems like we should store all wrappers for efficiency's sake, right? Seems like we could modify GetWrappedJS and SetWrappedJS on binding manager to take an IID arg. Then we could hand back specific wrappers and not end up recreating the same wrapper over and over again.

Time to pass this one on.

Fixed.

hyatt: what about your 2001-08-23 12:26 comment? I didn't see that addressed in what you checked in. Want to tackle that optimisation at a later time?