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

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

 /*
  * 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, 2008, 2009, 2010, 2013 Apple Inc. All rights reserved.
  * Copyright (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  *
  * This library is free software; you can redistribute it and/or
@@ skipping 107 matching lines @@
 
 void LayoutTableSection::styleDidChange(StyleDifference diff, const ComputedStyle* oldStyle)
 {
     LayoutTableBoxComponent::styleDidChange(diff, oldStyle);
     propagateStyleToAnonymousChildren();
 
     // If border was changed, notify table.
     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()
 {
     LayoutTableBoxComponent::willBeRemovedFromTree();
 
     // Preventively invalidate our cells as we may be re-inserted into
     // a new table which would require us to rebuild our structure.
     setNeedsCellRecalc();
 }
@@ skipping 1071 matching lines @@
         if (rowChildrenOverflowChanged)
             rowLayouter->computeOverflow();
         childrenOverflowChanged |= rowChildrenOverflowChanged;
     }
     // TODO(crbug.com/604136): Add visual overflow from rows too.
     if (childrenOverflowChanged)
         computeOverflowFromCells(totalRows, numEffCols);
     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();
     if (!m_grid.size() || !totalCols)
         return 0;
 
     int borderWidth = 0;
 
     const BorderValue& sb = side == BorderBefore ? style()->borderBefore() : style()->borderAfter();
     if (sb.style() == BorderStyleHidden)
@@ skipping 450 matching lines @@
     // FIXME: The table's direction should determine our row's direction, not the section's (see bug 96691).
     if (!style()->isLeftToRightDirection())
         cellLocation.setX(LayoutUnit(table()->effectiveColumnPositions()[table()->numEffectiveColumns()] - table()->effectiveColumnPositions()[table()->absoluteColumnToEffectiveColumn(cell->absoluteColumnIndex() + cell->colSpan())] + horizontalBorderSpacing));
     else
         cellLocation.setX(LayoutUnit(table()->effectiveColumnPositions()[effectiveColumn] + horizontalBorderSpacing));
 
     cell->setLogicalLocation(cellLocation);
 }
 
 } // namespace blink