Use appropriate background object visual rect for composited table cell backgrounds.

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

Index: third_party/WebKit/Source/core/paint/TableCellPainter.cpp
diff --git a/third_party/WebKit/Source/core/paint/TableCellPainter.cpp b/third_party/WebKit/Source/core/paint/TableCellPainter.cpp
index 2fc5973ee3b9f7a2d94a19dc07925adb6f5f3e76..c1cfde98178b8402e725b6d45455eb197ed7aac0 100644
--- a/third_party/WebKit/Source/core/paint/TableCellPainter.cpp
+++ b/third_party/WebKit/Source/core/paint/TableCellPainter.cpp
@@ -69,14 +69,20 @@ static EBorderStyle collapsedBorderStyle(EBorderStyle style) {
 }
 
 const DisplayItemClient& TableCellPainter::displayItemClientForBorders() const {
-  // TODO(wkorman): We may need to handle PaintInvalidationDelayedFull.
-  // http://crbug.com/657186
-  return m_layoutTableCell.usesTableAsAdditionalDisplayItemClient()
+  return m_layoutTableCell.usesCompositedCellDisplayItemClients()
              ? static_cast<const DisplayItemClient&>(
                    *m_layoutTableCell.collapsedBorderValues())
              : m_layoutTableCell;
 }
 
+const DisplayItemClient& TableCellPainter::displayItemClientForBackground()
+    const {
+  return m_layoutTableCell.usesCompositedCellDisplayItemClients()
+             ? static_cast<const DisplayItemClient&>(
+                   *m_layoutTableCell.rowBackgroundDisplayItemClient())
+             : m_layoutTableCell;
+}
+
 void TableCellPainter::paintCollapsedBorders(
     const PaintInfo& paintInfo,
     const LayoutPoint& paintOffset,
@@ -195,14 +201,15 @@ void TableCellPainter::paintContainerBackgroundBehindCell(
       !m_layoutTableCell.firstChild())
     return;
 
-  if (LayoutObjectDrawingRecorder::useCachedDrawingIfPossible(
-          paintInfo.context, m_layoutTableCell, type))
+  const DisplayItemClient& client = displayItemClientForBackground();
+  if (DrawingRecorder::useCachedDrawingIfPossible(paintInfo.context, client,
+                                                  type))

[Xianzhu, 2016/11/03 03:06:49]

This seems incorrect when we paint background of the containing column or
section behind a cell. In the case we should use table or section visual rect.

Should we paint all backgrounds from a container as a whole display item with
the container as the DisplayItemClient?

Or should we paint all backgrounds from the containers of a cell on the cell's
backing instead of containers' backing, still with the cell as the
DisplayItemClient?

The latter seems more correct, considering rowspan, colspan, etc. 

[wkorman, 2016/11/03 04:02:39]

Hmm, ok. I missed that this was called from non-row code paths, but of course it
would be, I see it now in TableSectionPainter::paintBackgroundsBehindCell.

It's unfortunate we're missing so much test coverage for composited table
cells/rows/etc., given this is wrong we should have had some failing tests. I'd
like to add more to validate that those things break with this change, and then
fix them.

I am however OOO until Monday so I will have to pick this up then unless someone
else can take it.

On your questions, I don't know enough yet on the tradeoffs for the two choices
to pick one. The latter sounds conceptually better, but potentially less
performant?

     return;
 
   LayoutRect paintRect =
       paintRectNotIncludingVisualOverflow(adjustedPaintOffset);
-  LayoutObjectDrawingRecorder recorder(paintInfo.context, m_layoutTableCell,
-                                       type, paintRect);
+  DrawingRecorder recorder(paintInfo.context, client, type,
+                           FloatRect(paintRect));
   paintBackground(paintInfo, paintRect, backgroundObject);
 }