Closed
Bug 834020
Opened 12 years ago
Closed 12 years ago
remove use of nsIEnumerator in mailnews C++ code
Categories
(MailNews Core :: Backend, defect)
MailNews Core
Backend
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 21.0
People
(Reporter: aceman, Assigned: aceman)
References
()
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
aceman
:
review+
|
Details | Diff | Splinter Review |
nsIEnumerator seems obsolete per https://developer.mozilla.org/en-US/docs/XPCOM_array_guide.
Looks like there are only a few occurrences left:
/mailnews/addrbook/src/nsAddrDatabase.cpp (View Hg log or Hg annotations)
line 2697 -- // nsIEnumerator methods:
/mailnews/addrbook/src/nsAddrDatabase.h (View Hg log or Hg annotations)
line 15 -- #include "nsIEnumerator.h"
line 337 -- nsresult CreateCardsForMailList(nsIMdbRow *pListRow, nsIEnumerator **result);
/mailnews/base/src/nsMessengerOSXIntegration.mm (View Hg log or Hg annotations)
line 681 -- nsCOMPtr<nsIEnumerator> enumerator;
/mailnews/base/src/nsMessengerWinIntegration.cpp (View Hg log or Hg annotations)
line 790 -- nsCOMPtr<nsIEnumerator> enumerator;
/mailnews/base/util/iteratorUtils.jsm (View Hg log or Hg annotations)
line 53 -- * nsIEnumerator
line 101 -- // Now try nsIEnumerator
line 102 -- if (aEnum instanceof Ci.nsIEnumerator) {
/mailnews/base/util/nsMsgUtils.h (View Hg log or Hg annotations)
line 11 -- #include "nsIEnumerator.h"
/mailnews/compose/src/nsMsgCompose.cpp (View Hg log or Hg annotations)
line 4690 -- nsCOMPtr<nsIEnumerator> enumerator;
/mailnews/db/msgdb/src/nsMsgDatabase.cpp (View Hg log or Hg annotations)
line 18 -- #include "nsIEnumerator.h"
/mailnews/imap/src/nsImapMailFolder.cpp (View Hg log or Hg annotations)
line 14 -- #include "nsIEnumerator.h"
/mailnews/imap/src/nsImapOfflineSync.h (View Hg log or Hg annotations)
line 17 -- #include "nsIEnumerator.h"
line 67 -- nsCOMPtr <nsIEnumerator> m_serverEnumerator;
/mailnews/import/src/nsImportService.cpp (View Hg log or Hg annotations)
line 19 -- #include "nsIEnumerator.h"
/mailnews/import/src/nsImportService.h (View Hg log or Hg annotations)
line 15 -- #include "nsIEnumerator.h"
/mailnews/local/src/nsLocalMailFolder.cpp (View Hg log or Hg annotations)
line 21 -- #include "nsIEnumerator.h"
/mailnews/news/src/nsNewsDownloader.h (View Hg log or Hg annotations)
line 17 -- #include "nsIEnumerator.h"
line 119 -- nsCOMPtr <nsIEnumerator> m_serverEnumerator;
/mailnews/news/src/nsNewsFolder.cpp (View Hg log or Hg annotations)
line 55 -- #include "nsIEnumerator.h"
Some of them get naturally away via the nsISupportsArray conversions in other bugs.
Looks like many of those are just stray leftover includes.
One open topic I see is if we can remove support for nsIEnumerator from /mailnews/base/util/iteratorUtils.jsm too. Or do we leave it in for some time until extensions stop using it too? Can anybody check if any TB/SM extensions are using it?
After applying patches in bug 436089 and bug 833988 and this patch, I see no traces of nsIEnumerator left in c-c. What do you think?
Attachment #705609 -
Flags: review?(neil)
Attachment #705609 -
Flags: feedback?(kent)
(In reply to :aceman from comment #1)
> Looks like many of those are just stray leftover includes.
> One open topic I see is if we can remove support for nsIEnumerator from
> /mailnews/base/util/iteratorUtils.jsm too.
Ok, I think we should leave it in until we kill nsISupportsArray first, as there are still some spots where we call .enumerate() on some arrays (in JS) and those could produce nsIEnumerator if an array was nsISupportsArray. Or is somebody up to review what type those arrays are? :)
Depends on: 394167
Comment 4•12 years ago
|
||
Do I need a patch for nsImapOfflineSync.cpp or is my tree out of date?
"patches in bug 436089 and bug 833988 and this patch" applied in that order worked for me yesterday. Bug 436089 changes nsImapOfflineSync.cpp.
Comment 6•12 years ago
|
||
(In reply to aceman from comment #5)
> "patches in bug 436089 and bug 833988 and this patch" applied in that order
> worked for me yesterday. Bug 436089 changes nsImapOfflineSync.cpp.
Actually I guess I only need the C++ attachments to compile this patch.
Comment 8•12 years ago
|
||
Comment on attachment 705609 [details] [diff] [review]
patch
r=me pending f=rkent on the JS enumerator change.
Attachment #705609 -
Flags: review?(neil) → review+
Comment 9•12 years ago
|
||
Comment on attachment 705609 [details] [diff] [review]
patch
createCardsForMailList in not xpcom, so it does not show in addons.
The only uses that I see of nsIEnumerator in mail-related addons are of the nature:
else if (iter instanceof Components.interfaces.nsIEnumerator) { // TB 2
or otherwise checks for old versions of TB. As long as the underlying interface continues to exist so that Components.interfaces.nsIEnumerator doesn't fail, then they should be fine.
Attachment #705609 -
Flags: feedback?(kent) → feedback+
Assignee | ||
Comment 10•12 years ago
|
||
I remove CreateCardsForMailList because it looks like an unused define (only in .h, but no implementation).
I understand what you are saying. But I think there may be hidden usages of nsIEnumerator in core and in extensions (e.g. nsISupportsArray.enumerate()).
So I'll leave the support in iteratorUtils.jsm for now. I make a new bug to remove it after we remove more of nsISupportsArray from c-c.
No longer depends on: 394167
Summary: remove use of nsIEnumerator in mailnews → remove use of nsIEnumerator in mailnews C++ code
Assignee | ||
Comment 11•12 years ago
|
||
Actually I think we need to keep it until all of nsISupportsArray and nsIEnumerator are removed from c-c AND m-c (which may be harder).
Assignee | ||
Comment 12•12 years ago
|
||
Attachment #705609 -
Attachment is obsolete: true
Attachment #706595 -
Flags: review+
Keywords: checkin-needed
Keywords: checkin-needed
Comment 14•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 21.0
Comment 15•12 years ago
|
||
Backed out for build bustage.
https://hg.mozilla.org/comm-central/rev/c0d255b60629
https://tbpl.mozilla.org/php/getParsedLog.php?id=19494063&tree=Thunderbird-Trunk
In file included from ../../../../mailnews/news/src/nsNntpService.cpp:41:0:
../../../../mailnews/news/src/nsNewsDownloader.h:118:13: error: 'nsIEnumerator' was not declared in this scope
../../../../mailnews/news/src/nsNewsDownloader.h:118:26: error: template argument 1 is invalid
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Thunderbird 21.0 → ---
Assignee | ||
Comment 16•12 years ago
|
||
Yes, this depends on bug 436089 that was backed out right now and you checked it in before I could remove the checkin-needed :)
Sorry about that.
Comment 17•12 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 21.0
You need to log in
before you can comment on or make changes to this bug.
Description
•