[webkit-reviews] review granted: [Bug 71056] RenderTableSection::recalcCells should not free its grid : [Attachment 112806] Avoid free'ing the m_grid vector, refactored |row| to be part of RowStruct and made the 'reset' logic more apparent.

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Julien Chaffraix
<jchaffraix at webkit.org>'s request for review:
Bug 71056: RenderTableSection::recalcCells should not free its grid
https://bugs.webkit.org/show_bug.cgi?id=71056

Attachment 112806: Avoid free'ing the m_grid vector, refactored |row| to be
part of RowStruct and made the 'reset' logic more apparent.
https://bugs.webkit.org/attachment.cgi?id=112806&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=112806&action=review


Patch does not apply. I am going to say review+ but not sure about that aspect
of things.

> Source/WebCore/rendering/RenderTableSection.cpp:77
>  RenderTableSection::~RenderTableSection()
>  {
> -    clearGrid();
> +    m_grid.clear();
>  }

This is not needed in a destructor. The Vector will be destroyed automatically,
so no need to clear it. You can just remove the call to clearGrid entirely and
not replace it with anything.

> Source/WebCore/rendering/RenderTableSection.cpp:1159
> +	   m_grid[row].row.clear();
> +	   m_grid[row].row.resize(effectiveColumnCount);

If we do need to clear and then re-allocate, we can at least factor out one
unnecessary branch by using grow instead of resize.