Fix blink_perf.paint regression about tables

third_party/WebKit/Source/core/layout/LayoutTableSection.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)
  *
@@ skipping 240 matching lines @@
       table()->effectiveColumns();
   unsigned insertionRow = row->rowIndex();
 
   // ### mozilla still seems to do the old HTML way, even for strict DTD
   // (see the annotation on table cell layouting in the CSS specs and the
   // testcase below:
   // <TABLE border>
   // <TR><TD>1 <TD rowspan="2">2 <TD>3 <TD>4
   // <TR><TD colspan="2">5
   // </TABLE>
-  while (m_cCol < numCols(insertionRow) &&
-         (cellAt(insertionRow, m_cCol).hasCells() ||
-          cellAt(insertionRow, m_cCol).inColSpan))
+  unsigned nCols = numCols(insertionRow);
+  while (m_cCol < nCols && (cellAt(insertionRow, m_cCol).hasCells() ||
+                            cellAt(insertionRow, m_cCol).inColSpan))
     m_cCol++;
 
   updateLogicalHeightForCell(m_grid[insertionRow], cell);
 
   ensureRows(insertionRow + rSpan);
 
   m_grid[insertionRow].rowLayoutObject = row;
 
   unsigned col = m_cCol;
   // tell the cell where it is
   bool inColSpan = false;
+  unsigned colSize = columns.size();
   while (cSpan) {
     unsigned currentSpan;
-    if (m_cCol >= columns.size()) {
+    if (m_cCol >= colSize) {
       table()->appendEffectiveColumn(cSpan);
       currentSpan = cSpan;
     } else {
       if (cSpan < columns[m_cCol].span)
         table()->splitEffectiveColumn(m_cCol, cSpan);
       currentSpan = columns[m_cCol].span;
     }
     for (unsigned r = 0; r < rSpan; r++) {
       ensureCols(insertionRow + r, m_cCol + 1);
       CellStruct& c = cellAt(insertionRow + r, m_cCol);
@@ skipping 857 matching lines @@
       }
       rowLayoutObject->setLogicalHeight(rowLogicalHeight);
       rowLayoutObject->updateLayerTransformAfterLayout();
     }
   }
 
   // Vertically align and flex the cells in each row.
   for (unsigned r = 0; r < totalRows; r++) {
     LayoutTableRow* rowLayoutObject = m_grid[r].rowLayoutObject;
 
-    for (unsigned c = 0; c < numCols(r); c++) {
+    unsigned nCols = numCols(r);

[mstensho (USE GERRIT), 2017/01/20 10:02:04]

These are good. I was meaning to point it out in the initial review, but I filed
it under nit-picking, so I didn't say anything. :(

+    for (unsigned c = 0; c < nCols; c++) {
       CellStruct& cs = cellAt(r, c);
       LayoutTableCell* cell = cs.primaryCell();
 
       if (!cell || cs.inColSpan)
         continue;
 
       if (cell->rowIndex() != r)
         continue;  // Rowspanned cells are handled in the first row they occur.
 
       int rHeight;
@@ skipping 99 matching lines @@
           : gMaxAllowedOverflowingCellRatioForFastPaintPath * totalCellsCount;
 
   m_overflow.reset();
   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++) {
-    for (unsigned c = 0; c < numCols(r); c++) {
+    unsigned nCols = numCols(r);
+    unsigned nextRowCells = r < totalRows - 1 ? numCols(r + 1) : 0;
+    for (unsigned c = 0; c < nCols; c++) {
       CellStruct& cs = cellAt(r, c);
       LayoutTableCell* cell = cs.primaryCell();
       if (!cell || cs.inColSpan)
         continue;
-      if (r < totalRows - 1 && cell == primaryCellAt(r + 1, c))
+      if (r < totalRows - 1 && c < nextRowCells &&
+          cell == primaryCellAt(r + 1, c))
         continue;
       addOverflowFromChild(cell);
 #if ENABLE(ASSERT)
       hasOverflowingCell |= cell->hasVisualOverflow();
 #endif
       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
@@ skipping 15 matching lines @@
   clearChildNeedsOverflowRecalcAfterStyleChange();
   unsigned totalRows = m_grid.size();
   bool childrenOverflowChanged = false;
   for (unsigned r = 0; r < totalRows; r++) {
     LayoutTableRow* rowLayouter = rowLayoutObjectAt(r);
     if (!rowLayouter ||
         !rowLayouter->childNeedsOverflowRecalcAfterStyleChange())
       continue;
     rowLayouter->clearChildNeedsOverflowRecalcAfterStyleChange();
     bool rowChildrenOverflowChanged = false;
-    for (unsigned c = 0; c < numCols(r); c++) {
+    unsigned nCols = numCols(r);
+    for (unsigned c = 0; c < nCols; c++) {
       CellStruct& cs = cellAt(r, c);
       LayoutTableCell* cell = cs.primaryCell();
       if (!cell || cs.inColSpan || !cell->needsOverflowRecalcAfterStyleChange())
         continue;
       rowChildrenOverflowChanged |= cell->recalcOverflowAfterStyleChange();
     }
     if (rowChildrenOverflowChanged)
       rowLayouter->computeOverflow();
     childrenOverflowChanged |= rowChildrenOverflowChanged;
   }
@@ skipping 32 matching lines @@
   const BorderValue& rb = side == BorderBefore
                               ? firstRow()->style()->borderBefore()
                               : lastRow()->style()->borderAfter();
   if (rb.style() == BorderStyleHidden)
     return -1;
   if (rb.style() > BorderStyleHidden && rb.width() > borderWidth)
     borderWidth = rb.width();
 
   bool allHidden = true;
   unsigned r = side == BorderBefore ? 0 : m_grid.size() - 1;
-  for (unsigned c = 0; c < numCols(r); c++) {
+  unsigned nCols = numCols(r);
+  for (unsigned c = 0; c < nCols; c++) {
     const CellStruct& current = cellAt(r, c);
     if (current.inColSpan || !current.hasCells())
       continue;
     const ComputedStyle& primaryCellStyle = current.primaryCell()->styleRef();
     // FIXME: Make this work with perpendicular and flipped cells.
     const BorderValue& cb = side == BorderBefore
                                 ? primaryCellStyle.borderBefore()
                                 : primaryCellStyle.borderAfter();
     // FIXME: Don't repeat for the same col group
     LayoutTableCol* col =
@@ skipping 300 matching lines @@
 void LayoutTableSection::setNeedsCellRecalc() {
   m_needsCellRecalc = true;
   if (LayoutTable* t = table())
     t->setNeedsSectionRecalc();
 }
 
 unsigned LayoutTableSection::numEffectiveColumns() const {
   unsigned result = 0;
 
   for (unsigned r = 0; r < m_grid.size(); ++r) {
-    for (unsigned c = result; c < numCols(r); ++c) {
+    unsigned nCols = numCols(r);
+    for (unsigned c = result; c < nCols; ++c) {
       const CellStruct& cell = cellAt(r, c);
       if (cell.hasCells() || cell.inColSpan)
         result = c;
     }
   }
 
   return result + 1;
 }
 
 const BorderValue& LayoutTableSection::borderAdjoiningStartCell(
     const LayoutTableCell* cell) const {
   ASSERT(cell->isFirstOrLastCellInRow());
   return hasSameDirectionAs(cell) ? style()->borderStart()
                                   : style()->borderEnd();
 }
 
 const BorderValue& LayoutTableSection::borderAdjoiningEndCell(
     const LayoutTableCell* cell) const {
   ASSERT(cell->isFirstOrLastCellInRow());
   return hasSameDirectionAs(cell) ? style()->borderEnd()
                                   : style()->borderStart();
 }
 
 const LayoutTableCell* LayoutTableSection::firstRowCellAdjoiningTableStart()
     const {
   unsigned adjoiningStartCellColumnIndex =
       hasSameDirectionAs(table()) ? 0 : table()->lastEffectiveColumnIndex();
-  return primaryCellAt(0, adjoiningStartCellColumnIndex);
+  return adjoiningStartCellColumnIndex < numCols(0)
+             ? primaryCellAt(0, adjoiningStartCellColumnIndex)
+             : nullptr;
 }
 
 const LayoutTableCell* LayoutTableSection::firstRowCellAdjoiningTableEnd()
     const {
   unsigned adjoiningEndCellColumnIndex =
       hasSameDirectionAs(table()) ? table()->lastEffectiveColumnIndex() : 0;
-  return primaryCellAt(0, adjoiningEndCellColumnIndex);
+  return adjoiningEndCellColumnIndex < numCols(0)
+             ? primaryCellAt(0, adjoiningEndCellColumnIndex)
+             : nullptr;
 }
 
 void LayoutTableSection::appendEffectiveColumn(unsigned pos) {
   ASSERT(!m_needsCellRecalc);
 
   for (unsigned row = 0; row < m_grid.size(); ++row)
     m_grid[row].row.resize(pos + 1);
 }
 
 void LayoutTableSection::splitEffectiveColumn(unsigned pos, unsigned first) {
@@ skipping 62 matching lines @@
   LayoutRect hitTestRect = LayoutRect(locationInContainer.boundingBox());
   hitTestRect.moveBy(-adjustedLocation);
 
   LayoutRect tableAlignedRect =
       logicalRectForWritingModeAndDirection(hitTestRect);
   CellSpan rowSpan = spannedRows(tableAlignedRect);
   CellSpan columnSpan = spannedEffectiveColumns(tableAlignedRect);
 
   // Now iterate over the spanned rows and columns.
   for (unsigned hitRow = rowSpan.start(); hitRow < rowSpan.end(); ++hitRow) {
-    for (unsigned hitColumn = columnSpan.start(); hitColumn < columnSpan.end();
-         ++hitColumn) {
-      if (hitColumn >= numCols(hitRow))
-        break;
-
+    unsigned nCols = numCols(hitRow);
+    for (unsigned hitColumn = columnSpan.start();
+         hitColumn < nCols && hitColumn < columnSpan.end(); ++hitColumn) {
       CellStruct& current = cellAt(hitRow, hitColumn);
 
       // If the cell is empty, there's nothing to do
       if (!current.hasCells())
         continue;
 
       for (unsigned i = current.cells.size(); i;) {
         --i;
         LayoutTableCell* cell = current.cells[i];
         LayoutPoint cellPoint =
@@ skipping 248 matching lines @@
   // the header in all columns.
   // Note that this is in flow thread coordinates, not visual coordinates. The
   // enclosing LayoutFlowThread will convert to visual coordinates.
   if (table()->header() == this && isRepeatingHeaderGroup())
     rect.setHeight(table()->logicalHeight());
   return LayoutTableBoxComponent::mapToVisualRectInAncestorSpace(ancestor, rect,
                                                                  flags);
 }
 
 }  // namespace blink