All users were logged out of Bugzilla on October 13th, 2018

Remove nsIFolder.idl (and nsMsgFolder.*)

RESOLVED FIXED

Status

RESOLVED FIXED
15 years ago
10 years ago

People

(Reporter: neil, Assigned: sspitzer)

Tracking

({memory-footprint, perf})

Trunk
x86
Windows 2000
memory-footprint, perf

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments, 1 obsolete attachment)

(Reporter)

Description

15 years ago
IMHO nsIFolder serves no useful purpose, it just takes up time and space.

Steps to remove nsIFolder:
1. Integrate nsMsgFolder into nsMsgDBFolder (nice, but not essential)
2. Convert all users of nsIFolder to nsIMsgFolder
3. Move nsIFolder definitions into nsIMsgFolder (+ nsMsgFolder.h if not 1.)
4. Remove nsIFolder from the build

Additionally we could review some other interfaces e.g. nsIFolderListener could
use nsIMsgFolder on all of its item parameters.
(Reporter)

Comment 1

15 years ago
Created attachment 131695 [details] [diff] [review]
Change all consumers of nsIFolder (except nsMsgFolder) to use nsIMsgFolder
(Reporter)

Updated

15 years ago
Attachment #131695 - Flags: superreview?(scott)
Attachment #131695 - Flags: review?(bienvenu)

Comment 2

15 years ago
this looks good, r=bienvenu with some nits,

+  *aParentMsgFolder = parent;
+  NS_IF_ADDREF(*aParentMsgFolder);
the assignment can be inside the NS_IF_ADDREF (which is apparently templatized
for this very reason)

same here:
+  *aRootMsgFolder = m_rootFolder;
+  NS_IF_ADDREF(*aRootMsgFolder);

here, please use the prevelant braces style
+  if (path.Length() == 1) {
+    NS_ADDREF(*aFolder = rootFolder);
+    return NS_OK;
+  }

so 
if (path.Length() == 1)
{
}
(Reporter)

Comment 3

15 years ago
Created attachment 131728 [details] [diff] [review]
Updated for review comments and bitrot

As well as those three fixes I also fixed a second use of ) {, plus I deleted
an unused nsIFolder (rather than renaming it to nsIMsgFolder).
(Reporter)

Updated

15 years ago
Attachment #131695 - Attachment is obsolete: true
(Reporter)

Comment 4

15 years ago
Comment on attachment 131728 [details] [diff] [review]
Updated for review comments and bitrot

Transferring requests.
Attachment #131728 - Flags: superreview?(scott)
Attachment #131728 - Flags: review?(bienvenu)
(Reporter)

Updated

15 years ago
Attachment #131695 - Flags: superreview?(scott)
Attachment #131695 - Flags: review?(bienvenu)

Updated

15 years ago
Attachment #131728 - Flags: review?(bienvenu) → review+
(Reporter)

Comment 5

15 years ago
Comment on attachment 131728 [details] [diff] [review]
Updated for review comments and bitrot

Trying different reviewer.
Attachment #131728 - Flags: superreview?(scott) → superreview?(dmose)

Comment 6

15 years ago
Comment on attachment 131728 [details] [diff] [review]
Updated for review comments and bitrot

sr=Henry
Attachment #131728 - Flags: superreview?(dmose) → superreview+
(Reporter)

Comment 7

15 years ago
Created attachment 135414 [details] [diff] [review]
Change consumers of nsMsgFolder to use nsMsgDBFolder
(Reporter)

Updated

15 years ago
Attachment #135414 - Flags: superreview?(Henry.Jia)
Attachment #135414 - Flags: review?(bienvenu)

Updated

15 years ago
Attachment #135414 - Flags: review?(bienvenu) → review+

Updated

15 years ago
Attachment #135414 - Flags: superreview?(Henry.Jia) → superreview+
(Reporter)

Comment 8

15 years ago
Created attachment 135472 [details]
"Diff" to aid review of the next patch

The next patch removes nsMsgFolder.cpp and moves all of its methods into
nsMsgDBFolder.cpp, except where they are already overridden. This bloats the
size of the patch, so what I have done here is to concatenate nsMsgDBFolder.cpp
and nsMsgFolder.cpp and rename the functions so you can see there isn't much
difference between the contatenation and the new nsMsgDBFolder.cpp
(Reporter)

Comment 9

15 years ago
Created attachment 135496 [details] [diff] [review]
Supplement to attachment 135414 [details] [diff] [review]

Don't know how I manged to leave these out :-[
(Reporter)

Updated

15 years ago
Attachment #135496 - Flags: superreview?(Henry.Jia)
Attachment #135496 - Flags: review?(bienvenu)

Comment 10

15 years ago
Comment on attachment 135496 [details] [diff] [review]
Supplement to attachment 135414 [details] [diff] [review]

r/sr=bienvenu
Attachment #135496 - Flags: review?(bienvenu) → review+
(Reporter)

Comment 11

15 years ago
Created attachment 135499 [details] [diff] [review]
Delete nsMsgFolder.* and move to nsMsgDBFolder.*

In answer to my rhetorical question, probably the same way that I managed to
leave out the "a" between the "n" and "g" in "manged" ;-)
(Reporter)

Comment 12

15 years ago
Created attachment 135507 [details] [diff] [review]
Delete nsIFolder.idl and move to nsMsgFolder.idl

As an alternative to reviewing the last two patches together, the previous
patch may be reviewed separately if the line NS_DECL_NSIFOLDER is temporarily
added between NS_DECL_NSIMSGFOLDER and NS_DECL_NSICOLLECTION.
(Reporter)

Updated

15 years ago
Attachment #135499 - Flags: superreview?(Henry.Jia)
Attachment #135499 - Flags: review?(bienvenu)
(Reporter)

Updated

15 years ago
Attachment #135507 - Flags: superreview?(Henry.Jia)
Attachment #135507 - Flags: review?(bienvenu)

Updated

15 years ago
Attachment #135507 - Flags: review?(bienvenu) → review+

Comment 13

15 years ago
Comment on attachment 135499 [details] [diff] [review]
Delete nsMsgFolder.* and move to nsMsgDBFolder.*

there's lot of stuff that could be cleaned up (nit-type stuff in the existing
code) but it's all code that was simply moved. Let's just make sure this works
and then we can clean up other stuff later.
Attachment #135499 - Flags: review?(bienvenu) → review+

Updated

15 years ago
Attachment #135496 - Flags: superreview?(Henry.Jia) → superreview+

Updated

15 years ago
Attachment #135499 - Flags: superreview?(Henry.Jia) → superreview+

Updated

15 years ago
Attachment #135507 - Flags: superreview?(Henry.Jia) → superreview+
(Reporter)

Comment 14

15 years ago
Fix checked in.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED

Comment 15

15 years ago
I filled bug 227175 (leak in a collation).
Is it possible that kCollationKeyGenerator is not freed?
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.