716408 - Minor cleanup in table code

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Comment 1

[Mats Palmgren (PTO)]

Attachment: Remove null-checks of GetFirstInFlow() result

GetFirstInFlow() can never return null so we can remove these null-checks

Comment 2

[Mats Palmgren (PTO)]

Attachment: Remove unnecessary nsTableFrame::GetFirstInFlow() calls to get IsBorderCollapse() bit

The mIsBorderCollapse bit is set on all table frames in Init so there's
no need to get this bit from the first-in-flow frame.

Comment 3

[Mats Palmgren (PTO)]

Attachment: Make nsTableFrame::GetTableFrame abort if the given frame isn't a table frame descendant

Make GetTableFrame() abort if the arg isn't a table frame descendant.
Consequently, remove null-checks of GetTableFrame() results.

Comment 4

[Mats Palmgren (PTO)]

Attachment: Initialize all bits of mBits to zero

Comment 5

[Mats Palmgren (PTO)]

Attachment: No need to null-check for delete, no need to null out members in dtor

Comment 6

[Mats Palmgren (PTO)]

Attachment: Assert that a prev-in-flow is null or of the same frame type. No need to null out mCellMap, the ctor does that and re-Init of frames isn't allowed

Comment 7

[Mats Palmgren (PTO)]

Attachment: Indent nsTableFrame::DidSetStyleContext by two spaces, not three

Comment 9

[Bernd]

I will only on the weekend (14.01, 15.01) get to this reviews

Comment 10

[Bernd]

Comment on attachment 586835 [details] [diff] [review]
Remove null-checks of GetFirstInFlow() result

Review of attachment 586835 [details] [diff] [review]:
-----------------------------------------------------------------

please remove also mxr.mozilla.org/mozilla-central/source/layout/tables/nsTableFrame.cpp#3311 it doubles 
http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsSplittableFrame.cpp#197 
Further remove 
http://mxr.mozilla.org/mozilla-central/source/layout/tables/nsTableFrame.cpp#6237
http://mxr.mozilla.org/mozilla-central/source/layout/tables/nsTableFrame.cpp#6248

The patch is OK in itself, but lets get rid of it all once.

Comment 11

[Mats Palmgren (PTO)]

Attachment: Remove null-checks of GetFirstInFlow() result

Hmm, yes, those changes got lost somehow...

I took the opportunity of simplifying GetColumnWidth as well.

Comment 12

[Bernd]

Comment on attachment 588777 [details] [diff] [review]
Remove null-checks of GetFirstInFlow() result

Review of attachment 588777 [details] [diff] [review]:
-----------------------------------------------------------------

thats what I wanted.

Comment 13

[Bernd]

Comment on attachment 586836 [details] [diff] [review]
Remove unnecessary nsTableFrame::GetFirstInFlow() calls to get IsBorderCollapse() bit

Review of attachment 586836 [details] [diff] [review]:
-----------------------------------------------------------------

yep the master himself was inconsistent with respect to this, http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/tables/nsTableRowGroupFrame.cpp&rev=3.411&mark=394#394

Comment 14

[Bernd]

Comment on attachment 586840 [details] [diff] [review]
Make nsTableFrame::GetTableFrame abort if the given frame isn't a table frame descendant

Review of attachment 586840 [details] [diff] [review]:
-----------------------------------------------------------------

I like the change to NS_RUNTIMEABORT("unable to find table parent");

Comment 15

[Mats Palmgren (PTO)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/57fc94d1196a
https://hg.mozilla.org/integration/mozilla-inbound/rev/14223e7cfb3b
https://hg.mozilla.org/integration/mozilla-inbound/rev/302fde3d16e3
https://hg.mozilla.org/integration/mozilla-inbound/rev/5ba6c4100885
https://hg.mozilla.org/integration/mozilla-inbound/rev/f0eeea7083ec
https://hg.mozilla.org/integration/mozilla-inbound/rev/9d4556015326
https://hg.mozilla.org/integration/mozilla-inbound/rev/3077b729dfd5

Comment 16

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/57fc94d1196a
https://hg.mozilla.org/mozilla-central/rev/14223e7cfb3b
https://hg.mozilla.org/mozilla-central/rev/302fde3d16e3
https://hg.mozilla.org/mozilla-central/rev/5ba6c4100885
https://hg.mozilla.org/mozilla-central/rev/f0eeea7083ec
https://hg.mozilla.org/mozilla-central/rev/9d4556015326
https://hg.mozilla.org/mozilla-central/rev/3077b729dfd5