Let RenderTable reach table cells needing overflow recalc

Source/core/rendering/RenderTableSection.cpp

Index: Source/core/rendering/RenderTableSection.cpp
diff --git a/Source/core/rendering/RenderTableSection.cpp b/Source/core/rendering/RenderTableSection.cpp
index f1983f60442b97de4e318913bdadcee70a486cf4..5e8a9f9f5e52bbbe0bfa680273c917d6c8988f4d 100644
--- a/Source/core/rendering/RenderTableSection.cpp
+++ b/Source/core/rendering/RenderTableSection.cpp
@@ -943,9 +943,10 @@ void RenderTableSection::layoutRows()
 
     // Set the width of our section now.  The rows will also be this width.
     setLogicalWidth(table()->contentLogicalWidth());
-    m_overflow.clear();
+
+    // We will clear and recompute overflow in computeOverflowFromCells().
+    // Clear m_overflowCells early to avoid stale pointers from it.
     m_overflowingCells.clear();
-    m_forceSlowPaintPathWithOverflowingCell = false;
 
     int vspacing = table()->vBorderSpacing();
     unsigned nEffCols = table()->numEffCols();
@@ -960,8 +961,6 @@ void RenderTableSection::layoutRows()
             rowRenderer->setLogicalWidth(logicalWidth());
             rowRenderer->setLogicalHeight(m_rowPos[r + 1] - m_rowPos[r] - vspacing);
             rowRenderer->updateLayerTransformAfterLayout();
-            rowRenderer->clearAllOverflows();
-            rowRenderer->addVisualEffectOverflow();
         }
 
         int rowHeightIncreaseForPagination = 0;
@@ -1053,9 +1052,6 @@ void RenderTableSection::layoutRows()
                 cell->computeOverflow(oldLogicalHeight, false);
             }
 
-            if (rowRenderer)
-                rowRenderer->addOverflowFromCell(cell);
-
             LayoutSize childOffset(cell->location() - oldCellRect.location());
             if (childOffset.width() || childOffset.height()) {
                 // If the child moved, we have to issue paint invalidations to it as well as any floating/positioned
@@ -1098,11 +1094,21 @@ void RenderTableSection::computeOverflowFromCells(unsigned totalRows, unsigned n
     unsigned totalCellsCount = nEffCols * totalRows;
     unsigned maxAllowedOverflowingCellsCount = totalCellsCount < gMinTableSizeToUseFastPaintPathWithOverflowingCell ? 0 : gMaxAllowedOverflowingCellRatioForFastPaintPath * totalCellsCount;
 
+    m_overflow.clear();
+    m_overflowingCells.clear();
+    m_forceSlowPaintPathWithOverflowingCell = false;
+
 #if ENABLE(ASSERT)
     bool hasOverflowingCell = false;
 #endif
     // Now that our height has been determined, add in overflow from cells.
     for (unsigned r = 0; r < totalRows; r++) {
+        RenderTableRow* rowRenderer = rowRendererAt(r);
+        if (rowRenderer) {
+            rowRenderer->clearAllOverflows();
+            rowRenderer->addVisualEffectOverflow();
+        }
+
         for (unsigned c = 0; c < nEffCols; c++) {
             CellStruct& cs = cellAt(r, c);
             RenderTableCell* cell = cs.primaryCell();
@@ -1114,6 +1120,10 @@ void RenderTableSection::computeOverflowFromCells(unsigned totalRows, unsigned n
 #if ENABLE(ASSERT)
             hasOverflowingCell |= cell->hasVisualOverflow();
 #endif
+
+            if (rowRenderer)
+                rowRenderer->addOverflowFromCell(cell);
+
             if (cell->hasVisualOverflow() && !m_forceSlowPaintPathWithOverflowingCell) {
                 m_overflowingCells.add(cell);
                 if (m_overflowingCells.size() > maxAllowedOverflowingCellsCount) {
@@ -1129,6 +1139,38 @@ void RenderTableSection::computeOverflowFromCells(unsigned totalRows, unsigned n
     ASSERT(hasOverflowingCell == this->hasOverflowingCell());
 }
 
+bool RenderTableSection::recalcChildOverflowAfterStyleChange()
+{
+    ASSERT(childNeedsOverflowRecalcAfterStyleChange());
+    clearChildNeedsOverflowRecalcAfterStyleChange();
+
+    unsigned totalRows = m_grid.size();
+    unsigned numEffCols = table()->numEffCols();
+    bool childrenOverflowChanged = false;
+
+    for (unsigned r = 0; r < totalRows; r++) {
+        RenderTableRow* rowRenderer = rowRendererAt(r);
+        if (!rowRenderer || !rowRenderer->childNeedsOverflowRecalcAfterStyleChange())
+            continue;
+
+        rowRenderer->clearChildNeedsOverflowRecalcAfterStyleChange();
+
+        for (unsigned c = 0; c < numEffCols; c++) {
+            CellStruct& cs = cellAt(r, c);
+            RenderTableCell* cell = cs.primaryCell();
+            if (!cell || cs.inColSpan || !cell->needsOverflowRecalcAfterStyleChange())
+                continue;
+            if (cell->recalcOverflowAfterStyleChange())
+                childrenOverflowChanged = true;
+        }
+    }
+
+    if (childrenOverflowChanged)

[Julien - ping for review, 2014/11/26 19:23:34]

If we changed only overflow for the second (think change an outline / filter /
box-shadow on the section itself), then we won't recompute the overflow? It
seems to be an existing bug in the code that we don't call
addVisualEffectOverflow() so it's fine to put a FIXME about it.

[Xianzhu, 2014/11/26 20:53:30]

Done.

Actually we never call addVisualEffectOverflow() for RenderTableSection (as well
as RenderTableRow), because RenderTableSection is a RenderBox, but
addVisualEffectOverflow() is only called from RenderBlock.

I'm working on a change to move RenderBlock::recomputeOverflow and
RenderBlock::recalcOverflowAfterStyleChange etc. into RenderBox so that
RenderBoxes can also recompute their overflows when needed. With that change,
computeOverflowFromCells will be part of recomputeOverflow which will be called
from from recalcOverflowAfterStyleChange() when
recalcChildOverflowAfterStyleChange() returns true. 

+        computeOverflowFromCells(totalRows, numEffCols);

[Julien - ping for review, 2014/11/26 19:23:34]

While the performance is better, you will still blow away the computed overflow
for rows / cells that haven't changed. That's caused by keeping only a single
bit of information about what changed. As child needs overflow recalc is set, I
would assume we should *always* call computeOverflowFromCell and we can just
lazily recompute their overflow based on their booleans.

[Xianzhu, 2014/11/26 20:53:30]

Thought of a similar method, but considering the following complexities, this
seems not feasible because we keep only one accumulated visual overflow rect
(for each of the rows and sections):
- when a cell's overflow shrinks;
- when a cell's overflow changes by expanding at one side and shrinking at
another side.

In the above cases, we don't know how to incrementally update the accumulated
overflow rect of the section. It seems the only way to fully recompute the
overflow.

[Julien - ping for review, 2014/11/26 23:21:25]

By incremental, I mean we have to recompute the full rect but we don't have to
recompute the rows / cells that haven't been marked for overflow computation.

[Xianzhu, 2014/11/27 00:28:48]

computeOverflowFromCells() doesn't recompute overflow for cells. It does
recompute overflow for all rows. Extracted RenderTableRow::recomputeOverflow()
from the original place and call it if rowChildrenOverflowChanged. Then
computeOverflowFromCells() just recomputes overflow of section itself (will be
RenderTableSection::recomputeOverflow() for crbug.com/437012).

+
+    return childrenOverflowChanged;
+}
+
 int RenderTableSection::calcBlockDirectionOuterBorder(BlockBorderSide side) const
 {
     unsigned totalCols = table()->numEffCols();