403894 - Change nsIAddrDatabase::getDeletedCardsList so that it doesn't use nsISupportsArray

Status: RESOLVED FIXED

(MailNews Core :: Address Book, defect)

Description

[Mark Banner (:standard8)]

Currently in nsIAddrDatabase we have:

void getDeletedCardList(out unsigned long aCount, out nsISupportsArray aDeletedList);

Firstly we shouldn't need to return the count (as the array does that for us), secondly, nsISupportsArray is obsolete, thirdly the palm sync code (nsAbPalmHotSync::LoadNewModifiedCardsSinceLastSync) leaks when using this function, and we can fix it at the same time as changing the signature.

The ideal signature in this case seems to be:

readonly attribute nsIArray deletedCardsList;

Comment 1

[Mark Banner (:standard8)]

Attachment: The fix

Changes the GetDeletedCardList function to an attribute of nsIArray.

I've also changed the idl signature of GetDeletedCardCount as it'll be easier for anyone who wishes to access it from js to do so, it just doesn't change the c++ signature.

The nsIAddressBook.idl removal is just a redundant nsISupportsArray forward definition.

Comment 2

[David :Bienvenu]

Comment on attachment 288880 [details] [diff] [review]
The fix

Nice, Mark!

this looks like a place you could use .swap instead:

+
+  NS_IF_ADDREF(*aResult = resultCardArray);
+

Comment 3

[Mark Banner (:standard8)]

(In reply to comment #2)
> this looks like a place you could use .swap instead:
> 
> +
> +  NS_IF_ADDREF(*aResult = resultCardArray);
> +

That doesn't appear to work unfortunately, as resultCardArray is an nsIMutableArray and an aResult is a nsIArray. So we'd have to do a QI or cast or something, which I think is more complicated than is necessary.

Comment 4

[Robert Kaiser]

This apparently broke PalmSync:

Building deps for /cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/mailnews/extensions/palmsync/src/nsAbPalmSync.cpp
/cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/build/cygwin-wrapper cl -FonsAbPalmSync.obj -c  -DMOZILLA_INTERNAL_API -DOSTYPE=\"WINNT5.1\" -DOSARCH=WINNT -DUNICODE -D_UNICODE  -I../../../../dist/include/xpcom -I../../../../dist/include/PalmSyncProxy -I../../../../dist/include/embedcomponents -I../../../../dist/include/pref -I../../../../dist/include/string -I../../../../dist/include/rdf -I../../../../dist/include/rdfutil -I../../../../dist/include/mork -I../../../../dist/include/unicharutil -I../../../../dist/include/MapiProxy -I../../../../dist/include/locale -I../../../../dist/include/mailnews -I../../../../dist/include/msgbase -I../../../../dist/include/msgbaseutil -I../../../../dist/include/necko -I../../../../dist/include/addrbook -I../../../../dist/include/xulapp -I../../../../dist/include/extensions -I../../../../dist/include   -I../../../../dist/include/palmsync -I../../../../dist/include/nspr    -DLCMS_DLL       -GR- -TP -nologo -Zc:wchar_t- -W3 -Gy -Fdpalmsync.pdb  -DNDEBUG -DTRIMMED -O1 -MD            -D_CRT_SECURE_NO_DEPRECATE=1 -D_CRT_NONSTDC_NO_DEPRECATE=1 -DWINVER=0x500 -D_WIN32_WINNT=0x500 -D_WIN32_IE=0x0500 -DX_DISPLAY_MISSING=1 -DMOZILLA_VERSION=\"1.9b2pre\" -DMOZILLA_VERSION_U=1.9b2pre -DHAVE_SNPRINTF=1 -D_WINDOWS=1 -D_WIN32=1 -DWIN32=1 -DXP_WIN=1 -DXP_WIN32=1 -DHW_THREADS=1 -DSTDC_HEADERS=1 -DWIN32_LEAN_AND_MEAN=1 -DNO_X11=1 -D_X86_=1 -DD_INO=d_ino -DMOZ_EMBEDDING_LEVEL_DEFAULT=1 -DMOZ_EMBEDDING_LEVEL_BASIC=1 -DMOZ_EMBEDDING_LEVEL_MINIMAL=1 -DMOZ_SUITE=1 -DMOZ_BUILD_APP=suite -DMOZ_XUL_APP=1 -DMOZ_DEFAULT_TOOLKIT=\"cairo-windows\" -DMOZ_THEBES=1 -DMOZ_CAIRO_GFX=1 -DMOZ_STATIC_MAIL_BUILD=1 -DMOZ_DISTRIBUTION_ID=\"org.mozilla\" -DOJI=1 -DIBMBIDI=1 -DMOZ_VIEW_SOURCE=1 -DACCESSIBILITY=1 -DMOZ_XPINSTALL=1 -DMOZ_JSLOADER=1 -DNS_PRINTING=1 -DNS_PRINT_PREVIEW=1 -DMOZ_NO_XPCOM_OBSOLETE=1 -DMOZ_XTF=1 -DMOZ_CRASHREPORTER=1 -DMOZ_MATHML=1 -DMOZ_ENABLE_CANVAS=1 -DMOZ_SVG=1 -DMOZ_SVG_FOREIGNOBJECT=1 -DMOZ_UPDATE_CHANNEL=default -DMOZ_FEEDS=1 -DMOZ_STORAGE=1 -DMOZ_LOGGING=1 -DMOZ_USER_DIR=\"Mozilla\" -DHAVE_UINT64_T=1 -DMOZ_XUL=1 -DMOZ_PROFILELOCKING=1 -DMOZ_RDF=1 -DMOZ_MORK=1 -DMOZ_DLL_SUFFIX=\".dll\" -DJS_THREADSAFE=1 -DMOZILLA_LOCALE_VERSION=\"1.9a1\" -DMOZILLA_REGION_VERSION=\"1.9a1\" -DMOZILLA_SKIN_VERSION=\"1.8\"  -D_MOZILLA_CONFIG_H_ -DMOZILLA_CLIENT /cygdrive/c/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/mailnews/extensions/palmsync/src/nsAbPalmSync.cpp
nsAbPalmSync.cpp
c:/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/mailnews/extensions/palmsync/src/nsAbPalmSync.cpp(517) : error C2664: 'nsIAddrDatabase::GetDeletedCardList' : cannot convert parameter 1 from 'nsCOMPtr<T> *' to 'nsIArray **'
        with
        [
            T=nsIArray
        ]
        Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast
c:/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/mailnews/extensions/palmsync/src/nsAbPalmSync.cpp(528) : error C2664: 'do_QueryElementAt' : cannot convert parameter 1 from 'nsCOMPtr<T>' to 'nsICollection *'
        with
        [
            T=nsIArray
        ]
        No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called
c:/tinderbox/SeaMonkey-exp/WINNT_5.1_Depend/mozilla/mailnews/extensions/palmsync/src/nsAbPalmSync.cpp(529) : error C2143: syntax error : missing ')' before 'if'

See http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Ports/1195225800.1195228502.10863.gz

Comment 5

[Mark Banner (:standard8)]

(In reply to comment #4)
> This apparently broke PalmSync:

Yep, I've now seen and checked in a fix. I'll check up on it tomorrow.

Comment 6

[Mark Banner (:standard8)]

Bustage now fixed - patch checked in.