Replace idl nsISimpleEnumerator usage with Array<T> in mailnews/base/
Categories
(MailNews Core :: General, task)
Tracking
(thunderbird_esr78 wontfix)
Tracking | Status | |
---|---|---|
thunderbird_esr78 | --- | wontfix |
People
(Reporter: benc, Assigned: benc)
References
(Blocks 1 open bug)
Details
Attachments
(5 files, 1 obsolete file)
7.10 KB,
patch
|
mkmelin
:
review+
|
Details | Diff | Splinter Review |
1.98 KB,
patch
|
mkmelin
:
review+
|
Details | Diff | Splinter Review |
84.26 KB,
patch
|
benc
:
review+
|
Details | Diff | Splinter Review |
21.15 KB,
patch
|
mkmelin
:
review+
|
Details | Diff | Splinter Review |
19.38 KB,
patch
|
mkmelin
:
review+
|
Details | Diff | Splinter Review |
Replacing nsISimpleEnumerator with Array<> (where sensible!)
This covers:
mailnews/base/public/nsIMsgThread.idl
mailnews/base/public/nsIMsgFolder.idl
mailnews/base/public/nsIMsgDBView.idl
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 1•3 years ago
|
||
Tangentially relevant - there's a bunch of unnecessary nsIMsgFolder::GetMessages() implementations which can all be replaced by the default one in nsMsgDBFolder.
try run here, looks OK I think:
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=f0439d22660ea6e8879237498d89a6a7d82261d5
Comment 2•3 years ago
|
||
Comment on attachment 9197677 [details] [diff] [review] [checked in] 1682941-remove-redundant-getMessages-implementations-1.patch Review of attachment 9197677 [details] [diff] [review]: ----------------------------------------------------------------- LGTM, r=mkmelin
Updated•3 years ago
|
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/77cf4b7a44d9
Remove redundant nsIMsgFolder::GetMessages() implementations. r=mkmelin
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 4•3 years ago
|
||
Just snips out a redundant internal use of nsISimpleEnumerator.
(I did remove a comment about avoiding GetSubFolders() to avoid folder discovery, but that was deliberate - I'm trying to disentangle folder discovery from GetSubFolders())
Assignee | ||
Comment 5•3 years ago
|
||
Nasty big monster patch to switch nsIMsgFolder.subFolders from nsISimpleEnumerator to an array.
Comment 6•3 years ago
|
||
Comment on attachment 9203138 [details] [diff] [review] [checked in] 1682941-simplify-nsMsgDBFolder.WriteToFolderCache-1.patch Review of attachment 9203138 [details] [diff] [review]: ----------------------------------------------------------------- Very redundant indeed! r=mkmelin
Comment 7•3 years ago
|
||
Comment on attachment 9203140 [details] [diff] [review] 1682941-remove-nsISimpleEnumerator-from-nsIMsgFolder.subFolders-1.patch Review of attachment 9203140 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, r=mkmelin ::: mailnews/imap/src/nsImapMailFolder.cpp @@ +7831,5 @@ > + nsCOMPtr<nsIMsgImapMailFolder> imapFolder(do_QueryInterface(f, &rv)); > + NS_ENSURE_SUCCESS(rv, rv); > + rv = imapFolder->FindOnlineSubFolder(targetOnlineName, aResultFolder); > + NS_ENSURE_SUCCESS(rv, rv); > + if (*aResultFolder) { It doesn't look like you should check rv first. If rv fails you never get to the if clause. ::: mailnews/local/src/nsPop3IncomingServer.cpp @@ +121,5 @@ > if (NS_SUCCEEDED(rv)) { > + for (nsIMsgFolder* subFolder : subFolders) { > + nsCOMPtr<nsIMsgDatabase> subFolderDB; > + subFolder->GetMsgDatabase(getter_AddRefs(subFolderDB)); > + if (subFolderDB) { could do an `if (!subFolderDB) continue;` here
Assignee | ||
Comment 8•3 years ago
|
||
(In reply to Magnus Melin [:mkmelin] from comment #7)
::: mailnews/imap/src/nsImapMailFolder.cpp
It doesn't look like you should check rv first. If rv fails you never get to
the if clause.
I did keep the rv check in, but have tweaked things to try to clarify the three possible exit conditions (found, not found, error).
Assignee | ||
Updated•3 years ago
|
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/0587ca622bb8
Simplify nsMsgDBFolder::WriteToFolderCache(). r=mkmelin
https://hg.mozilla.org/comm-central/rev/d0c8f7c8585e
Remove nsISimpleEnumerator from nsIMsgFolder.subFolders property. r=mkmelin
Updated•3 years ago
|
Updated•3 years ago
|
Assignee | ||
Comment 10•3 years ago
|
||
Assignee | ||
Comment 11•3 years ago
|
||
(missing explanation for the patch I just uploaded):
Preparation for replacing the nsISimpleEnumerator use in nsIMsgFolder with a custom enumerator (one which returns nsIMsgDBHdr objects rather than generic nsISupport ones).
This patch just switches a bunch of javascript over to use JS-style iterators (for...of) instead of the nsISimpleEnumerator methods (getNext/hasMoreElements).
Not strictly required - my new custom enumerator also supports getNext/hasMoreElements, but I'd like to ditch them eventually.
Try build here:
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=697caced186b721abc8aa294c6fd9d19bf9e38f6
Comment 12•3 years ago
|
||
Comment on attachment 9205306 [details] [diff] [review] [checked in] 1682941-use-JS-iterable-for-nsIMsgFolder-messages-1.patch Review of attachment 9205306 [details] [diff] [review]: ----------------------------------------------------------------- LGTM, r=mkmelin
Assignee | ||
Updated•3 years ago
|
Comment 13•3 years ago
|
||
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/43c38063986f
Use JS iterable protocol instead of nsISimpleEnumerator for nsIMsgFolder.messages access. r=mkmelin
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 14•3 years ago
|
||
(oops - jumped the gun and forgot that this patch was still outstanding!)
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 15•3 years ago
|
||
Comment 16•3 years ago
|
||
Comment on attachment 9205973 [details] [diff] [review] [checked in] 1682941-replace-nsISimpleEnumerator-in-nsIMsgThread-enumerateMessages-1.patch Review of attachment 9205973 [details] [diff] [review]: ----------------------------------------------------------------- r=mkmelin
Updated•3 years ago
|
Comment 17•3 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/6d29f6fff3ad
Replace nsISimpleEnumerator with nsIMsgEnumerator in nsIMsgThread.enumerateMessages(). r=mkmelin
Updated•3 years ago
|
Description
•