1310040 - Make nsISupportsArray implement nsIArray

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Eric Rahm [:erahm]]

As a stop-gap on the way to full deprecation (and breaking of addons) we'd like to add a path for addons to transition to using nsIArray.

The plan is to make nsISupportsArray implement the nsIArray interface. Then we can happily convert our few remaining interfaces that rely on nsISupportsArray to nsIArray. After a full release cycle we can then remove nsISupportsArray.

Comment 1

[Eric Rahm [:erahm]]

Attachment: Make nsSupportsArray implement nsIArray

This makes nsSupportsArray implement the |nsIArray| interface. This will allow
us to replace references to nsISupportsArray as a param in our gecko interfaces
with nsIArray instead.

The goal is to remove this adapter (and nsISupportsArray) after a full release
cycle.

MozReview-Commit-ID: If7RiO5muIk

Comment 2

[Eric Rahm [:erahm]]

Comment on attachment 8801931 [details] [diff] [review]
Make nsSupportsArray implement nsIArray

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

::: xpcom/ds/nsICollection.idl
@@ +18,5 @@
>    void SetElementAt(in uint32_t index, in nsISupports item);
>    void AppendElement(in nsISupports item);
>    void RemoveElement(in nsISupports item);
>  
> +  [binaryname(DeprecatedEnumerate)]

This clashes with |nsISumpleEnumerator nsIArray.enumerate()| (only on the binary side), so it is renamed.

::: xpcom/ds/nsSupportsArray.cpp
@@ +218,5 @@
> +  return Count(aLength);
> +}
> +
> +NS_IMETHODIMP
> +nsSupportsArray::QueryElementAt(uint32_t aIndex, const nsIID& aIID, void** aResult)

Both nsIArray and nsISupportsArray (via nsICollection) declare this method with identical signatures. Having them share seems to work but the implementation had to be moved out of the header to avoid redefinition.

::: xpcom/tests/unit/test_nsIMutableArray.js
@@ +106,5 @@
>    }
>    do_check_eq(arr.length, i);
>  }
>  
> +function test_nssupportsarray_interop() {

We didn't have xpcshell tests for nsISupportsArray, so this seemed like a reasonable place to add a test for QI'ing an |nsIArray| from an |nsISupportsArray|.

Comment 3

[Eric Rahm [:erahm]]

Note: this can't land until bug 1308317 lands as it depends on simplifications made there.

Comment 4

[Nathan Froyd [:froydnj]]

Comment on attachment 8801931 [details] [diff] [review]
Make nsSupportsArray implement nsIArray

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

Progress!

::: xpcom/ds/nsICollection.idl
@@ +18,5 @@
>    void SetElementAt(in uint32_t index, in nsISupports item);
>    void AppendElement(in nsISupports item);
>    void RemoveElement(in nsISupports item);
>  
> +  [binaryname(DeprecatedEnumerate)]

I think we should add a comment here that is more-or-less a cut-and-paste of your bug comment.

Comment 5

[Kris Maglione [:kmag]]

It might also be a good idea to add a console message about the deprecation when nsSupportsArray is instantiated from JS. But I guess we probably have too much internal code using it at this point...

Comment 6

[Eric Rahm [:erahm]]

(In reply to Kris Maglione [:kmag] from comment #5)
> It might also be a good idea to add a console message about the deprecation
> when nsSupportsArray is instantiated from JS. But I guess we probably have
> too much internal code using it at this point...

I agree, I'd like to mark the whole interface as deprecated (which I think will make it spit out console warnings) but there are still too many internal uses. We're definitely getting close though!

Comment 7

[Eric Rahm [:erahm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ad002143ba47b08d4c65ab58ad4e063f296050d5
Bug 1310040 - Make nsSupportsArray implement nsIArray. r=froydnj

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ad002143ba47