[webkit-reviews] review granted: [Bug 225949] CrashOnOverflow in WebCore::RenderTable::cellBefore(WebCore::RenderTableCell const*) : [Attachment 429614] Patch

Tue May 25 01:46:37 PDT 2021

Sergio Villar Senin <svillar at igalia.com> has granted zalan <zalan at apple.com>'s
request for review:
Bug 225949: CrashOnOverflow in
WebCore::RenderTable::cellBefore(WebCore::RenderTableCell const*)
https://bugs.webkit.org/show_bug.cgi?id=225949

Attachment 429614: Patch

https://bugs.webkit.org/attachment.cgi?id=429614&action=review

--- Comment #2 from Sergio Villar Senin <svillar at igalia.com> ---
Comment on attachment 429614
  --> https://bugs.webkit.org/attachment.cgi?id=429614
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=429614&action=review

> Source/WebCore/ChangeLog:18
> +	   (WebCore::RenderTableSection::willInsertTableRow): fun fact: this
code dates back to r3351 "The new table code from Lars.".

:O

> Source/WebCore/rendering/RenderTableSection.cpp:126
> +    setNeedsCellRecalc();

I guess this won't regress the performance that much as beforeChild is not null
as long as we are not inserting the first row. Just not sure how often this is
done in the wild...

> LayoutTests/fast/table/table-split-on-insert-with-section-crash.html:1
> +<style>

Can we have a <!DOCTYPE html> or do we depend on quirks mode?

> LayoutTests/fast/table/table-split-on-insert-with-section-crash.html:16
> +  onload = () => {

Check indentation of the whole onload block

> LayoutTests/fast/table/table-split-on-insert-with-section-crash.html:32
> +<!-- Pass if no crash or assert. -->

The designMode="on" normally turns down any attempt to show any message in the
expected file via html elements. What about using console.log() to show the
text from this comment?