Remove Utility Function usage, create using XPCOM in mailnews

RESOLVED FIXED

Status

MailNews Core
Backend
RESOLVED FIXED
10 years ago
5 years ago

People

(Reporter: Nick Kreeger, Assigned: Nick Kreeger)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

10 years ago
To help with the push to frozen linkage, mailnews needs to get rid of utility function calls for creating objects.

For example, there are many occurrences of |NS_NewISupportsArray|. Instead we should be using |do_CreateInstance(NS_SUPPORTSARRAY_CONTRACTID)|.

I suppose the best way to tackle this it to do it the same way we did the |nsXPIDL*| string removal, one at a time.
(Assignee)

Updated

10 years ago

Updated

10 years ago
OS: Mac OS X → All
Hardware: Macintosh → All
(Assignee)

Comment 1

10 years ago
Created attachment 292162 [details] [diff] [review]
IMAP Fix V1

Here is the cleanup of all the classes in mailnews/imap that use the utility function.
Assignee: nobody → nick.kreeger
Status: NEW → ASSIGNED
Attachment #292162 - Flags: superreview?(bienvenu)

Comment 2

10 years ago
Comment on attachment 292162 [details] [diff] [review]
IMAP Fix V1

thx, Nick. One nit - I'm going to go out on a limb and say that things like this:

+        NS_ENSURE_TRUE(m_allFolders, NS_ERROR_FAILURE);

should return NS_ERROR_OUT_OF_MEMORY instead, or pass in &rv to do_CreateInstance and NS_ENSURE_SUCCESS(rv, rv) to be sure to get the right error code.
Attachment #292162 - Flags: superreview?(bienvenu) → superreview+
(Assignee)

Comment 3

10 years ago
Created attachment 294304 [details] [diff] [review]
Checkin Patch Version [Checked In]

This patch addresses David's comments about getting the nsresult value to return in the NS_ERROR_FAILURE() calls.
Attachment #292162 - Attachment is obsolete: true
(Assignee)

Comment 4

10 years ago
Comment on attachment 294304 [details] [diff] [review]
Checkin Patch Version [Checked In]

Checked into trunk.
Attachment #294304 - Attachment description: Checkin Patch Version → Checkin Patch Version [Checked In]

Comment 5

10 years ago
Actually nsISupportsArray is deprecated, and we should be looking into replacing it with nsI(Mutable)Array where possible.
Product: Core → MailNews Core

Comment 6

5 years ago
Should this stay open? The patch is marked checked in.
Nick why didn't you close the bug ?
Status: ASSIGNED → NEW
(In reply to neil@parkwaycc.co.uk from comment #5)
> Actually nsISupportsArray is deprecated, and we should be looking into
> replacing it with nsI(Mutable)Array where possible.

Is anything more needed?  If yes, should we close this and do meta with new bugs for "removal, one at a time"?

http://mxr.mozilla.org/comm-central/search?find=%2Fmailnews%2Fbase%2F&string=NS_NewISupportsArray ?
Flags: needinfo?
(In reply to Wayne Mery (:wsmwk) from comment #8)
> (In reply to neil@parkwaycc.co.uk from comment #5)
> > Actually nsISupportsArray is deprecated, and we should be looking into
> > replacing it with nsI(Mutable)Array where possible.

i.e. bug 394167.  which still has a couple unresolved bugs blocking it
Flags: needinfo?
mike, comment 8  (since everyone else punts)
Flags: needinfo?(mconley)
My opinion: the goal of this bug is no longer valid, since, as Neil rightly points out, nsISupportsArray is deprecated in favour of nsIMutableArray.

Switching us over to nsIMutableArray seems to be tracked in bug 394167.

My position is to close this bug.
Flags: needinfo?(mconley)
Using FIXED since it had one patch
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.