165574 - PSM embedding freeze/ step 3/ replace nsISupportsArray with nsIArray

Status: VERIFIED FIXED

(Core Graveyard :: Security: UI, defect, P1)

Description

[Kai Engert [:KaiE:]]

Stephane told me, PSM should no longer use nsISupportArray.
Should that change be done before 1.2?

Comment 1

[Alec Flett]

yes! 
its not that you have to stop using nsISupportsArray entirely, its that your
interfaces should not use nsISupportsArray to return lists of objects to
callers. The problem is that embeddors are calling a few of your APIs, and are
getting an nsISupportsArray, which is off-limit to embeddors because we're not
going to freeze it.

Comment 2

[Alec Flett]

(that "yes!" was in response to the question about 1.2 - the sooner we get this
the better, but we absolutely must have it by 1.2beta. Thanks :)

Comment 3

[Kai Engert [:KaiE:]]

nsISimpleEnumerator is a very simple interface. Please see my comments in bug
162115. If possible, we should introduce and freeze a simpler array interface,
to avoid unnecessary copying.

Comment 4

[Kai Engert [:KaiE:]]

Attachment: Patch v1

Comment 5

[Kai Engert [:KaiE:]]

The attached patch implements the changes to PSM, that removes nsISupportsArray
from all interfaces, and uses Alec's new array classes instead.

Please review.

Comment 6

[Javier Delgadillo]

Comment on attachment 101631 [details] [diff] [review]
Patch v1

r=javi

Comment 7

[Alec Flett]

Comment on attachment 101631 [details] [diff] [review]
Patch v1

mostly looks good, a few issues:

    * Get a list of Crl entries in the DB.
    */
-  nsISupportsArray getCrls();
+  nsIMutableArray getCrls();

Do you really want this to be mutable? Shouldn't this be nsIArray instead?
see:
http://lxr.mozilla.org/seamonkey/source/xpcom/ds/nsIArray.idl#58
http://lxr.mozilla.org/seamonkey/source/xpcom/ds/nsIArray.idl#129

-  attribute nsISupportsArray ASN1Objects;
+  attribute nsIMutableArray ASN1Objects;

-  nsISupportsArray getChain();
+  nsIMutableArray getChain();

-  nsISupportsArray getOCSPResponders();
+  nsIMutableArray getOCSPResponders();

these too.

Actually, that's the only problem I see...

with those switched to nsIArray, sr=alecf

Comment 8

[Kai Engert [:KaiE:]]

> Do you really want this to be mutable? Shouldn't this be nsIArray instead?

You partially convinced me.

+  nsIMutableArray getCrls();
+  nsIMutableArray getChain();
+  nsIMutableArray getOCSPResponders();
ok, changed to nsIArray

+  attribute nsIMutableArray ASN1Objects;
The object referenced through the interface is passed around and the attribute
gets modified.
This must be mutable, if we don't want to rewrite the code.

Comment 9

[Alec Flett]

oh! that wasn't obvious from the patch, but from looking in LXR I see what you
mean. my sr= stands

Comment 10

[Kai Engert [:KaiE:]]

Attachment: Patch v2

This is the patch having the promised and blessed changes.

Comment 11

[Kai Engert [:KaiE:]]

Comment on attachment 102567 [details] [diff] [review]
Patch v2

marking r=javi from comment 6 and carrying forward sr=alecf

Comment 12

[Asa Dotzler [:asa]]

Comment on attachment 102567 [details] [diff] [review]
Patch v2

a=asa for checkin to 1.2beta (on behalf of drivers)

Comment 13

[Kai Engert [:KaiE:]]

fixed on trunk

Comment 14

[John Unruh]

Verified.