Remove nsISupportsObsolete.h

RESOLVED FIXED in mozilla38

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: poiru, Assigned: poiru)

Tracking

Trunk
mozilla38
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 1 obsolete attachment)

There is only one more use of a nsISupportsObsolete.h macro in mozilla-central. froydnj, do we want to remove nsISupportsObsolete.h?

If so, we need to take care of uses in comm-central as well. I volunteer to expand the macros in comm-central, but the Thunderbird devs might prefer to copy nsISupportsObsolete.h into comm-central instead.
Flags: needinfo?(nfroyd)
The NS_IMPL_CLASS_* macros look useful, but if we haven't been using them, then they're not worth keeping around.  Let's remove the file.
Flags: needinfo?(nfroyd)
jcranmer, would you prefer to expand the macros in comm-central (I can do that) or to move nsISupportsObsolete.h into comm-central?
Flags: needinfo?(Pidgeot18)
Replacing the macros seems the better way to go. Although note that NS_FREE_XPCOM_ISUPPORTS_POINTER_ARRAY is also defined in nsMemory.h, so there's probably no need to rewrite those macros.
Flags: needinfo?(Pidgeot18)
Assignee: nobody → birunthan
Status: NEW → ASSIGNED
Attachment #8549053 - Flags: review?(Pidgeot18)
Attachment #8549056 - Flags: review?(nfroyd)
Attachment #8549065 - Flags: review?(nfroyd)
Comment on attachment 8549053 [details] [diff] [review]
Import nsISupportsObsolete.h from mozilla-central (for comm-central)

Review of attachment 8549053 [details] [diff] [review]:
-----------------------------------------------------------------

Half of this file is unused, and most of the rest of the half is used in just one file. I'm going to instead prepare a patch that simply stops using it altogether.
Attachment #8549053 - Flags: review?(Pidgeot18) → review-
Comment on attachment 8549066 [details] [diff] [review]
Remove NS_INIT_ISUPPORTS (for comm-central)

Review of attachment 8549066 [details] [diff] [review]:
-----------------------------------------------------------------

Really? Only one use of a useless API and its only used is in a surprisingly new portion of libmime? I'm honestly surprised we had this few uses.
Attachment #8549066 - Flags: review?(Pidgeot18) → review+
Notes:
1. I added NS_IMPL_GETSET to nsMsgUtils.h because there was enough use of it and I didn't file like a massive copy-paste job.
2. nsNNTPNewsgroupPost got a massive overhaul by changing things to nsCString from char*. Actually, all of those methods are unused, come to think of it. You could replace the interface with nsIFile and probably no one would care.
3. There's a few NS_IMPL_GETTER expansions, because I didn't feel like porting the macro for two or three implementations.
4. Ditto for NS_IMPL_CLASS_GETSET and friends (especially because it's arguably more useful to just use NS_DECL_NSIFOOBAR).
5. I killed off the NS_LOCK_INSTANCE and NS_UNLOCK_INSTANCE. nsNNTPNewsgroupPost isn't threadsafe in the first place, so it makes no sense there; we don't do anything with NNTP off-main-thread and the methods in nsNNTPService.cpp already early-return, so there's no point locking there, even though it's nominally threadsafe.
[threadsafe here means it supports threadsafe AddRef/Release. Neither of those classes are likely to be threadsafe in the least.]
Attachment #8549053 - Attachment is obsolete: true
Attachment #8549364 - Flags: review?(kent)
Comment on attachment 8549364 [details] [diff] [review]
Stop using nsISupportsObsolete in comm-central

Review of attachment 8549364 [details] [diff] [review]:
-----------------------------------------------------------------

Details look good, compiles fine.
Attachment #8549364 - Flags: review?(kent) → review+
Attachment #8549065 - Flags: review?(nfroyd) → review+
Attachment #8549056 - Flags: review?(nfroyd) → review+
https://hg.mozilla.org/mozilla-central/rev/93828c511189
https://hg.mozilla.org/mozilla-central/rev/244f30e89f71
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.