25886 - |NS_DEFINE_IID| is bad; use |NS_GET_IID| instead

Status: RESOLVED FIXED

(Core :: XPCOM, task, P3)

Description

[Scott Collins]

|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]

We've already agreed this is a P1; see
  <http://www.mozilla.org/projects/xpcom/XPCOM_plans.html>

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]

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]

mass re-assigning to my new bugzilla account

Comment 4

[Dawn Endico]

dp is no longer @netscape.com. changing qa contact to default for this product

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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
macros.

Comment 6

[jag (Peter Annema)]

Attachment: 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]]

jag: great work.

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

/be

Comment 8

[jag (Peter Annema)]

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)]

Comment on attachment 128085 [details] [diff] [review]
Get rid of a bunch of unused NS_DEFINE_IID, NS_DEFINE_CID

r=caillon

Comment 10

[Mikael Parknert]

*** Bug 61624 has been marked as a duplicate of this bug. ***

Comment 11

[jag (Peter Annema)]

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)]

Attachment: Backup. This compiles on Mac.

Comment 13

[[e-mail]]

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

[[e-mail]]

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)]

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

[[e-mail]]

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)]

Fixed, dropped the QI to nsISupports and instead I'm just AppendElement()ing pID directly.

Comment 18

[jag (Peter Annema)]

Attachment: Backup. This compiles on all the Try servers and Neil's Windows box.

Comment 19

[Andrew McCreight [:mccr8]]

There are no additional uses of NS_DEFINE_IID in comm-central.

Comment 20

[Andrew McCreight [:mccr8]]

Attachment: Bug 25886 - Remove NS_DEFINE_IID.

There are no remaining uses.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5ea7c27a36a0
Remove NS_DEFINE_IID. r=xpcom-reviewers,nika

Comment 22

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/5ea7c27a36a0