73395 - Implement GetFirstSubFolder() in all sub-classes of nsFolder

Status: VERIFIED WONTFIX

(MailNews Core :: Backend, defect)

Description

[Håkan Waara]

When I looked at bug 72899 I realized that we need a GetFirstSubFolder().

The reason for this is that I've seen several places in the code where you call
GetSubFolders() and then just use/get the first element. GetSubFolders() is
quite memory-intensive, so using a GetFirstSubFolder() instead of
GetSubFodlers() would be a performance boost (I don't know how big though) and
would surely optimize some of the mailnews code.

I think we should implement it in all of the sub-classes of nsFolder, since
nsIFolder.idl has a GetSubFolder(). Should we also implement it in nsIFolder?

This is (very roughly, and maybe also buggy) what I've thought it should look like.

nsresult GetFirstSubFolder(nsIEnumerator** firstFolder)
{
  if(!mSubFolder)
    GetSubFolders(&mSubFolder);

  *firstFolder = mSubFolder->ElementAt(0);

   return NS_OK;
}

Something like that.. Please give comments/feedback on the best way to do this.

Comment 1

[Håkan Waara]

Here comes a patch that implements this for nsMsgFolder. I think everything is
done *except* that I haven't put it in any .idl file yet.

I'm uncertain where (and how!) I should put it. In nsIFolder or each sub-class'
.idl file?

Comment 2

[Håkan Waara]

Attachment: implementation of nsMsgFolder::GetFirstSubFolder(), v1

Comment 3

[Håkan Waara]

Attachment: same as above, v2 (thanks timeless!)

Comment 4

[David :Bienvenu]

this bug is laboring under a misconception - creating an enumerator is not
memory intensive operation - it allocates a small single chunk of memory, and
that's about it. Plus, I don't see many places where we do this at all - just
one, in fact, and that place is just trying to find out if there are subfolders,
not what the first sub-folder is.

Comment 5

[Håkan Waara]

Bienvenu, does this mean that you think that it is unnecessary to implement this
API?  Should we mark it WONTFIX?

I would also want to hear Seth's voice on this.

Comment 6

[(not reading, please use [e-mail] instead)]

bienvenu is right.  don't add GetFirstSubFolder, we don't need it.

see my comments in #72899 for what I think we do need.

hwaara, you should mark this wontfix.

Comment 7

[Håkan Waara]

I will instead get my hands dirty on bug 72899.

Please VERIFY.

Comment 8

[(not reading, please use [e-mail] instead)]

verified.