Closed
Bug 1060696
Opened 8 years ago
Closed 7 years ago
Remove nsISupportsObsolete.h
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla38
People
(Reporter: poiru, Assigned: poiru)
Details
Attachments
(4 files, 1 obsolete file)
10.16 KB,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
1.82 KB,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
760 bytes,
patch
|
jcranmer
:
review+
|
Details | Diff | Splinter Review |
30.05 KB,
patch
|
rkent
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(nfroyd)
![]() |
||
Comment 1•8 years ago
|
||
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)
Assignee | ||
Comment 2•8 years ago
|
||
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)
Comment 3•8 years ago
|
||
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 | ||
Comment 4•7 years ago
|
||
Assignee | ||
Comment 5•7 years ago
|
||
Attachment #8549056 -
Flags: review?(nfroyd)
Assignee | ||
Comment 6•7 years ago
|
||
Attachment #8549065 -
Flags: review?(nfroyd)
Assignee | ||
Comment 7•7 years ago
|
||
Attachment #8549066 -
Flags: review?(Pidgeot18)
Comment 8•7 years ago
|
||
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 9•7 years ago
|
||
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+
Comment 10•7 years ago
|
||
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 11•7 years ago
|
||
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+
![]() |
||
Updated•7 years ago
|
Attachment #8549065 -
Flags: review?(nfroyd) → review+
![]() |
||
Updated•7 years ago
|
Attachment #8549056 -
Flags: review?(nfroyd) → review+
Comment 12•7 years ago
|
||
https://hg.mozilla.org/comm-central/rev/cdeb25624d5e Pushed comm-central patches: https://hg.mozilla.org/comm-central/rev/5c7840849e72
Assignee | ||
Comment 13•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/93828c511189 https://hg.mozilla.org/integration/mozilla-inbound/rev/244f30e89f71
Comment 14•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/93828c511189 https://hg.mozilla.org/mozilla-central/rev/244f30e89f71
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in
before you can comment on or make changes to this bug.
Description
•