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

Changeset 100183 in webkit


Ignore:
Timestamp:
Nov 14, 2011, 1:21:43 PM (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

    r100179 r100183  
     12011-11-14  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-14  Jer Noble  <jer.noble@apple.com>
    219

  • trunk/Source/WebCore/ChangeLog

    r100182 r100183  
     12011-11-14  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-14  Adam Barth  <abarth@webkit.org>
    239

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

    r100177 r100183  
    645645    m_columns[position + 1].span = oldSpan - firstSpan;
    646646
    647     // change width of all rows.
     647    // Propagate the change in our columns representation to the sections that don't need
     648    // cell recalc. If they do, they will be synced up directly with m_columns later.
    648649    for (RenderObject* child = firstChild(); child; child = child->nextSibling()) {
    649         if (child->isTableSection())
    650             toRenderTableSection(child)->splitColumn(position, firstSpan);
     650        if (!child->isTableSection())
     651            continue;
     652
     653        RenderTableSection* section = toRenderTableSection(child);
     654        if (section->needsCellRecalc())
     655            continue;
     656
     657        section->splitColumn(position, firstSpan);
    651658    }
    652659
    … …  
    657664void RenderTable::appendColumn(int span)
    658665{
    659     // easy case.
    660     int pos = m_columns.size();
    661     int newSize = pos + 1;
     666    unsigned pos = m_columns.size();
     667    unsigned newSize = pos + 1;
    662668    m_columns.grow(newSize);
    663669    m_columns[pos].span = span;
    664670
    665     // change width of all rows.
     671    // Propagate the change in our columns representation to the sections that don't need
     672    // cell recalc. If they do, they will be synced up directly with m_columns later.
    666673    for (RenderObject* child = firstChild(); child; child = child->nextSibling()) {
    667         if (child->isTableSection())
    668             toRenderTableSection(child)->appendColumn(pos);
     674        if (!child->isTableSection())
     675            continue;
     676
     677        RenderTableSection* section = toRenderTableSection(child);
     678        if (section->needsCellRecalc())
     679            continue;
     680
     681        section->appendColumn(pos);
    669682    }
    670683

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

    r100067 r100183  
    188188void RenderTableSection::addCell(RenderTableCell* cell, RenderTableRow* row)
    189189{
     190    // We don't insert the cell if we need cell recalc as our internal columns' representation
     191    // will have drifted from the table's representation. Also recalcCells will call addCell
     192    // at a later time after sync'ing our columns' with the table's.
     193    if (needsCellRecalc())
     194        return;
     195
    190196    int rSpan = cell->rowSpan();
    191197    int cSpan = cell->colSpan();
    … …  
    11171123void RenderTableSection::recalcCells()
    11181124{
     1125    ASSERT(m_needsCellRecalc);
     1126    // We reset the flag here to ensure that |addCell| works. This is safe to do as
     1127    // fillRowsWithDefaultStartingAtPosition makes sure we match the table's columns
     1128    // representation.
     1129    m_needsCellRecalc = false;
     1130
    11191131    m_cCol = 0;
    11201132    m_cRow = 0;
    … …  
    11431155
    11441156    m_grid.shrinkToFit();
    1145     m_needsCellRecalc = false;
    11461157    setNeedsLayout(true);
    11471158}
    … …  
    11761187void RenderTableSection::appendColumn(int pos)
    11771188{
     1189    ASSERT(!m_needsCellRecalc);
     1190
    11781191    for (unsigned row = 0; row < m_grid.size(); ++row)
    11791192        m_grid[row].row.resize(pos + 1);
Note: See TracChangeset for help on using the changeset viewer.