Skip PaintPhaseDescendantBlockBackgroundsOnly phase if possible

third_party/WebKit/Source/core/paint/TableRowPainter.cpp

Index: third_party/WebKit/Source/core/paint/TableRowPainter.cpp
diff --git a/third_party/WebKit/Source/core/paint/TableRowPainter.cpp b/third_party/WebKit/Source/core/paint/TableRowPainter.cpp
index b249ca929cc002e16f6c4c1830ecbda7f794e9f5..b7c333e0b4a8f13c7777cdf8bc27d333803d3d09 100644
--- a/third_party/WebKit/Source/core/paint/TableRowPainter.cpp
+++ b/third_party/WebKit/Source/core/paint/TableRowPainter.cpp
@@ -17,25 +17,18 @@ void TableRowPainter::paint(const PaintInfo& paintInfo, const LayoutPoint& paint
 {
     ASSERT(m_layoutTableRow.hasSelfPaintingLayer());
 
-    // Table rows don't paint self background. The cells paint table section's background
-    // behind them when needed during PaintPhaseBlockBackground or PaintPhaseDescendantBlockBackgroundOnly.
-    if (paintInfo.phase == PaintPhaseSelfBlockBackgroundOnly)
-        return;
-
     // TODO(wangxianzhu): This painting order is inconsistent with other outlines. crbug.com/577282.
     paintOutlineForRowIfNeeded(paintInfo, paintOffset);
     if (paintInfo.phase == PaintPhaseSelfOutlineOnly)
         return;
 
     PaintInfo paintInfoForCells = paintInfo.forDescendants();
+    bool shouldPaintRowBackground = shouldPaintSelfBlockBackground(paintInfo.phase) && m_layoutTableRow.hasBackground();
+    bool shouldPaintCells = paintInfo.phase != PaintPhaseSelfBlockBackgroundOnly;
     for (LayoutTableCell* cell = m_layoutTableRow.firstCell(); cell; cell = cell->nextCell()) {
-        // Paint the row background behind the cell.
-        if (shouldPaintSelfBlockBackground(paintInfoForCells.phase)) {
-            if (m_layoutTableRow.hasBackground())
-                TableCellPainter(*cell).paintBackgroundsBehindCell(paintInfoForCells, paintOffset, &m_layoutTableRow, DisplayItem::TableCellBackgroundFromRow);
-        }
-
-        if (!cell->hasSelfPaintingLayer())
+        if (shouldPaintRowBackground)
+            TableCellPainter(*cell).paintBackgroundsBehindCell(paintInfoForCells, paintOffset, &m_layoutTableRow, DisplayItem::TableCellBackgroundFromRow);

[chrishtr, 2016/01/27 23:51:47]

Won't this paint twice now? For PaintPhaseSelfBlockBackgroundOnly and
PaintPhaseBlockBackground?

Also, it would be better to break this out into its own patch. The changes to
table painting here
are not trivial to verify.

[Xianzhu, 2016/01/28 00:13:13]

No. PaintPhaseBlockBackground is like (PaintPhaseSelfBlockBackgroundOnly |
PaintPhaseDescendantBlockBackgroundsOnly) (but we can't actually use them as
bits, so use shouldPaintSelfBlockBackground() and
shouldPaintDescendantBlockBackgrounds()). PaintPhaseBlockBackground never happen
when either or both of the xxxOnly phases happens.
Done. Ptal at https://codereview.chromium.org/1646743002.

+        if (shouldPaintCells && !cell->hasSelfPaintingLayer())
             cell->paint(paintInfoForCells, paintOffset);
     }
 }