198685 - need nsCOMArray function like IndexOf that checks COM object identity

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Benjamin Smedberg]

nsCOMArray::IndexOf does not check COM object identity. This may be desired in
certain applications, such as implementing tearoffs, etc.  But there should be a
method that does check for COM object identity so that you don't get a "false
negative" when checking for an object.

Reasoning: for any given COM object (aObject), you can have multiple interfaces
nsIFoo of that object with *different* pointers (e.g. some tearoff
implementations, multiple XPConnect wrappers). Because IndexOf() does a "blind"
pointer comparison, it can't be used to check object identity.

I have implemented a similar function IndexOfObject() that performs the QI to
nsISupports that's necessary to check for object identity. 

Patch forthcoming.

Comment 1

[Benjamin Smedberg]

Attachment: add IndexOfObject to nsCOMArray

Comment 2

[Alec Flett]

well first of all I don't see how this can compile since you have 2 IndexOf()s
with the same signature.

secondly, is there a bug that this depends on? who is breaking because of this?
I'm really reluctant to just add this blindly and have people somehow think that
this is what they should be using.

Comment 3

[Benjamin Smedberg]

Attachment: add IndexOfObject to nsCOMArray, fixed

Maybe I should compile patches before posting them... ;)

Yes, this is needed for nsBookmarksService.cpp (see bug 191783).
http://lxr.mozilla.org/seamonkey/source/xpfe/components/bookmarks/src/nsBookmarksService.cpp#5081

is doing an object-identity check that is COM-unsafe. This call is used to
prevent infinite recursion writing bookmarks.

I can see uses of nsCOMArray::IndexOf (such as maintaining a list of active
tearoffs in an interface implementation), but I would argue that most of the
time, people actually want to check object identity, not interface identity,
and IndexOfObject is the correct call. Yes, there's a performance hit there,
but it doesn't really matter in my case that much, and if it really matters,
you should be using something else like a hash anyway.

The same thing goes for nsCOMArray::RemoveObject, but since I'm not using that
I'll let somebody else worry about it.

Comment 4

[Alec Flett]

Comment on attachment 118137 [details] [diff] [review]
add IndexOfObject to nsCOMArray, fixed

ok then... sr=alecf if you add a comment in nsCOMArray.h saying something to
the effect of

// uses QueryInterface to determine actual COM identity of the object
// be careful this is much slower than IndexOf()!

(or if you come up with a better way of phrasing it, that's fine too)

Comment 5

[Benjamin Smedberg]

Checked in 3/25/2003