92573 - nsVoidArray -> nsAutoVoidArray changes for Layout

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Randell Jesup [:jesup] (needinfo me)]

Taken from bug 90545.  I _really_ want to see these committed ASAP.  The patch
will require that the phase 1 patch from that bug be applied/committed first.

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

I'm taking this bug (it's a split off from my bug 90545).

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Just the layout, ma'am. Patch for nsVoidArray->nsAutoVoidArray for layout/*

Comment 3

[Marc Attinasi]

Review Comments: 

In nsCellMap.cpp, the method OrderRowGroups should be changed to take and
nsAutoVoidArray reference, and the following cast should be eliminated:
 -  aTableFrame.OrderRowGroups(orderedRowGroups, numRowGroups);
 +  aTableFrame.OrderRowGroups((nsVoidArray &)orderedRowGroups, numRowGroups);
It seems bad to cast the nsVoidArray to an nsAutoVoidArray like that.

Other than that, the change looks good to me, but <paranoias> since table code
is so heavily involved, it might be good to run the table regression tests
</paranoias> [s]r=attinasi

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Given a review from attinasi, I'll try to get this for 0.9.4.  Risk is low.

Comment 5

[Marc Attinasi]

Randell, what about my comments?

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

Sorry, I was going to post an updated patch, but I was waiting for it to compile
and test it first.

I simply removed the cast; it was a leftover and was irrelevant.  C++ does the
type-conversion automatically.

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

Ran every testcase I could find; no errors that weren't present before the patch
(I ran two side-by-side, one with one without).

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Updated patch with cast removed

Comment 9

[kinmoz]

r=[e-mail]

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

Kin asked about possible bloat due to nsHTMLReflowCommands getting bigger.  Here
are the stats I gathered:

nsHTMLReflowCommand->mPath stats:
	Number created:    368
	Number in-use max:  10
	Num max size == 0:   0
	Num max size <= 8: 298
	Num max size  > 8:  70

So the worst possible bloat is 32*10 = 320 bytes, but since ~80% of them use no
more than 8 items, and none use 0, the likely bloat is more like 64 bytes.
(This was loading and resizing each of mozilla.org, cnn.com, yahoo.com and
espn.com.)

I'm comitting this patch now.

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

*** Bug 80210 has been marked as a duplicate of this bug. ***