Table cell background painting

third_party/WebKit/Source/core/layout/LayoutTableCol.cpp

Index: third_party/WebKit/Source/core/layout/LayoutTableCol.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutTableCol.cpp b/third_party/WebKit/Source/core/layout/LayoutTableCol.cpp
index aaf594c9e7481959cd7c6dcd0ae31103047c1b45..3b41c73b48692fc1cf1261f7d32a647b93d30daf 100644
--- a/third_party/WebKit/Source/core/layout/LayoutTableCol.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutTableCol.cpp
@@ -114,11 +114,9 @@ LayoutRect LayoutTableCol::clippedOverflowRectForPaintInvalidation(const LayoutB
     // FIXME: Find a better way to do this, e.g., need to paint invalidate all the cells that we
     // might have propagated a background color or borders into.
     // FIXME: check for paintInvalidationContainer each time here?

[Xianzhu, 2016/03/09 18:40:55]

Remove the above FIXMEs.

[atotic1, 2016/03/15 16:50:48]

Done.

-
-    LayoutTable* parentTable = table();
-    if (!parentTable)
-        return LayoutRect();
-    return parentTable->clippedOverflowRectForPaintInvalidation(paintInvalidationContainer, paintInvalidationState);
+    LayoutRect r = positionByCellSpan();
+    mapToVisibleRectInAncestorSpace(paintInvalidationContainer, r, paintInvalidationState);
+    return r;
 }
 
 void LayoutTableCol::imageChanged(WrappedImagePtr, const IntRect*)
@@ -154,6 +152,29 @@ LayoutTableCol* LayoutTableCol::enclosingColumnGroup() const
     return parentColumnGroup;
 }
 
+LayoutTableCol * LayoutTableCol::lastColumnInGroup() const

[Xianzhu, 2016/03/09 18:40:55]

Remove extra space before '*'.

[atotic1, 2016/03/15 16:50:48]

Done.

+{
+    ASSERT(this->isTableColumnGroup());
+
+    LayoutTableCol * endCol = 0;
+    LayoutTableCol * currentCol = this->nextColumn();

[Xianzhu, 2016/03/09 18:40:56]

Remove extra spaces.

[atotic1, 2016/03/15 16:50:48]

Done.

+    // if column has children, traverse the children to find last

[Xianzhu, 2016/03/09 18:40:55]

Start a sentence with uppercase letter. End a sentence with a '.'.

[atotic1, 2016/03/15 16:50:48]

Done.

+    if (this->firstChild()) {
+        while (currentCol && currentCol->enclosingColumnGroup() == this) {
+            endCol = currentCol;
+            currentCol = currentCol->nextColumn();
+        }
+    } else { // otherwise, if we have span, use that to find next child
+        auto span = this->span();
+        endCol = currentCol;
+        while (--span > 0 && currentCol) {
+            currentCol = currentCol->nextColumn();
+            endCol = currentCol;
+        }
+    }
+    return endCol;
+}
+
 LayoutTableCol* LayoutTableCol::nextColumn() const
 {
     // If |this| is a column-group, the next column is the colgroup's first child column.
@@ -164,8 +185,9 @@ LayoutTableCol* LayoutTableCol::nextColumn() const
     LayoutObject* next = nextSibling();
 
     // Failing that, the child is the last column in a column-group, so the next column is the next column/column-group after its column-group.
-    if (!next && parent()->isLayoutTableCol())
-        next = parent()->nextSibling();
+    auto p = parent();
+    if (!next && p && p->isLayoutTableCol())
+        next = p->nextSibling();
 
     for (; next && !next->isLayoutTableCol(); next = next->nextSibling()) { }
 
@@ -194,4 +216,74 @@ const BorderValue& LayoutTableCol::borderAdjoiningCellAfter(const LayoutTableCel
     return style()->borderEnd();
 }
 
+// computes col/colgroup position that only spans cells

[Xianzhu, 2016/03/09 18:40:55]

Move the comment into header file.

[atotic1, 2016/03/15 16:50:48]

Done.

+LayoutRect LayoutTableCol::positionByCellSpan() const
+{
+
+    LayoutTable * myTable = table();

[Xianzhu, 2016/03/09 18:40:56]

Suggestion: LayoutTable* table = this->table();

[atotic1, 2016/03/15 16:50:48]

Done.

+    LayoutRect position;
+
+    if (!myTable)
+        return position;

[Xianzhu, 2016/03/09 18:40:56]

We don't need to check null of table().

[atotic1, 2016/03/15 16:50:48]

table() can return nullptr. and I have to guard against it.

+
+    LayoutTableSection* topSection = myTable->topNonEmptySection();
+    LayoutTableSection* bottomSection = myTable->bottomNonEmptySection();
+    if (!topSection || !bottomSection)
+        return position;
+
+    Vector<unsigned> colIndexes = getEffectiveColumnIndexes();
+
+    if (colIndexes.size() == 0)
+        return position;
+
+    unsigned startColumnIndex = colIndexes[0];
+    unsigned endColumnIndex = colIndexes.last();
+
+    position = topSection->positionOfIdealCell(0, startColumnIndex);
+    position.moveBy(topSection->location());
+    LayoutRect bottomPosition = bottomSection->positionOfIdealCell(bottomSection->numRows() - 1, endColumnIndex);
+    bottomPosition.moveBy(bottomSection->location());
+    position.unite(bottomPosition);
+    return position;

[Xianzhu, 2016/03/09 20:15:57]

What if the column contains some cell having a span bigger than the column's
span?

[Xianzhu, 2016/03/09 20:24:34]

The test case for my question:

<!DOCTYPE html>
<table>
  <col style="background-color: green">
  <col style="background-color: blue">
  <tr>
    <td>Cell11</td>
    <td>Cell12</td>
  <tr>
    <td colspan="2">Cell2</td>
  </tr>
</table>

The first column actually covers Cell11 and Cell2, not just the 'ideal cells'
1,1 and 2,1. You might need to unite overflow rects of all actual cells in the
column instead of just uniting the top and bottom 'ideal cells'.

An alternative way to fix the column paint invalidation issue might be to keep
the original non-optimal method (that uses table's invalidation rect for
columns), but just fix the offset issue.

[atotic1, 2016/03/15 16:50:48]

That's ok. We are not looking for cell position, but position of the column. If
the caller needs a region spanned by cells that intersect this region, that's a
different call.

+}
+
+// returns indexes to all columns that span this column
+// could return vector with size 0 if out of range

[Xianzhu, 2016/03/09 18:40:55]

Ditto (comment style).

[atotic1, 2016/03/15 16:50:48]

Done.

+Vector<unsigned> LayoutTableCol::getEffectiveColumnIndexes() const
+{
+    LayoutTable * myTable = table();
+    Vector<unsigned> indexes;
+    if (isTableColumn()) {
+        unsigned idx = myTable->colElementToEffectiveColumn(this);
+        if (idx != LayoutTable::npos) {
+            auto span = this->span();
+            while (span--) {
+                if (idx < myTable->numEffectiveColumns())
+                    indexes.append(idx);
+            }
+        }
+    } else {
+        // isTableColumnGroup
+        if (isTableColumnGroupWithColumnChildren()) {
+            LayoutTableCol* currentCol = nullptr;
+            auto lastCol = lastColumnInGroup();
+            do {
+                currentCol = currentCol ? currentCol->nextColumn() : nextColumn();
+                if (currentCol) {
+                    auto colIndexes = currentCol->getEffectiveColumnIndexes();
+                    for (auto c = colIndexes.begin(); c != colIndexes.end(); c++)
+                        indexes.append(*c);
+                }
+            } while (currentCol && currentCol != lastCol);
+        } else {
+            unsigned idx = myTable->colElementToEffectiveColumn(this);
+            auto span = this->span();
+            while (span--) {
+                if (idx < myTable->numEffectiveColumns())
+                    indexes.append(idx);
+            }
+        }
+    }
+    return indexes;
+}
 } // namespace blink