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..6c0c91950430e51aa9654ec3d35b8fdc5e7782e9 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();

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

I missed that layoutRows calls computeOverflowFromCells so it makes sense just
to move all the overflow-related logic there.

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

Acknowledged.

-    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,11 @@ void RenderTableSection::computeOverflowFromCells(unsigned totalRows, unsigned n
 #if ENABLE(ASSERT)
             hasOverflowingCell |= cell->hasVisualOverflow();
 #endif
+
+            // FIXME: this misses visual effect overflow of of the rowRenderer. crbug.com/437012.

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

Note that RenderTableRow::layout() actually calls addVisualEffectOverflow() so
we should centralize the re-computation of overflow as part of crbug.com/437012.

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

Acknowledged.

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

Acknowledged.

+            if (rowRenderer)
+                rowRenderer->addOverflowFromCell(cell);
+
             if (cell->hasVisualOverflow() && !m_forceSlowPaintPathWithOverflowingCell) {
                 m_overflowingCells.add(cell);
                 if (m_overflowingCells.size() > maxAllowedOverflowingCellsCount) {
@@ -1129,6 +1140,39 @@ 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();

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

Do we check somewhere that this is cleared for every renderer in the tree?

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

No for now. Will consider adding it.

+
+        for (unsigned c = 0; c < numEffCols; c++) {
+            CellStruct& cs = cellAt(r, c);
+            RenderTableCell* cell = cs.primaryCell();
+            if (!cell || cs.inColSpan || !cell->needsOverflowRecalcAfterStyleChange())
+                continue;
+            childrenOverflowChanged |= cell->recalcOverflowAfterStyleChange();
+        }
+    }
+
+    // FIXME: We should do a full recomputeOverflow which will be feasible after we make RenderBox recomputeOverflow-capable.
+    // crbug.com/437012.

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

I don't think we should do it this way: we should fix the computeOverflow
situation and then this change would land without nefarious FIXMEs and
performance degradations.

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

Removed FIXME. With extracted RenderTableRow::recomputeOverflow(), there is also
no performance degradations.

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