Last Comment Bug 25886 - |NS_DEFINE_IID| is bad; use |NS_GET_IID| instead
: |NS_DEFINE_IID| is bad; use |NS_GET_IID| instead
Status: NEW
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: Trunk
: All All
: P3 normal with 1 vote (vote)
: Future
Assigned To: jag (Peter Annema)
: Nathan Froyd [:froydnj]
: 61624 (view as bug list)
Depends on:
Blocks: 21326
  Show dependency treegraph
Reported: 2000-01-31 09:01 PST by Scott Collins
Modified: 2009-07-24 11:49 PDT (History)
6 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Get rid of a bunch of unused NS_DEFINE_IID, NS_DEFINE_CID (160.75 KB, patch)
2003-07-19 16:33 PDT, jag (Peter Annema)
caillon: review+
dbaron: superreview+
Details | Diff | Splinter Review
Backup. This compiles on Mac. (274.69 KB, patch)
2007-08-16 10:49 PDT, jag (Peter Annema)
no flags Details | Diff | Splinter Review
Backup. This compiles on all the Try servers and Neil's Windows box. (308.08 KB, patch)
2007-08-17 14:00 PDT, jag (Peter Annema)
no flags Details | Diff | Splinter Review

Description Scott Collins 2000-01-31 09:01:08 PST
|NS_DEFINE_IID| is the old space-wasting scheme.  |NS_GET_IID| is the (not so) 
new, accepted, more efficient (on most platforms) scheme.  We need to root out 
all the old instances of |NS_DEFINE_IID| and replace the uses of their defined 
IIDs with |NS_GET_IID(type)|.
Comment 1 Scott Collins 2000-01-31 09:13:15 PST
We've already agreed this is a P1; see

There are thousands of occurances.  I don't think we can get them all by M14; but 
I'll have to do work on them in M14 to get them done by M15.  Probably need to 
file individual bugs on different sections of the code to get this done.
Comment 2 Scott Collins 2000-03-30 09:57:20 PST
Another one of those sweeping changes that fits in between everything else.  
There are still more instances than lxr is willing to display.
Comment 3 Scott Collins 2000-05-15 01:37:53 PDT
mass re-assigning to my new bugzilla account
Comment 4 Dawn Endico 2000-12-21 14:10:02 PST
dp is no longer changing qa contact to default for this product
Comment 5 David Baron :dbaron: ⌚️UTC-10 2001-07-10 20:30:38 PDT
I've been hoping that we could move even further and try to minimize the number
of places where we need to use NS_GET_IID as well.  NS_GET_IID forces the
programmer to specify the type of a variable in two different places and does
not check that the types are the same -- it is therefore not typesafe, and I
think we should prefer the typesafe functions available that will reduce the
risk of serious errors:  the nsCOMPtr helpers (do_QueryInterface,
do_CreateInstance, do_GetService, do_QueryReferent) and the function templates
for out parameters (CallQueryInterface, CallCreateInstance, CallGetService,
CallQueryReferent).  Many of the other uses of NS_GET_IID are in hand-written
QueryInterface methods that would be better written using the NS_IMPL_ISUPPORTSn
Comment 6 jag (Peter Annema) 2003-07-19 16:33:37 PDT
Created attachment 128085 [details] [diff] [review]
Get rid of a bunch of unused NS_DEFINE_IID, NS_DEFINE_CID

I grep'ed my tree for NS_DEFINE_.ID, parsed the output so I had a list of
(filename, cidVariableName), then did a grep -c to get the count of the
variable name in that file. If the count is 1, that means the file only has the
NS_DEFINE_.ID in it, and it can be stripped (unless it's a header, had to do
some manual checking there and undo a few removals). Initially I manually
removed these NS_DEFINE_.IDs and removed a few unneeded #includes in the
process, but that took forever so I wrote a little script to remove it for me
(hence no further #include clean-up).
Comment 7 Brendan Eich [:brendan] 2003-07-19 17:52:46 PDT
jag: great work.

Do we need to deprecate NS_DEFINE_IID harder, or are all the occurrences old?

Comment 8 jag (Peter Annema) 2003-07-19 18:19:02 PDT
I don't know, I could find out. It doesn't seem too hard to completely get rid
of NS_DEFINE_IID, I might go play with that this week.
Comment 9 Christopher Aillon (sabbatical, not receiving bugmail) 2003-07-19 19:15:43 PDT
Comment on attachment 128085 [details] [diff] [review]
Get rid of a bunch of unused NS_DEFINE_IID, NS_DEFINE_CID

Comment 10 Mikael Parknert 2004-02-15 03:56:31 PST
*** Bug 61624 has been marked as a duplicate of this bug. ***
Comment 11 jag (Peter Annema) 2007-08-16 07:49:35 PDT
I have a patch locally that leaves me with this:

ipc/ipcd/extensions/dconnect/src/ipcDConnectService.cpp:static NS_DEFINE_IID(kDConnectStubID, DCONNECT_STUB_ID);
layout/mathml/base/src/nsIMathMLFrame.h:static NS_DEFINE_IID(kIMathMLFrameIID, NS_IMATHMLFRAME_IID);
layout/tables/nsTableRowGroupFrame.cpp:  static NS_DEFINE_IID(kITableRowGroupIID, NS_ITABLEROWGROUPFRAME_IID);
xpcom/proxy/src/nsProxyEvent.cpp:NS_DEFINE_IID(kFilterIID, NS_PROXYEVENT_FILTER_IID);
xpcom/proxy/src/nsProxyEventObject.cpp:NS_DEFINE_IID(kFilterIID, NS_PROXYEVENT_FILTER_IID);
xpcom/reflect/xptcall/tests/eVC4/TestXPTCInvokeInIDE.cpp:static NS_DEFINE_IID(kTheCID, INVOKETESTTARGETINTERFACE_IID);

I've removed them if they were unused, replaced QueryInterface implementations with NS_IMPL_ISUPPORTSx, replaced QueryInterface calls with {do_,Call}QueryInterface, replaced NS_DEFINE_IID() for CIDs with NS_DEFINE_CID(), and replaced the rest with NS_GET_IID()

Oh, and 'coz I couldn't help myself I've thrown in some random clean-up. I don't think there's too much, but I'll remove any you'd rather see in a different bug.

Once my tree finishes building I'll attach a patch.
Comment 12 jag (Peter Annema) 2007-08-16 10:49:55 PDT
Created attachment 276974 [details] [diff] [review]
Backup. This compiles on Mac.
Comment 13 2007-08-16 13:59:49 PDT
Comment on attachment 276974 [details] [diff] [review]
Backup. This compiles on Mac.

ns4xPlugin.cpp(878) : error C2065: 'kCPluginManagerCID' : undeclared identifier
ns4xPlugin.cpp(2284) : error C3861: 'kCPluginManagerCID': identifier not found, even with argument-dependent lookup
make[5]: *** [ns4xPlugin.obj] Error 2
Comment 14 2007-08-16 23:52:00 PDT
Comment on attachment 276974 [details] [diff] [review]
Backup. This compiles on Mac.

nsScriptablePeer.cpp(85) : error C2039: 'nsIScriptable' : is not a member of 'operator``global namespace'''
make[5]: *** [nsScriptablePeer.obj] Error 2
Comment 15 jag (Peter Annema) 2007-08-17 01:51:47 PDT
Oooh, thanks Neil.

Changed all kPluginManagerCIDs in ns4xPlugin.cpp to kCPluginManagerCID, and fixed all three copies of that nsScriptablePeer to have nsIScriptable be nsIScriptablePluginSample.
Comment 16 2007-08-17 02:02:35 PDT
Comment on attachment 276974 [details] [diff] [review]
Backup. This compiles on Mac.

nsOEImport.cpp(589) : error C2065: 'kISupportsIID' : undeclared identifier
make[4]: *** [nsOEImport.obj] Error 2
Comment 17 jag (Peter Annema) 2007-08-17 02:17:17 PDT
Fixed, dropped the QI to nsISupports and instead I'm just AppendElement()ing pID directly.
Comment 18 jag (Peter Annema) 2007-08-17 14:00:47 PDT
Created attachment 277140 [details] [diff] [review]
Backup. This compiles on all the Try servers and Neil's Windows box.

Note You need to log in before you can comment on or make changes to this bug.