[css-tables] Set table and cell widths dirty when section border changes

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

Index: third_party/WebKit/Source/core/layout/LayoutTableSection.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutTableSection.cpp b/third_party/WebKit/Source/core/layout/LayoutTableSection.cpp
index 330709877e78c9eb82ea2f75984d83f8f1108141..1095c46637a9556bac9598477bbe1d680e3ecafc 100644
--- a/third_party/WebKit/Source/core/layout/LayoutTableSection.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutTableSection.cpp
@@ -125,6 +125,10 @@ void LayoutTableSection::styleDidChange(StyleDifference diff, const ComputedStyl
     LayoutTable* table = this->table();
     if (table && !table->selfNeedsLayout() && !table->normalChildNeedsLayout() && oldStyle && oldStyle->border() != style()->border())
         table->invalidateCollapsedBorders();
+
+    if (table && oldStyle && diff.needsFullLayout() && needsLayout() && table->collapseBorders() && borderWidthChanged(oldStyle, style())) {
+        markAllCellsWidthsDirtyAndOrNeedsLayout(true /*markNeedsLayout*/);
+    }
 }
 
 void LayoutTableSection::willBeRemovedFromTree()
@@ -1216,6 +1220,20 @@ bool LayoutTableSection::recalcChildOverflowAfterStyleChange()
     return childrenOverflowChanged;
 }
 
+void LayoutTableSection::markAllCellsWidthsDirtyAndOrNeedsLayout(bool markNeedsLayout)
+{
+    for (unsigned i = 0; i < numRows(); i++) {
+        LayoutTableRow* row = rowLayoutObjectAt(i);
+        if (!row)

[eae, 2016/07/01 22:20:25]

When would row ever be null? Sparse tables?

[dgrogan, 2016/07/06 23:10:29]

Good question. I don't think it will be. I cargo culted the code from
TableLayoutAlgorithmFixed.cpp. It was added in https://crrev.com/27262002 (Need
to set preferred logical widths dirty bits when switching table layout). Adding
trchen@ to see if he remembers what it was for.

Removing the conditional doesn't cause any layout tests to fail -- what do you
think of the strategy hinted at by the comments? Meaning:
(1) collect crash reports for at most 1 release
(2) if there are none replace the CHECK with DCHECK
(3) if there are some re-add the conditional at least with comments about why
it's there, hopefully with a layout test that exercises it

[trchen, 2016/07/11 21:12:32]

I don't remember, but other functions in LayoutTableSection does the null check
as well. I think it may have something to do with cells with rowspan that
exceeded the last row.

[dgrogan, 2016/07/11 22:27:16]

You nailed it; cells with rowspan that exceeded the last row have null
LayoutTableRow objects.

+            continue;
+        for (LayoutTableCell* cell = row->firstCell(); cell; cell = cell->nextCell()) {
+            cell->setPreferredLogicalWidthsDirty();
+            if (markNeedsLayout)
+                cell->setChildNeedsLayout();
+        }
+    }
+}
+
 int LayoutTableSection::calcBlockDirectionOuterBorder(BlockBorderSide side) const
 {
     unsigned totalCols = table()->numEffectiveColumns();