[webkit-reviews] review granted: [Bug 98221] Make tables which don't use col/row span much faster to layout : [Attachment 167186] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Oct 4 15:51:26 PDT 2012


Julien Chaffraix <jchaffraix at webkit.org> has granted Eric Seidel
<eric at webkit.org>'s request for review:
Bug 98221: Make tables which don't use col/row span much faster to layout
https://bugs.webkit.org/show_bug.cgi?id=98221

Attachment 167186: Patch
https://bugs.webkit.org/attachment.cgi?id=167186&action=review

------- Additional Comments from Julien Chaffraix <jchaffraix at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=167186&action=review


> Source/WebCore/ChangeLog:9
> +	   so I stole another 2 bits from RenderTableCell::m_column to avoid

you stole 1 bit actually ;)

> Source/WebCore/rendering/RenderTableCell.cpp:55
> +COMPILE_ASSERT(sizeof(RenderTableCell) == sizeof(SameSizeAsRenderTableCell),
RenderTableCell_should_stay_small);

As you pointed out, this will likely fail on Windows due to the different
member's type.

> Source/WebCore/rendering/RenderTableCell.cpp:81
> +    if (!node())
> +	   return 1;

This could be moved around the call to updateColAndRowSpanFlags() in the
constructor and removed from both parse function. You are guaranteed to have
the right type if you get called from colSpanOrRowSpanChanged.

> Source/WebCore/rendering/RenderTableCell.cpp:123
> +    // FIXME: I suspect that we could return early here if !m_hasColSpan &&
!m_hasRowSpan.

I don't think this is true. You could probably bail out if the old / new values
of rowSpan and colSpan were identical though but we don't have enough
information in this function to check that.


More information about the webkit-reviews mailing list