Col width is not honored when dynamically updated and it would not make table narrower

Source/core/rendering/RenderTableSection.cpp

 /*
  * Copyright (C) 1997 Martin Jones (mjones@kde.org)
  *           (C) 1997 Torben Weis (weis@kde.org)
  *           (C) 1998 Waldo Bastian (bastian@kde.org)
  *           (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  * Copyright (C) 2003, 2004, 2005, 2006, 2008, 2009, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  *
  * This library is free software; you can redistribute it and/or
@@ skipping 1469 matching lines @@
 
 void RenderTableSection::imageChanged(WrappedImagePtr, const IntRect*)
 {
     // FIXME: Examine cells and repaint only the rect the image paints in.
     repaint();
 }
 
 void RenderTableSection::recalcCells()
 {
     ASSERT(m_needsCellRecalc);
-    // We reset the flag here to ensure that |addCell| works. This is safe to do as
-    // fillRowsWithDefaultStartingAtPosition makes sure we match the table's columns
-    // representation.
+    // We reset the flag here to ensure that addCell() works. This is safe to do because we clear the grid
+    // and update its dimensions to be consistent with the table's column representation before we rebuild
+    // the grid using addCell().
     m_needsCellRecalc = false;
 
     m_cCol = 0;
     m_cRow = 0;
     m_grid.clear();
 
     for (RenderObject* row = firstChild(); row; row = row->nextSibling()) {
         if (row->isTableRow()) {
             unsigned insertionRow = m_cRow;
             m_cRow++;
@@ skipping 31 matching lines @@
         if (!cell->isTableCell())
             continue;
 
         updateLogicalHeightForCell(m_grid[rowIndex], toRenderTableCell(cell));
     }
 }
 
 void RenderTableSection::setNeedsCellRecalc()
 {
     m_needsCellRecalc = true;
+
+    // Clear the grid now to ensure that we don't hold onto any stale pointers (e.g. a cell renderer that is being removed).
+    m_grid.clear();

[Julien - ping for review, 2014/03/29 01:04:21]

As stated in codereview.chromium.org/208263012 (based on the comments in
https://bugs.webkit.org/show_bug.cgi?id=129561), this doesn't seem to be the
right fix. It shouldn't solve a crasher (or if it does, I would like to have
some investigation as to why) but is still a good defense-in-depth pattern.

+
     if (RenderTable* t = table())
         t->setNeedsSectionRecalc();
 }
 
 unsigned RenderTableSection::numColumns() const
 {
+    ASSERT(!m_needsCellRecalc);
     unsigned result = 0;
 
     for (unsigned r = 0; r < m_grid.size(); ++r) {
         for (unsigned c = result; c < table()->numEffCols(); ++c) {
             const CellStruct& cell = cellAt(r, c);
             if (cell.hasCells() || cell.inColSpan)
                 result = c;
         }
     }
 
@@ skipping 170 matching lines @@
     else
         cellLocation.setX(table()->columnPositions()[effectiveColumn] + horizontalBorderSpacing);
 
     cell->setLogicalLocation(cellLocation);
 
     if (!RuntimeEnabledFeatures::repaintAfterLayoutEnabled())
         view()->addLayoutDelta(oldCellLocation - cell->location());
 }
 
 } // namespace WebCore