227491 - [FIXr]Better array stuff in nsBaseWidget

Status: RESOLVED FIXED

(Core :: Web Painting, defect, P2)

Description

[Boris Zbarsky [:bzbarsky]]

Less QI/addref/release, coming up.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: nsCOMArray!

Comment 2

[Brendan Eich [:brendan]]

Comment on attachment 136808 [details] [diff] [review]
nsCOMArray!

Beauty -- expand tabs to 2-space-equiv while you're there?

/be

Comment 3

[Boris Zbarsky [:bzbarsky]]

Most of the file is tab-indented, actually...  I purposefully kept them.

Comment 4

[Brendan Eich [:brendan]]

1.7a sounds like a fine time to expand 'em all.  diff -w will help us see past
them, and cvs annotate -r will help cvs archeologists find whodunnit.

/be

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 136808 [details] [diff] [review]
nsCOMArray!

This is actually slightly wrong...

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Updated, with tabs replaced with spaces

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Same as diff -w

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 136916 [details] [diff] [review]
Same as diff -w

Looks fine, but how about other platforms?

Comment 9

[Boris Zbarsky [:bzbarsky]]

The other platforms don't access mChildren directly -- they go through the silly
enumerator GetChildren() returns.  In fact, the GTK code does so itself at other
callsites.  I suppose I could clean that up to use mChildren where possible and
roll that into this bug...

Comment 10

[Brendan Eich [:brendan]]

Comment on attachment 136916 [details] [diff] [review]
Same as diff -w

(Pre-existing) Irrational Control Flow alert:

+  if ( mCurrentPosition < mParent.mChildren.Count() )
+    NS_IF_ADDREF(*aItem = mParent.mChildren[mCurrentPosition]);
   else
     return NS_ERROR_FAILURE;

   return NS_OK;

if (A)
  B;
else
  return C;
return D;

should be

if (!A)
  return C;
B;
return D:

/be

Comment 11

[Boris Zbarsky [:bzbarsky]]

Fixed in 1.7a, without making the changes in comment 10 (they would have to be
made all over, really; that pattern is common in that file).