270893 - Freeze nsIArray

Status: RESOLVED FIXED

(Core Graveyard :: Embedding: APIs, defect, P2)

Description

[jhp (no longer active)]

Freeze nsIArray.

According to http://www.mozilla.org/projects/embedding/HowToFreeze.html, we
should have only one interface per IDL file.  So this needs to be split into
nsIArray.idl and nsIMutableArray.

Comment 1

[Benjamin Smedberg]

Attachment: Separate nsIMutableArray.idl and cleanup, rev. 1

This separates nsIMutableArray into its own IDL; moves the comptr-helpers to the glue; removes the NS_NewArray internal methods which are either poorly used or easy to replace using createinstance; fixes up treewide #includes and usage of NS_NewArray.

Comment 2

[Darin Fisher]

Comment on attachment 218099 [details] [diff] [review]
Separate nsIMutableArray.idl and cleanup, rev. 1

>Index: gfx/src/x11shared/nsFT2FontCatalog.cpp

>+  nsCOMPtr<nsIMutableArray> entries =
>+    do_CreateInstance(NS_ARRAY_CONTRACTID);
>   if (!entries)
>     return NS_ERROR_OUT_OF_MEMORY;

Well, that's not exactly true.  I see several other cases where
do_CreateInstance failure is treated as an OOM condition.


>Index: mailnews/base/src/nsMsgFolderDataSource.cpp

>   nsresult rv;
> 
>   nsCOMPtr<nsIMsgFolder> folder(do_QueryInterface(source, &rv));
>   if (NS_FAILED(rv)) return rv;
> 
>-  nsCOMPtr<nsIMutableArray> cmds;
>-  NS_NewArray(getter_AddRefs(cmds));
>+  nsCOMPtr<nsIMutableArray> cmds =
>+    do_CreateInstance(NS_ARRAY_CONTRACTID);
>   if (!cmds) return rv;

This seems to return a success code if it cannot instantiate the array.
Hmm...


>Index: modules/oji/src/nsJVMConfigManagerUnix.cpp

>+    nsCOMPtr<nsIMutableArray> array =
>+        do_CreateInstance(NS_ARRAY_CONTRACTID);
>+    NS_ENSURE_TRUE(array, NS_ERROR_OUT_OF_MEMORY);

OOM, really?

NS_ENSURE_STATE(array) might be better.


>Index: xpcom/ds/nsIMutableArray.idl

>+ */
>+
>+[scriptable, uuid(af059da0-c85b-40ec-af07-ae4bfdc192cc)]

Please kill the gap here.  It causes doxygen trouble.


r=darin

Comment 3

[Benjamin Smedberg]

Fixed on trunk, with a couple additional platform- or configuration-specific bustage fixes.

Comment 4

[Ben Turner (not reading bugmail, use the needinfo flag!)]

This checkin broke XULRunner on Windows (at least). I know our tinderbox (solaria) has been busted for a while but you can see that the reason for the bustage switched right after this was checked in.

From http://tinderbox.mozilla.org/showlog.cgi?log=XULRunner/1144862700.16448.gz -

c:/builds/tinderbox/XR-Trunk/WINNT_5.2_Depend/mozilla/toolkit/library/dlldeps.cpp(61) : fatal error C1083: Cannot open include file: 'nsArray.h': No such file or directory

I'm also seeing this on my machine.

P.S. What's the deal with solaria being busted for so long? Is that a cairo thing?

Comment 5

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(In reply to comment #4)
> This checkin broke XULRunner on Windows (at least).

bsmedberg patched the dlldeps.cpp file that was broken.

http://tinderbox.mozilla.org/bonsai/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/xpcom/build&command=DIFF_FRAMESET&file=dlldeps.cpp&rev1=1.147&rev2=1.148&root=/cvsroot

Comment 6

[Jon Henry]

I believe this checkin also broke the spatial navigation extension, please see bug 333873.