Let RenderTable reach table cells needing overflow recalc

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, 2013 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 925 matching lines @@
 #endif
 
     ASSERT(!needsLayout());
 
     // FIXME: Changing the height without a layout can change the overflow so it seems wrong.
 
     unsigned totalRows = m_grid.size();
 
     // 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();
 
     LayoutState state(*this, locationOffset());
 
     for (unsigned r = 0; r < totalRows; r++) {
         // Set the row's x/y position and width/height.
         RenderTableRow* rowRenderer = m_grid[r].rowRenderer;
         if (rowRenderer) {
             rowRenderer->setLocation(LayoutPoint(0, m_rowPos[r]));
             rowRenderer->setLogicalWidth(logicalWidth());
             rowRenderer->setLogicalHeight(m_rowPos[r + 1] - m_rowPos[r] - vspacing);
             rowRenderer->updateLayerTransformAfterLayout();
-            rowRenderer->clearAllOverflows();
-            rowRenderer->addVisualEffectOverflow();
         }
 
         int rowHeightIncreaseForPagination = 0;
 
         for (unsigned c = 0; c < nEffCols; c++) {
             CellStruct& cs = cellAt(r, c);
             RenderTableCell* cell = cs.primaryCell();
 
             if (!cell || cs.inColSpan)
                 continue;
@@ skipping 71 matching lines @@
                 // FIXME: Pagination might have made us change size. For now just shrink or grow the cell to fit without doing a relayout.
                 // We'll also do a basic increase of the row height to accommodate the cell if it's bigger, but this isn't quite right
                 // either. It's at least stable though and won't result in an infinite # of relayouts that may never stabilize.
                 LayoutUnit oldLogicalHeight = cell->logicalHeight();
                 if (oldLogicalHeight > rHeight)
                     rowHeightIncreaseForPagination = std::max<int>(rowHeightIncreaseForPagination, oldLogicalHeight - rHeight);
                 cell->setLogicalHeight(rHeight);
                 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
                 // descendants. An exception is if we need a layout. In this case, we know we're going to
                 // issue paint invalidations ourselves (and the child) anyway.
                 if (!table()->selfNeedsLayout())
                     cell->setMayNeedPaintInvalidation(true);
             }
         }
         if (rowHeightIncreaseForPagination) {
@@ skipping 22 matching lines @@
     unsigned totalRows = m_grid.size();
     unsigned nEffCols = table()->numEffCols();
     computeOverflowFromCells(totalRows, nEffCols);
 }
 
 void RenderTableSection::computeOverflowFromCells(unsigned totalRows, unsigned nEffCols)
 {
     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();
             if (!cell || cs.inColSpan)
                 continue;
             if (r < totalRows - 1 && cell == primaryCellAt(r + 1, c))
                 continue;
             addOverflowFromChild(cell);
 #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) {
                     // We need to set m_forcesSlowPaintPath only if there is a least one overflowing cells as the hit testing code rely on this information.
                     m_forceSlowPaintPathWithOverflowingCell = true;
                     // The slow path does not make any use of the overflowing cells info, don't hold on to the memory.
                     m_overflowingCells.clear();
                 }
             }
         }
     }
 
     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();
     if (!m_grid.size() || !totalCols)
         return 0;
 
     unsigned borderWidth = 0;
 
     const BorderValue& sb = side == BorderBefore ? style()->borderBefore() : style()->borderAfter();
     if (sb.style() == BHIDDEN)
@@ skipping 471 matching lines @@
     // FIXME: The table's direction should determine our row's direction, not the section's (see bug 96691).
     if (!style()->isLeftToRightDirection())
         cellLocation.setX(table()->columnPositions()[table()->numEffCols()] - table()->columnPositions()[table()->colToEffCol(cell->col() + cell->colSpan())] + horizontalBorderSpacing);
     else
         cellLocation.setX(table()->columnPositions()[effectiveColumn] + horizontalBorderSpacing);
 
     cell->setLogicalLocation(cellLocation);
 }
 
 } // namespace blink