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

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

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/paint/TableCellPainter.h"
 
 #include "core/layout/LayoutTableCell.h"
 #include "core/paint/BlockPainter.h"
 #include "core/paint/BoxPainter.h"
 #include "core/paint/LayoutObjectDrawingRecorder.h"
@@ skipping 51 matching lines @@
 
 static EBorderStyle collapsedBorderStyle(EBorderStyle style) {
   if (style == BorderStyleOutset)
     return BorderStyleGroove;
   if (style == BorderStyleInset)
     return BorderStyleRidge;
   return 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,
     const CollapsedBorderValue& currentBorderValue) {
   if (m_layoutTableCell.style()->visibility() != EVisibility::Visible)
     return;
 
   LayoutPoint adjustedPaintOffset = paintOffset + m_layoutTableCell.location();
   if (!BlockPainter(m_layoutTableCell)
            .intersectsPaintRect(paintInfo, adjustedPaintOffset))
@@ skipping 98 matching lines @@
   if (!BlockPainter(m_layoutTableCell)
            .intersectsPaintRect(paintInfo, adjustedPaintOffset))
     return;
 
   LayoutTable* table = m_layoutTableCell.table();
   if (!table->collapseBorders() &&
       m_layoutTableCell.style()->emptyCells() == EEmptyCells::Hide &&
       !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);
 }
 
 void TableCellPainter::paintBackground(const PaintInfo& paintInfo,
                                        const LayoutRect& paintRect,
                                        const LayoutObject& backgroundObject) {
   if (m_layoutTableCell.backgroundStolenForBeingBody())
     return;
 
   Color c = backgroundObject.resolveColor(CSSPropertyBackgroundColor);
@@ skipping 82 matching lines @@
   BoxPainter(m_layoutTableCell).paintMaskImages(paintInfo, paintRect);
 }
 
 LayoutRect TableCellPainter::paintRectNotIncludingVisualOverflow(
     const LayoutPoint& paintOffset) {
   return LayoutRect(paintOffset,
                     LayoutSize(m_layoutTableCell.pixelSnappedSize()));
 }
 
 }  // namespace blink