Optimize collapsed border calculation (step 2)

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

Index: third_party/WebKit/Source/core/layout/LayoutTableCell.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutTableCell.cpp b/third_party/WebKit/Source/core/layout/LayoutTableCell.cpp
index 67af45c2514fb14aec3e1f65d1c3a99a5fa1754e..0b6b44c0082086ee757eaa8979309505789df0e0 100644
--- a/third_party/WebKit/Source/core/layout/LayoutTableCell.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutTableCell.cpp
@@ -108,6 +108,9 @@ void LayoutTableCell::WillBeRemovedFromTree() {
   DCHECK(enclosing_table);
   if (!enclosing_table->CollapseBorders())
     return;
+  // TODO(wangxianzhu,dgrogan): The following seems incorrect if the cell has
+  // adjacent cells other than PreviousCell() and NextCell(). Could reuse some
+  // code in LayoutTableCell::InvalidateCollapsedBordersOfAffectedCells().
   if (PreviousCell()) {
     // TODO(dgrogan): Should this be setChildNeedsLayout or setNeedsLayout?
     // remove-cell-with-border-box.html only passes with setNeedsLayout but
@@ -497,11 +500,15 @@ void LayoutTableCell::StyleDidChange(StyleDifference diff,
   if (!table)
     return;
 
-  LayoutTableBoxComponent::InvalidateCollapsedBordersOnStyleChange(
-      *this, *table, diff, *old_style);
+  if (LayoutTableBoxComponent::NeedsInvalidateCollapsedBordersOnStyleChange(
+          *this, *table, diff, *old_style))
+    InvalidateCollapsedBordersOfAffectedCells();
 
   if (LayoutTableBoxComponent::DoCellsHaveDirtyWidth(*this, *table, diff,
                                                      *old_style)) {
+    // TODO(wangxianzhu,dgrogan): The following seems incorrect if the cell has

[dgrogan, 2017/04/27 20:43:25]

Could you expound on this a little? When can a cell have adjacent cells other
than PreviousCell and NextCell? Do you mean above and below? Or something with
spans? Or something else entirely?

[Xianzhu, 2017/04/27 21:18:31]

Actually I'm not very sure if the TODO is valid. I will remove it if it's
invalid, otherwise will make it clearer.

For example:
<tr>
  <td rowspan="2">A</td><td>B</td>
</tr>
<tr>
  <td>C</td>
</tr>

It seems that A's border width may affect preferred width of both B and C.

(However, I'm not sure if C's border width should affect A. A's right border
actually has two segments which may differ in width. Currently LayoutTableCell
supports only one CollapsedBorderValue in each side so for layout it seems that
we just consider the first segment for now. @robhogan's
https://codereview.chromium.org/2791433003/ will support multiple segments
though.)

+    // adjacent cells other than PreviousCell() and NextCell(). Could reuse some
+    // code in LayoutTableCell::InvalidateCollapsedBordersOfAffectedCells().
     if (PreviousCell()) {
       // TODO(dgrogan) Add a layout test showing that setChildNeedsLayout is
       // needed instead of setNeedsLayout.
@@ -1261,6 +1268,44 @@ void LayoutTableCell::Paint(const PaintInfo& paint_info,
   TableCellPainter(*this).Paint(paint_info, paint_offset);
 }
 
+void LayoutTableCell::InvalidateCollapsedBordersOfAffectedCells() {
+  auto* table = this->Table();
+  DCHECK(table && table->CollapseBorders());
+  if (table->NeedsInvalidateCollapsedBordersForAllCells())
+    return;
+
+  table->RecalcSectionsIfNeeded();
+  table->InvalidateCollapsedBorders();
+
+  InvalidateCollapsedBorderValues();
+
+  auto col_span = this->ColSpan();
+  auto row_span = this->RowSpan();
+  auto row = RowIndex();
+  auto effective_column =
+      table->AbsoluteColumnToEffectiveColumn(AbsoluteColumnIndex());
+  auto end_effective_column =
+      table->AbsoluteColumnToEffectiveColumn(AbsoluteColumnIndex() + col_span);
+  auto* section = this->Section();
+
+  // Invalidate cells above and below this cell.
+  for (auto c = effective_column; c < end_effective_column; ++c) {
+    if (auto* cell = section->PrimaryCellAboveInTable(row, c))
+      cell->InvalidateCollapsedBorderValues();
+    if (auto* cell = section->PrimaryCellBelowInTable(row, c))
+      cell->InvalidateCollapsedBorderValues();
+  }
+  // Invalidate cells before and after this cell.
+  for (unsigned i = 0; i < row_span; ++i) {
+    if (effective_column > 0) {
+      if (auto* cell = section->PrimaryCellAt(row + i, effective_column - 1))
+        cell->InvalidateCollapsedBorderValues();
+    }
+    if (auto* cell = section->PrimaryCellAt(row + i, end_effective_column))
+      cell->InvalidateCollapsedBorderValues();
+  }
+}
+
 void LayoutTableCell::RecalcCollapsedBorderValuesIfNeeded() const {
   Table()->InvalidateCollapsedBordersForAllCellsIfNeeded();
   if (collapsed_border_values_valid_)