⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 99744 in webkit


Ignore:
Timestamp:
Nov 9, 2011, 11:57:16 AM (15 years ago)
Author:
jchaffraix@webkit.org
Message:

Crash in RenderTableSection::splitColumn
​https://bugs.webkit.org/show_bug.cgi?id=70171

Reviewed by David Hyatt.

Source/WebCore:

Tests: fast/table/crash-splitColumn-2.html

fast/table/crash-splitColumn-3.html
fast/table/crash-splitColumn.html

The old code would not take into account the fact that each RenderTableSection
can set its m_needsCellRecalc flag independently of the rest.

This means that you cannot assume that you can always split or append columns to
all the sections. Our approach is to skip sections needing cell recalc in several
parts of the code as they will be properly reset to the table's representations
during a cell recalc.

  • rendering/RenderTable.cpp:

(WebCore::RenderTable::splitColumn):
(WebCore::RenderTable::appendColumn):
Skip sections needing cell recalc as they will be properly updated later.

  • rendering/RenderTableSection.cpp:

(WebCore::RenderTableSection::addCell):
Ignore a section needing cell recalc as addCell will be called after sync'ing
the internal column representation in recalcCells.

(WebCore::RenderTableSection::recalcCells):
Clear the flag at the beginning of the function to activate the previous functions.
Added a comment as to why this is fine.

(WebCore::RenderTableSection::appendColumn):
Added an ASSERT. If we need cell recalc, we should NEVER update m_grid outside
of recalcCells().

LayoutTests:

Added a couple of tests where different sections get their
m_needsCellRecalc set independently.

  • fast/table/crash-splitColumn-2-expected.txt: Added.
  • fast/table/crash-splitColumn-2.html: Added.
  • fast/table/crash-splitColumn-3-expected.txt: Added.
  • fast/table/crash-splitColumn-3.html: Added.
  • fast/table/crash-splitColumn-expected.txt: Added.
  • fast/table/crash-splitColumn.html: Added.
Location:
trunk
Files:
6 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r99742 r99744  
     12011-11-09  Julien Chaffraix  <jchaffraix@webkit.org>
     2
     3        Crash in RenderTableSection::splitColumn
     4        https://bugs.webkit.org/show_bug.cgi?id=70171
     5
     6        Reviewed by David Hyatt.
     7
     8        Added a couple of tests where different sections get their
     9        m_needsCellRecalc set independently.
     10
     11        * fast/table/crash-splitColumn-2-expected.txt: Added.
     12        * fast/table/crash-splitColumn-2.html: Added.
     13        * fast/table/crash-splitColumn-3-expected.txt: Added.
     14        * fast/table/crash-splitColumn-3.html: Added.
     15        * fast/table/crash-splitColumn-expected.txt: Added.
     16        * fast/table/crash-splitColumn.html: Added.
     17
    1182011-11-09  Arko Saha  <arko@motorola.com>
    219

  • trunk/Source/WebCore/ChangeLog

    r99742 r99744  
     12011-11-09  Julien Chaffraix  <jchaffraix@webkit.org>
     2
     3        Crash in RenderTableSection::splitColumn
     4        https://bugs.webkit.org/show_bug.cgi?id=70171
     5
     6        Reviewed by David Hyatt.
     7
     8        Tests: fast/table/crash-splitColumn-2.html
     9               fast/table/crash-splitColumn-3.html
     10               fast/table/crash-splitColumn.html
     11
     12        The old code would not take into account the fact that each RenderTableSection
     13        can set its m_needsCellRecalc flag independently of the rest.
     14
     15        This means that you cannot assume that you can always split or append columns to
     16        all the sections. Our approach is to skip sections needing cell recalc in several
     17        parts of the code as they will be properly reset to the table's representations
     18        during a cell recalc.
     19
     20        * rendering/RenderTable.cpp:
     21        (WebCore::RenderTable::splitColumn):
     22        (WebCore::RenderTable::appendColumn):
     23        Skip sections needing cell recalc as they will be properly updated later.
     24
     25        * rendering/RenderTableSection.cpp:
     26        (WebCore::RenderTableSection::addCell):
     27        Ignore a section needing cell recalc as addCell will be called after sync'ing
     28        the internal column representation in recalcCells.
     29
     30        (WebCore::RenderTableSection::recalcCells):
     31        Clear the flag at the beginning of the function to activate the previous functions.
     32        Added a comment as to why this is fine.
     33
     34        (WebCore::RenderTableSection::appendColumn):
     35        Added an ASSERT. If we need cell recalc, we should NEVER update m_grid outside
     36        of recalcCells().
     37
    1382011-11-09  Arko Saha  <arko@motorola.com>
    239

  • trunk/Source/WebCore/rendering/RenderTable.cpp

    r98676 r99744  
    646646    m_columns[position + 1].span = oldSpan - firstSpan;
    647647
    648     // change width of all rows.
     648    // Propagate the change in our columns representation to the sections that don't need
     649    // cell recalc. If they do, they will be synced up directly with m_columns later.
    649650    for (RenderObject* child = firstChild(); child; child = child->nextSibling()) {
    650         if (child->isTableSection())
    651             toRenderTableSection(child)->splitColumn(position, firstSpan);
     651        if (!child->isTableSection())
     652            continue;
     653
     654        RenderTableSection* section = toRenderTableSection(child);
     655        if (section->needsCellRecalc())
     656            continue;
     657
     658        section->splitColumn(position, firstSpan);
    652659    }
    653660
    … …  
    658665void RenderTable::appendColumn(int span)
    659666{
    660     // easy case.
    661     int pos = m_columns.size();
    662     int newSize = pos + 1;
     667    unsigned pos = m_columns.size();
     668    unsigned newSize = pos + 1;
    663669    m_columns.grow(newSize);
    664670    m_columns[pos].span = span;
    665671
    666     // change width of all rows.
     672    // Propagate the change in our columns representation to the sections that don't need
     673    // cell recalc. If they do, they will be synced up directly with m_columns later.
    667674    for (RenderObject* child = firstChild(); child; child = child->nextSibling()) {
    668         if (child->isTableSection())
    669             toRenderTableSection(child)->appendColumn(pos);
     675        if (!child->isTableSection())
     676            continue;
     677
     678        RenderTableSection* section = toRenderTableSection(child);
     679        if (section->needsCellRecalc())
     680            continue;
     681
     682        section->appendColumn(pos);
    670683    }
    671684

  • trunk/Source/WebCore/rendering/RenderTableSection.cpp

    r99254 r99744  
    192192void RenderTableSection::addCell(RenderTableCell* cell, RenderTableRow* row)
    193193{
     194    // We don't insert the cell if we need cell recalc as our internal columns' representation
     195    // will have drifted from the table's representation. Also recalcCells will call addCell
     196    // at a later time after sync'ing our columns' with the table's.
     197    if (needsCellRecalc())
     198        return;
     199
    194200    int rSpan = cell->rowSpan();
    195201    int cSpan = cell->colSpan();
    … …  
    11231129void RenderTableSection::recalcCells()
    11241130{
     1131    ASSERT(m_needsCellRecalc);
     1132    // We reset the flag here to ensure that |addCell| works. This is safe to do as
     1133    // fillRowsWithDefaultStartingAtPosition makes sure we match the table's columns
     1134    // representation.
     1135    m_needsCellRecalc = false;
     1136
    11251137    m_cCol = 0;
    11261138    m_cRow = 0;
    … …  
    11551167    gridSize = max(gridSize, m_cRow);
    11561168    m_grid.shrink(gridSize);
    1157     m_needsCellRecalc = false;
    11581169    setNeedsLayout(true);
    11591170}
    … …  
    12031214void RenderTableSection::appendColumn(int pos)
    12041215{
     1216    ASSERT(!m_needsCellRecalc);
     1217
    12051218    for (unsigned row = 0; row < m_grid.size(); ++row)
    12061219        m_grid[row].row.resize(pos + 1);
Note: See TracChangeset for help on using the changeset viewer.