834020 - remove use of nsIEnumerator in mailnews C++ code

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[:aceman]

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.

Comment 1

[:aceman]

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?

Comment 2

[:aceman]

Attachment: patch

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?

Comment 3

[:aceman]

(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? :)

Comment 4

[[e-mail]]

Do I need a patch for nsImapOfflineSync.cpp or is my tree out of date?

Comment 5

[:aceman]

"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

[[e-mail]]

(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 7

[:aceman]

Yes, if you do not need to run the app or mozmill tests.

Comment 8

[[e-mail]]

Comment on attachment 705609 [details] [diff] [review]
patch

r=me pending f=rkent on the JS enumerator change.

Comment 9

[Kent James (:rkent)]

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.

Comment 10

[:aceman]

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.

Comment 11

[:aceman]

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).

Comment 12

[:aceman]

Attachment: patch v2

Comment 13

[:aceman]

Can't checkin until bug 436089 lands :)

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/124128336484

Comment 15

[Ryan VanderMeulen [:RyanVM]]

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

Comment 16

[:aceman]

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

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/fa45b123a36a