1681009 - Replace idl nsIMutableArray usage with Array<T>

Status: RESOLVED FIXED

(MailNews Core :: General, task)

Description

[Ben Campbell]

This covers:
mailnews/search/public/nsIMsgFilter.idl
mailnews/search/public/nsIMsgSearchSession.idl
mailnews/base/public/nsIMsgDBView.idl
mailnews/extensions/mailviews/nsIMsgMailView.idl

https://searchfox.org/comm-central/search?q=nsIMutableArray&case=true&path=*.idl

Comment 1

[Ben Campbell]

Attachment: 1681009-remove-nsIMutableArray-use-1.patch

The nsIMutableArray use in xpidl interfaces is especially problematic because it means that the caller can mutate the array at any time without the owner being aware of it. In practice, I only found one place which seemed to do that - saveSearchTerms. I'm not too happy with the fix I used there, but without having a bigger-picture view I don't have a feel for what would be the Right Thing(tm). So for now, it'll work and is slightly more explicit than what was already there.

Try run here:
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=3f1f608aeb2283704ae5f5483b0e47fdd0bd7b6f

Comment 2

[Magnus Melin [:mkmelin]]

Comment on attachment 9192248 [details] [diff] [review]
1681009-remove-nsIMutableArray-use-1.patch

Review of attachment 9192248 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. The suite/ changes are kind of not needed but I guess doesn't hurt either.

::: mailnews/base/src/VirtualFolderWrapper.jsm
@@ +28,5 @@
>     * @param aParentFolder The folder in which to create the search folder.
>     * @param aSearchFolders A list of nsIMsgFolders that you want to use as the
>     *     sources for the virtual folder OR a string that is the already '|'
>     *     delimited list of folder URIs to use.
> +   * @param {Array.<nsIMsgSearchTerms>} aSearchTerms - The search terms to

@param {nsIMsgSearchTerms[]}

... I think is better.

::: mailnews/search/content/searchTerm.js
@@ +512,5 @@
>  }
>  
> +/**
> + * Save the search terms from the UI back to the actual search terms.
> + * @param {Array.<nsIMsgSearchTerm>} searchTerms - Array of terms

@param {nsIMsgSearchTerm[]}

Comment 3

[Ben Campbell]

Attachment: 1681009-remove-nsIMutableArray-use-2.patch

(In reply to Magnus Melin [:mkmelin] from comment #2)

• • @param {Array.<nsIMsgSearchTerms>} aSearchTerms - The search terms to
    @param {nsIMsgSearchTerms[]}
    ... I think is better.

Ahh! Thanks - didn't know about that syntax. Much nicer.

Comment 4

[Ben Campbell]

(oops - accidental comment)

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/4e7c4249e3b1
Remove nsIMutableArray use from nsIMsgMailView, nsIMsgFilter and nsIMsgSearchSession. r=mkmelin

Comment 6

[Magnus Melin [:mkmelin]]

Attachment: patch

Found an unused ref to spoil the stats ;)

Comment 7

[Magnus Melin [:mkmelin]]

Attachment: bug1681009_remove_stray_nsIMutableArray.patch

Comment 8

[Ben Campbell]

Comment on attachment 9195852 [details] [diff] [review]
bug1681009_remove_stray_nsIMutableArray.patch

Review of attachment 9195852 [details] [diff] [review]:
-----------------------------------------------------------------

The one that got away :-) But not for long - thanks!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/345c1cc51a98
remove stray nsIMutableArray reference in nsIMsgDBView.idl. r=benc