Fix spanning cell painting background from wrong row

third_party/WebKit/Source/core/layout/LayoutTableSection.h

 /*
  * 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, 2009, 2013 Apple Inc. All rights
  * reserved.
  *
  * This library is free software; you can redistribute it and/or
@@ skipping 123 matching lines @@
     // This Vector is sorted in DOM order.
     Vector<LayoutTableCell*, 1> cells;
     bool inColSpan;  // true for columns after the first in a colspan
 
     CellStruct();
     ~CellStruct();
 
     // This is the cell in the grid "slot" that is on top of the others
     // (aka the last cell in DOM order for this slot).
     //
-    // This is the cell originating from this slot if it exists.
+    // Multiple grid slots can have the same primary cell if the cell spans
+    // into the grid slots. The slot having the smallest row index and
+    // smallest effective column index is the originating slot of the cell.
     //
     // The concept of a primary cell is dubious at most as it doesn't
     // correspond to a DOM or rendering concept. Also callers should be
     // careful about assumptions about it. For example, even though the
     // primary cell is visibly the top most, it is not guaranteed to be
     // the only one visible for this slot due to different visual
     // overflow rectangles.
     LayoutTableCell* primaryCell() {
       return hasCells() ? cells[cells.size() - 1] : 0;
     }
@@ skipping 67 matching lines @@
     if (effectiveColumn >= rowVector.size())
       return nullptr;
     return rowVector[effectiveColumn].primaryCell();
   }
   const LayoutTableCell* primaryCellAt(unsigned row,
                                        unsigned effectiveColumn) const {
     return const_cast<LayoutTableSection*>(this)->primaryCellAt(
         row, effectiveColumn);
   }
 
+  // Returns the primary cell at (row, effectiveColumn) if the cell exists and
+  // originates from (instead of spanning into) the grid slot, or nullptr.
+  LayoutTableCell* originatingCellAt(unsigned row, unsigned effectiveColumn) {

[wkorman, 2017/03/29 18:12:33]

Do these methods have to be implemented in header? Prefer putting in .cpp if we
can for faster build time when modifying.

[Xianzhu, 2017/03/29 18:44:41]

Done.

+    LayoutTableCell* cell = primaryCellAt(row, effectiveColumn);
+    if ((!row || primaryCellAt(row - 1, effectiveColumn) != cell) &&
+        (!effectiveColumn || primaryCellAt(row, effectiveColumn - 1) != cell))
+      return cell;
+    return nullptr;
+  }
+  const LayoutTableCell* originatingCellAt(unsigned row,
+                                           unsigned effectiveColumn) const {
+    return const_cast<LayoutTableSection*>(this)->originatingCellAt(

[wkorman, 2017/03/29 18:12:33]

Why do we need the const_cast and the separate method? Is primaryCellAt not
const?

[Xianzhu, 2017/03/29 18:44:41]

This follows the strict-const style which is used in primaryCell etc., like:

  // non const version.
  Something* method() { ... }
  // const version.
  const SomeThing* method() const { return const_cast<Class*>(this)->method(); }

I usually use this style for new methods if the sibling methods are already in
this style.

Otherwise I would use the loose-const style:
  Something* method() const { ... }

[Xianzhu, 2017/03/29 18:49:12]

As only the const version is used, I will remove the non-const one.

+        row, effectiveColumn);
+  }
+
   unsigned numCols(unsigned row) const { return m_grid[row].row.size(); }
 
   // Returns null for cells with a rowspan that exceed the last row. Possibly
   // others.
   LayoutTableRow* rowLayoutObjectAt(unsigned row) {
     return m_grid[row].rowLayoutObject;
   }
   const LayoutTableRow* rowLayoutObjectAt(unsigned row) const {
     return m_grid[row].rowLayoutObject;
   }
@@ skipping 51 matching lines @@
       const LayoutObject* parent) const override {
     return createAnonymousWithParent(parent);
   }
 
   void paint(const PaintInfo&, const LayoutPoint&) const override;
 
   // Flip the rect so it aligns with the coordinates used by the rowPos and
   // columnPos vectors.
   LayoutRect logicalRectForWritingModeAndDirection(const LayoutRect&) const;
 
+  // Returns a row or column span covering all grid slots from each of which
+  // a primary cell intersecting visualRect originates.

[wkorman, 2017/03/29 18:12:33]

|visualRect|

[Xianzhu, 2017/03/29 18:44:41]

Done.

   CellSpan dirtiedRows(const LayoutRect& visualRect) const;
   CellSpan dirtiedEffectiveColumns(const LayoutRect& visualRect) const;
+
   const HashSet<LayoutTableCell*>& overflowingCells() const {
     return m_overflowingCells;
   }
   bool hasMultipleCellLevels() const { return m_hasMultipleCellLevels; }
 
   const char* name() const override { return "LayoutTableSection"; }
 
   // Whether a section has opaque background depends on many factors, e.g.
   // border spacing, border collapsing, missing cells, etc. For simplicity,
   // just conservatively assume all table sections are not opaque.
@@ skipping 153 matching lines @@
   // memory.
   HashSet<LayoutTableCell*> m_overflowingCells;
   bool m_forceSlowPaintPathWithOverflowingCell;
 
   // This boolean tracks if we have cells overlapping due to rowspan / colspan
   // (see class comment above about when it could appear).
   //
   // The use is to disable a painting optimization where we just paint the
   // invalidated cells.
   bool m_hasMultipleCellLevels;
+
+  // This is set if there is any cell spanning multiple rows or cols.

[wkorman, 2017/03/29 18:12:33]

Slight rephrase for clarity, something like, "Whether any cell spans multiple
rows or cols."
since one could set it to false and it would still have been "set".

[Xianzhu, 2017/03/29 18:44:41]

Done.

+  bool m_hasSpanningCells;
 };
 
 DEFINE_LAYOUT_OBJECT_TYPE_CASTS(LayoutTableSection, isTableSection());
 
 }  // namespace blink
 
 #endif  // LayoutTableSection_h