233481 - change nsAutoCompleteMdbResult to use nsCOMArray<nsIMdbRow> instead of nsVoidArray

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect)

Description

[Patrick McCormick]

This bug is inspired by bug 221484 and bug 233479.

I noticed that the internal usage of MdbResult is a void array, so references 
to nsIMdbRow objects are not held.  This seems dangerous; what if a row were to 
be deleted while the MdbResult was still in use?

Coming up is a patch to change this from nsVoidArray to nsCOMArray<nsIMdbRow>.  
This removes all the static casts.

Is there a reason why we are not refcounting these objects?

Comment 1

[Patrick McCormick]

Attachment: patch to remove casts in nsAutoCompleteMdbResult.cpp

Comment 2

[Brian Ryner (not reading)]

Comment on attachment 140915 [details] [diff] [review]
patch to remove casts in nsAutoCompleteMdbResult.cpp

I can't find anything in the ownership model that would otherwise guarantee
that the row pointers would remain valid, so r=me.

Comment 3

[Mike Connor [:mconnor]]

patrick, this patch no longer applies cleanly, can you update to tip and attach
an updated patch?

Comment 4

[Patrick McCormick]

Attachment: updated patch to remove casts in nsAutoCompleteMdbResult.cpp

updated for changes to RemoveRowAt.  builds cleanly for me on win32.

Comment 5

[Patrick McCormick]

Comment on attachment 149524 [details] [diff] [review]
updated patch to remove casts in nsAutoCompleteMdbResult.cpp

this is a tiny change (see GetRowAt) for this previously r+sr patch.  I'm not
sure if this even needs r+sr, but I'll ask just in case.

Comment 6

[Mike Connor [:mconnor]]

Comment on attachment 149524 [details] [diff] [review]
updated patch to remove casts in nsAutoCompleteMdbResult.cpp

not necessary in this case, the patch didn't change substantially, its just
bitrot

I'll check this in during my next batch of checkins.

Comment 7

[Mike Connor [:mconnor]]

fixed branch and trunk