Optimize collapsed border calculation (step 1)

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 609c713bb82745b5aa75a938fe01ffc0c1edc31a..67af45c2514fb14aec3e1f65d1c3a99a5fa1754e 100644
--- a/third_party/WebKit/Source/core/layout/LayoutTableCell.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutTableCell.cpp
@@ -59,6 +59,8 @@ LayoutTableCell::LayoutTableCell(Element* element)
     : LayoutBlockFlow(element),
       absolute_column_index_(kUnsetColumnIndex),
       cell_width_changed_(false),
+      collapsed_border_values_valid_(false),
+      collapsed_borders_visually_changed_(false),
       intrinsic_padding_before_(0),
       intrinsic_padding_after_(0) {
   // We only update the flags when notified of DOM changes in
@@ -596,36 +598,34 @@ bool LayoutTableCell::HasEndBorderAdjoiningTable() const {
   return HasSameDirectionAs(Table()) ? IsInEndColumn() : IsInStartColumn();
 }
 
-CollapsedBorderValue LayoutTableCell::ComputeCollapsedStartBorder(
-    IncludeBorderColorOrNot include_color) const {
+CollapsedBorderValue LayoutTableCell::ComputeCollapsedStartBorder() const {
   LayoutTable* table = this->Table();
+  LayoutTableCell* cell_before = table->CellBefore(this);
+  // We can use the border shared with cellBefore if it is valid.

[wkorman, 2017/04/27 20:11:15]

cellBefore -> |cell_before|

[Xianzhu, 2017/04/28 20:31:44]

Done.

+  if (cell_before && cell_before->collapsed_border_values_valid_ &&
+      cell_before->RowIndex() == RowIndex()) {
+    return cell_before->GetCollapsedBorderValues()
+               ? cell_before->GetCollapsedBorderValues()->EndBorder()
+               : CollapsedBorderValue();
+  }
 
   // For the start border, we need to check, in order of precedence:
   // (1) Our start border.
-  int start_color_property = include_color
-                                 ? CSSProperty::ResolveDirectionAwareProperty(
-                                       CSSPropertyWebkitBorderStartColor,
-                                       StyleForCellFlow().Direction(),
-                                       StyleForCellFlow().GetWritingMode())
-                                 : 0;
-  int end_color_property =
-      include_color
-          ? CSSProperty::ResolveDirectionAwareProperty(
-                CSSPropertyWebkitBorderEndColor, StyleForCellFlow().Direction(),
-                StyleForCellFlow().GetWritingMode())
-          : 0;
-  CollapsedBorderValue result(
-      Style()->BorderStart(),
-      include_color ? ResolveColor(start_color_property) : Color(),
-      kBorderPrecedenceCell);
+  int start_color_property = CSSProperty::ResolveDirectionAwareProperty(
+      CSSPropertyWebkitBorderStartColor, StyleForCellFlow().Direction(),
+      StyleForCellFlow().GetWritingMode());
+  int end_color_property = CSSProperty::ResolveDirectionAwareProperty(
+      CSSPropertyWebkitBorderEndColor, StyleForCellFlow().Direction(),
+      StyleForCellFlow().GetWritingMode());
+  CollapsedBorderValue result(Style()->BorderStart(),
+                              ResolveColor(start_color_property),
+                              kBorderPrecedenceCell);
 
   // (2) The end border of the preceding cell.
-  LayoutTableCell* cell_before = table->CellBefore(this);
   if (cell_before) {
     CollapsedBorderValue cell_before_adjoining_border = CollapsedBorderValue(
         cell_before->BorderAdjoiningCellAfter(this),
-        include_color ? cell_before->ResolveColor(end_color_property) : Color(),
-        kBorderPrecedenceCell);
+        cell_before->ResolveColor(end_color_property), kBorderPrecedenceCell);
     // |result| should be the 2nd argument as |cellBefore| should win in case of
     // equality per CSS 2.1 (Border conflict resolution, point 4).
     result = ChooseBorder(cell_before_adjoining_border, result);
@@ -637,21 +637,19 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedStartBorder(
   if (start_border_adjoins_table) {
     // (3) Our row's start border.
     result = ChooseBorder(
-        result, CollapsedBorderValue(
-                    Row()->BorderAdjoiningStartCell(this),
-                    include_color ? Parent()->ResolveColor(start_color_property)
-                                  : Color(),
-                    kBorderPrecedenceRow));
+        result,
+        CollapsedBorderValue(Row()->BorderAdjoiningStartCell(this),
+                             Parent()->ResolveColor(start_color_property),
+                             kBorderPrecedenceRow));
     if (!result.Exists())
       return result;
 
     // (4) Our row group's start border.
     result = ChooseBorder(
-        result, CollapsedBorderValue(Section()->BorderAdjoiningStartCell(this),
-                                     include_color ? Section()->ResolveColor(
-                                                         start_color_property)
-                                                   : Color(),
-                                     kBorderPrecedenceRowGroup));
+        result,
+        CollapsedBorderValue(Section()->BorderAdjoiningStartCell(this),
+                             Section()->ResolveColor(start_color_property),
+                             kBorderPrecedenceRowGroup));
     if (!result.Exists())
       return result;
   }
@@ -666,9 +664,7 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedStartBorder(
         result,
         CollapsedBorderValue(
             col_and_col_group.colgroup->BorderAdjoiningCellStartBorder(this),
-            include_color
-                ? col_and_col_group.colgroup->ResolveColor(start_color_property)
-                : Color(),
+            col_and_col_group.colgroup->ResolveColor(start_color_property),
             kBorderPrecedenceColumnGroup));
     if (!result.Exists())
       return result;
@@ -681,9 +677,7 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedStartBorder(
     result = ChooseBorder(
         result, CollapsedBorderValue(
                     col_and_col_group.col->BorderAdjoiningCellStartBorder(this),
-                    include_color ? col_and_col_group.col->ResolveColor(
-                                        start_color_property)
-                                  : Color(),
+                    col_and_col_group.col->ResolveColor(start_color_property),
                     kBorderPrecedenceColumn));
     if (!result.Exists())
       return result;
@@ -699,9 +693,7 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedStartBorder(
       result = ChooseBorder(
           CollapsedBorderValue(
               col_and_col_group.colgroup->BorderAdjoiningCellEndBorder(this),
-              include_color
-                  ? col_and_col_group.colgroup->ResolveColor(end_color_property)
-                  : Color(),
+              col_and_col_group.colgroup->ResolveColor(end_color_property),
               kBorderPrecedenceColumnGroup),
           result);
       if (!result.Exists())
@@ -715,9 +707,7 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedStartBorder(
       result = ChooseBorder(
           CollapsedBorderValue(
               col_and_col_group.col->BorderAdjoiningCellAfter(this),
-              include_color
-                  ? col_and_col_group.col->ResolveColor(end_color_property)
-                  : Color(),
+              col_and_col_group.col->ResolveColor(end_color_property),
               kBorderPrecedenceColumn),
           result);
       if (!result.Exists())
@@ -728,11 +718,9 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedStartBorder(
   if (start_border_adjoins_table) {
     // (7) The table's start border.
     result = ChooseBorder(
-        result,
-        CollapsedBorderValue(
-            table->TableStartBorderAdjoiningCell(this),
-            include_color ? table->ResolveColor(start_color_property) : Color(),
-            kBorderPrecedenceTable));
+        result, CollapsedBorderValue(table->TableStartBorderAdjoiningCell(this),
+                                     table->ResolveColor(start_color_property),
+                                     kBorderPrecedenceTable));
     if (!result.Exists())
       return result;
   }
@@ -740,68 +728,60 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedStartBorder(
   return result;
 }
 
-CollapsedBorderValue LayoutTableCell::ComputeCollapsedEndBorder(
-    IncludeBorderColorOrNot include_color) const {
+CollapsedBorderValue LayoutTableCell::ComputeCollapsedEndBorder() const {
   LayoutTable* table = this->Table();
   // Note: We have to use the effective column information instead of whether we
   // have a cell after as a table doesn't have to be regular (any row can have
   // less cells than the total cell count).
-  bool is_end_column = table->AbsoluteColumnToEffectiveColumn(
-                           AbsoluteColumnIndex() + ColSpan() - 1) ==
-                       table->NumEffectiveColumns() - 1;
+  bool is_end_column = IsInEndColumn();
+  LayoutTableCell* cell_after =
+      is_end_column ? nullptr : table->CellAfter(this);
+  // We can use the border shared with cellAfter if it is valid.
+  if (cell_after && cell_after->collapsed_border_values_valid_ &&
+      cell_after->RowIndex() == RowIndex()) {
+    return cell_after->GetCollapsedBorderValues()
+               ? cell_after->GetCollapsedBorderValues()->StartBorder()
+               : CollapsedBorderValue();
+  }
 
   // For end border, we need to check, in order of precedence:
   // (1) Our end border.
-  int start_color_property = include_color
-                                 ? CSSProperty::ResolveDirectionAwareProperty(
-                                       CSSPropertyWebkitBorderStartColor,
-                                       StyleForCellFlow().Direction(),
-                                       StyleForCellFlow().GetWritingMode())
-                                 : 0;
-  int end_color_property =
-      include_color
-          ? CSSProperty::ResolveDirectionAwareProperty(
-                CSSPropertyWebkitBorderEndColor, StyleForCellFlow().Direction(),
-                StyleForCellFlow().GetWritingMode())
-          : 0;
+  int start_color_property = CSSProperty::ResolveDirectionAwareProperty(

[wkorman, 2017/04/27 20:11:15]

We do similar to these 3 lines about 8 times in this file, perhaps worth
shortening via static helper function or similar.

[Xianzhu, 2017/04/28 20:31:44]

Done.

+      CSSPropertyWebkitBorderStartColor, StyleForCellFlow().Direction(),
+      StyleForCellFlow().GetWritingMode());
+  int end_color_property = CSSProperty::ResolveDirectionAwareProperty(
+      CSSPropertyWebkitBorderEndColor, StyleForCellFlow().Direction(),
+      StyleForCellFlow().GetWritingMode());
   CollapsedBorderValue result = CollapsedBorderValue(
-      Style()->BorderEnd(),
-      include_color ? ResolveColor(end_color_property) : Color(),
+      Style()->BorderEnd(), ResolveColor(end_color_property),
       kBorderPrecedenceCell);
 
   // (2) The start border of the following cell.
-  if (!is_end_column) {
-    if (LayoutTableCell* cell_after = table->CellAfter(this)) {
-      CollapsedBorderValue cell_after_adjoining_border = CollapsedBorderValue(
-          cell_after->BorderAdjoiningCellBefore(this),
-          include_color ? cell_after->ResolveColor(start_color_property)
-                        : Color(),
-          kBorderPrecedenceCell);
-      result = ChooseBorder(result, cell_after_adjoining_border);
-      if (!result.Exists())
-        return result;
-    }
+  if (cell_after) {
+    CollapsedBorderValue cell_after_adjoining_border = CollapsedBorderValue(
+        cell_after->BorderAdjoiningCellBefore(this),
+        cell_after->ResolveColor(start_color_property), kBorderPrecedenceCell);
+    result = ChooseBorder(result, cell_after_adjoining_border);
+    if (!result.Exists())
+      return result;
   }
 
   bool end_border_adjoins_table = HasEndBorderAdjoiningTable();
   if (end_border_adjoins_table) {
     // (3) Our row's end border.
     result = ChooseBorder(
-        result, CollapsedBorderValue(
-                    Row()->BorderAdjoiningEndCell(this),
-                    include_color ? Parent()->ResolveColor(end_color_property)
-                                  : Color(),
-                    kBorderPrecedenceRow));
+        result, CollapsedBorderValue(Row()->BorderAdjoiningEndCell(this),
+                                     Parent()->ResolveColor(end_color_property),
+                                     kBorderPrecedenceRow));
     if (!result.Exists())
       return result;
 
     // (4) Our row group's end border.
     result = ChooseBorder(
-        result, CollapsedBorderValue(
-                    Section()->BorderAdjoiningEndCell(this),
-                    include_color ? Section()->ResolveColor(end_color_property)
-                                  : Color(),
-                    kBorderPrecedenceRowGroup));
+        result,
+        CollapsedBorderValue(Section()->BorderAdjoiningEndCell(this),
+                             Section()->ResolveColor(end_color_property),
+                             kBorderPrecedenceRowGroup));
     if (!result.Exists())
       return result;
   }
@@ -816,9 +796,7 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedEndBorder(
         result,
         CollapsedBorderValue(
             col_and_col_group.colgroup->BorderAdjoiningCellEndBorder(this),
-            include_color
-                ? col_and_col_group.colgroup->ResolveColor(end_color_property)
-                : Color(),
+            col_and_col_group.colgroup->ResolveColor(end_color_property),
             kBorderPrecedenceColumnGroup));
     if (!result.Exists())
       return result;
@@ -831,9 +809,7 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedEndBorder(
     result = ChooseBorder(
         result, CollapsedBorderValue(
                     col_and_col_group.col->BorderAdjoiningCellEndBorder(this),
-                    include_color ? col_and_col_group.col->ResolveColor(
-                                        end_color_property)
-                                  : Color(),
+                    col_and_col_group.col->ResolveColor(end_color_property),
                     kBorderPrecedenceColumn));
     if (!result.Exists())
       return result;
@@ -851,9 +827,7 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedEndBorder(
           result,
           CollapsedBorderValue(
               col_and_col_group.colgroup->BorderAdjoiningCellStartBorder(this),
-              include_color ? col_and_col_group.colgroup->ResolveColor(
-                                  start_color_property)
-                            : Color(),
+              col_and_col_group.colgroup->ResolveColor(start_color_property),
               kBorderPrecedenceColumnGroup));
       if (!result.Exists())
         return result;
@@ -866,9 +840,7 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedEndBorder(
       result = ChooseBorder(
           result, CollapsedBorderValue(
                       col_and_col_group.col->BorderAdjoiningCellBefore(this),
-                      include_color ? col_and_col_group.col->ResolveColor(
-                                          start_color_property)
-                                    : Color(),
+                      col_and_col_group.col->ResolveColor(start_color_property),
                       kBorderPrecedenceColumn));
       if (!result.Exists())
         return result;
@@ -878,11 +850,9 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedEndBorder(
   if (end_border_adjoins_table) {
     // (7) The table's end border.
     result = ChooseBorder(
-        result,
-        CollapsedBorderValue(
-            table->TableEndBorderAdjoiningCell(this),
-            include_color ? table->ResolveColor(end_color_property) : Color(),
-            kBorderPrecedenceTable));
+        result, CollapsedBorderValue(table->TableEndBorderAdjoiningCell(this),
+                                     table->ResolveColor(end_color_property),
+                                     kBorderPrecedenceTable));
     if (!result.Exists())
       return result;
   }
@@ -890,37 +860,34 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedEndBorder(
   return result;
 }
 
-CollapsedBorderValue LayoutTableCell::ComputeCollapsedBeforeBorder(
-    IncludeBorderColorOrNot include_color) const {
+CollapsedBorderValue LayoutTableCell::ComputeCollapsedBeforeBorder() const {
   LayoutTable* table = this->Table();
+  LayoutTableCell* prev_cell = table->CellAbove(this);
+  // We can use the border shared with prevCell if it is valid.

[wkorman, 2017/04/27 20:11:15]

prevCell -> |prev_cell|

[Xianzhu, 2017/04/28 20:31:44]

Done.

+  if (prev_cell && prev_cell->collapsed_border_values_valid_ &&
+      prev_cell->AbsoluteColumnIndex() == AbsoluteColumnIndex()) {
+    return prev_cell->GetCollapsedBorderValues()
+               ? prev_cell->GetCollapsedBorderValues()->AfterBorder()
+               : CollapsedBorderValue();
+  }
 
   // For before border, we need to check, in order of precedence:
   // (1) Our before border.
-  int before_color_property = include_color
-                                  ? CSSProperty::ResolveDirectionAwareProperty(
-                                        CSSPropertyWebkitBorderBeforeColor,
-                                        StyleForCellFlow().Direction(),
-                                        StyleForCellFlow().GetWritingMode())
-                                  : 0;
-  int after_color_property = include_color
-                                 ? CSSProperty::ResolveDirectionAwareProperty(
-                                       CSSPropertyWebkitBorderAfterColor,
-                                       StyleForCellFlow().Direction(),
-                                       StyleForCellFlow().GetWritingMode())
-                                 : 0;
+  int before_color_property = CSSProperty::ResolveDirectionAwareProperty(
+      CSSPropertyWebkitBorderBeforeColor, StyleForCellFlow().Direction(),
+      StyleForCellFlow().GetWritingMode());
+  int after_color_property = CSSProperty::ResolveDirectionAwareProperty(
+      CSSPropertyWebkitBorderAfterColor, StyleForCellFlow().Direction(),
+      StyleForCellFlow().GetWritingMode());
   CollapsedBorderValue result = CollapsedBorderValue(
-      Style()->BorderBefore(),
-      include_color ? ResolveColor(before_color_property) : Color(),
+      Style()->BorderBefore(), ResolveColor(before_color_property),
       kBorderPrecedenceCell);
 
-  LayoutTableCell* prev_cell = table->CellAbove(this);
   if (prev_cell) {
     // (2) A before cell's after border.
     result = ChooseBorder(
         CollapsedBorderValue(prev_cell->Style()->BorderAfter(),
-                             include_color
-                                 ? prev_cell->ResolveColor(after_color_property)
-                                 : Color(),
+                             prev_cell->ResolveColor(after_color_property),
                              kBorderPrecedenceCell),
         result);
     if (!result.Exists())
@@ -929,11 +896,10 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedBeforeBorder(
 
   // (3) Our row's before border.
   result = ChooseBorder(
-      result, CollapsedBorderValue(
-                  Parent()->Style()->BorderBefore(),
-                  include_color ? Parent()->ResolveColor(before_color_property)
-                                : Color(),
-                  kBorderPrecedenceRow));
+      result,
+      CollapsedBorderValue(Parent()->Style()->BorderBefore(),
+                           Parent()->ResolveColor(before_color_property),
+                           kBorderPrecedenceRow));
   if (!result.Exists())
     return result;
 
@@ -947,11 +913,9 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedBeforeBorder(
 
     if (prev_row) {
       result = ChooseBorder(
-          CollapsedBorderValue(
-              prev_row->Style()->BorderAfter(),
-              include_color ? prev_row->ResolveColor(after_color_property)
-                            : Color(),
-              kBorderPrecedenceRow),
+          CollapsedBorderValue(prev_row->Style()->BorderAfter(),
+                               prev_row->ResolveColor(after_color_property),
+                               kBorderPrecedenceRow),
           result);
       if (!result.Exists())
         return result;
@@ -963,11 +927,10 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedBeforeBorder(
   if (!RowIndex()) {
     // (5) Our row group's before border.
     result = ChooseBorder(
-        result, CollapsedBorderValue(curr_section->Style()->BorderBefore(),
-                                     include_color ? curr_section->ResolveColor(
-                                                         before_color_property)
-                                                   : Color(),
-                                     kBorderPrecedenceRowGroup));
+        result,
+        CollapsedBorderValue(curr_section->Style()->BorderBefore(),
+                             curr_section->ResolveColor(before_color_property),
+                             kBorderPrecedenceRowGroup));
     if (!result.Exists())
       return result;
 
@@ -975,11 +938,9 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedBeforeBorder(
     curr_section = table->SectionAbove(curr_section, kSkipEmptySections);
     if (curr_section) {
       result = ChooseBorder(
-          CollapsedBorderValue(
-              curr_section->Style()->BorderAfter(),
-              include_color ? curr_section->ResolveColor(after_color_property)
-                            : Color(),
-              kBorderPrecedenceRowGroup),
+          CollapsedBorderValue(curr_section->Style()->BorderAfter(),
+                               curr_section->ResolveColor(after_color_property),
+                               kBorderPrecedenceRowGroup),
           result);
       if (!result.Exists())
         return result;
@@ -994,22 +955,19 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedBeforeBorder(
     if (col_elt) {
       result = ChooseBorder(
           result,
-          CollapsedBorderValue(
-              col_elt->Style()->BorderBefore(),
-              include_color ? col_elt->ResolveColor(before_color_property)
-                            : Color(),
-              kBorderPrecedenceColumn));
+          CollapsedBorderValue(col_elt->Style()->BorderBefore(),
+                               col_elt->ResolveColor(before_color_property),
+                               kBorderPrecedenceColumn));
       if (!result.Exists())
         return result;
       if (LayoutTableCol* enclosing_column_group =
               col_elt->EnclosingColumnGroup()) {
         result = ChooseBorder(
-            result, CollapsedBorderValue(
-                        enclosing_column_group->Style()->BorderBefore(),
-                        include_color ? enclosing_column_group->ResolveColor(
-                                            before_color_property)
-                                      : Color(),
-                        kBorderPrecedenceColumnGroup));
+            result,
+            CollapsedBorderValue(
+                enclosing_column_group->Style()->BorderBefore(),
+                enclosing_column_group->ResolveColor(before_color_property),
+                kBorderPrecedenceColumnGroup));
         if (!result.Exists())
           return result;
       }
@@ -1017,11 +975,9 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedBeforeBorder(
 
     // (9) The table's before border.
     result = ChooseBorder(
-        result, CollapsedBorderValue(
-                    table->Style()->BorderBefore(),
-                    include_color ? table->ResolveColor(before_color_property)
-                                  : Color(),
-                    kBorderPrecedenceTable));
+        result, CollapsedBorderValue(table->Style()->BorderBefore(),
+                                     table->ResolveColor(before_color_property),
+                                     kBorderPrecedenceTable));
     if (!result.Exists())
       return result;
   }
@@ -1029,61 +985,55 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedBeforeBorder(
   return result;
 }
 
-CollapsedBorderValue LayoutTableCell::ComputeCollapsedAfterBorder(
-    IncludeBorderColorOrNot include_color) const {
+CollapsedBorderValue LayoutTableCell::ComputeCollapsedAfterBorder() const {
   LayoutTable* table = this->Table();
+  LayoutTableCell* next_cell = table->CellBelow(this);
+  // We can use the border shared with nextCell if it is valid.
+  if (next_cell && next_cell->collapsed_border_values_valid_ &&
+      next_cell->AbsoluteColumnIndex() == AbsoluteColumnIndex()) {
+    return next_cell->GetCollapsedBorderValues()
+               ? next_cell->GetCollapsedBorderValues()->BeforeBorder()
+               : CollapsedBorderValue();
+  }
 
   // For after border, we need to check, in order of precedence:
   // (1) Our after border.
-  int before_color_property = include_color
-                                  ? CSSProperty::ResolveDirectionAwareProperty(
-                                        CSSPropertyWebkitBorderBeforeColor,
-                                        StyleForCellFlow().Direction(),
-                                        StyleForCellFlow().GetWritingMode())
-                                  : 0;
-  int after_color_property = include_color
-                                 ? CSSProperty::ResolveDirectionAwareProperty(
-                                       CSSPropertyWebkitBorderAfterColor,
-                                       StyleForCellFlow().Direction(),
-                                       StyleForCellFlow().GetWritingMode())
-                                 : 0;
+  int before_color_property = CSSProperty::ResolveDirectionAwareProperty(
+      CSSPropertyWebkitBorderBeforeColor, StyleForCellFlow().Direction(),
+      StyleForCellFlow().GetWritingMode());
+  int after_color_property = CSSProperty::ResolveDirectionAwareProperty(
+      CSSPropertyWebkitBorderAfterColor, StyleForCellFlow().Direction(),
+      StyleForCellFlow().GetWritingMode());
   CollapsedBorderValue result = CollapsedBorderValue(
-      Style()->BorderAfter(),
-      include_color ? ResolveColor(after_color_property) : Color(),
+      Style()->BorderAfter(), ResolveColor(after_color_property),
       kBorderPrecedenceCell);
 
-  LayoutTableCell* next_cell = table->CellBelow(this);
   if (next_cell) {
     // (2) An after cell's before border.
     result = ChooseBorder(
-        result, CollapsedBorderValue(next_cell->Style()->BorderBefore(),
-                                     include_color ? next_cell->ResolveColor(
-                                                         before_color_property)
-                                                   : Color(),
-                                     kBorderPrecedenceCell));
+        result,
+        CollapsedBorderValue(next_cell->Style()->BorderBefore(),
+                             next_cell->ResolveColor(before_color_property),
+                             kBorderPrecedenceCell));
     if (!result.Exists())
       return result;
   }
 
   // (3) Our row's after border. (FIXME: Deal with rowspan!)
   result = ChooseBorder(
-      result, CollapsedBorderValue(
-                  Parent()->Style()->BorderAfter(),
-                  include_color ? Parent()->ResolveColor(after_color_property)
-                                : Color(),
-                  kBorderPrecedenceRow));
+      result, CollapsedBorderValue(Parent()->Style()->BorderAfter(),
+                                   Parent()->ResolveColor(after_color_property),
+                                   kBorderPrecedenceRow));
   if (!result.Exists())
     return result;
 
   // (4) The next row's before border.
   if (next_cell) {
     result = ChooseBorder(
-        result,
-        CollapsedBorderValue(next_cell->Parent()->Style()->BorderBefore(),
-                             include_color ? next_cell->Parent()->ResolveColor(
-                                                 before_color_property)
-                                           : Color(),
-                             kBorderPrecedenceRow));
+        result, CollapsedBorderValue(
+                    next_cell->Parent()->Style()->BorderBefore(),
+                    next_cell->Parent()->ResolveColor(before_color_property),
+                    kBorderPrecedenceRow));
     if (!result.Exists())
       return result;
   }
@@ -1093,11 +1043,10 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedAfterBorder(
   if (RowIndex() + RowSpan() >= curr_section->NumRows()) {
     // (5) Our row group's after border.
     result = ChooseBorder(
-        result, CollapsedBorderValue(curr_section->Style()->BorderAfter(),
-                                     include_color ? curr_section->ResolveColor(
-                                                         after_color_property)
-                                                   : Color(),
-                                     kBorderPrecedenceRowGroup));
+        result,
+        CollapsedBorderValue(curr_section->Style()->BorderAfter(),
+                             curr_section->ResolveColor(after_color_property),
+                             kBorderPrecedenceRowGroup));
     if (!result.Exists())
       return result;
 
@@ -1105,12 +1054,10 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedAfterBorder(
     curr_section = table->SectionBelow(curr_section, kSkipEmptySections);
     if (curr_section) {
       result = ChooseBorder(
-          result,
-          CollapsedBorderValue(
-              curr_section->Style()->BorderBefore(),
-              include_color ? curr_section->ResolveColor(before_color_property)
-                            : Color(),
-              kBorderPrecedenceRowGroup));
+          result, CollapsedBorderValue(
+                      curr_section->Style()->BorderBefore(),
+                      curr_section->ResolveColor(before_color_property),
+                      kBorderPrecedenceRowGroup));
       if (!result.Exists())
         return result;
     }
@@ -1123,22 +1070,20 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedAfterBorder(
             .InnermostColOrColGroup();
     if (col_elt) {
       result = ChooseBorder(
-          result, CollapsedBorderValue(col_elt->Style()->BorderAfter(),
-                                       include_color ? col_elt->ResolveColor(
-                                                           after_color_property)
-                                                     : Color(),
-                                       kBorderPrecedenceColumn));
+          result,
+          CollapsedBorderValue(col_elt->Style()->BorderAfter(),
+                               col_elt->ResolveColor(after_color_property),
+                               kBorderPrecedenceColumn));
       if (!result.Exists())
         return result;
       if (LayoutTableCol* enclosing_column_group =
               col_elt->EnclosingColumnGroup()) {
         result = ChooseBorder(
-            result, CollapsedBorderValue(
-                        enclosing_column_group->Style()->BorderAfter(),
-                        include_color ? enclosing_column_group->ResolveColor(

[wkorman, 2017/04/27 20:11:15]

Perhaps worth mentioning this color change in CL description as well, as you did
for https://codereview.chromium.org/2838643003

[Xianzhu, 2017/04/28 20:31:45]

Done.

-                                            after_color_property)
-                                      : Color(),
-                        kBorderPrecedenceColumnGroup));
+            result,
+            CollapsedBorderValue(
+                enclosing_column_group->Style()->BorderAfter(),
+                enclosing_column_group->ResolveColor(after_color_property),
+                kBorderPrecedenceColumnGroup));
         if (!result.Exists())
           return result;
       }
@@ -1146,11 +1091,9 @@ CollapsedBorderValue LayoutTableCell::ComputeCollapsedAfterBorder(
 
     // (9) The table's after border.
     result = ChooseBorder(
-        result,
-        CollapsedBorderValue(
-            table->Style()->BorderAfter(),
-            include_color ? table->ResolveColor(after_color_property) : Color(),
-            kBorderPrecedenceTable));
+        result, CollapsedBorderValue(table->Style()->BorderAfter(),
+                                     table->ResolveColor(after_color_property),
+                                     kBorderPrecedenceTable));
     if (!result.Exists())
       return result;
   }
@@ -1250,8 +1193,12 @@ LayoutUnit LayoutTableCell::CollapsedBorderHalfBottom(bool outer) const {
 }
 
 LayoutUnit LayoutTableCell::CollapsedBorderHalfStart(bool outer) const {
-  CollapsedBorderValue border =
-      ComputeCollapsedStartBorder(kDoNotIncludeBorderColor);
+  RecalcCollapsedBorderValuesIfNeeded();
+  const auto* collapsed_border_values = this->GetCollapsedBorderValues();
+  if (!collapsed_border_values)
+    return LayoutUnit();
+
+  const auto& border = collapsed_border_values->StartBorder();
   if (border.Exists()) {
     return LayoutUnit(
         (border.Width() +
@@ -1262,8 +1209,12 @@ LayoutUnit LayoutTableCell::CollapsedBorderHalfStart(bool outer) const {
 }
 
 LayoutUnit LayoutTableCell::CollapsedBorderHalfEnd(bool outer) const {
-  CollapsedBorderValue border =
-      ComputeCollapsedEndBorder(kDoNotIncludeBorderColor);
+  RecalcCollapsedBorderValuesIfNeeded();
+  const auto* collapsed_border_values = this->GetCollapsedBorderValues();
+  if (!collapsed_border_values)
+    return LayoutUnit();
+
+  const auto& border = collapsed_border_values->EndBorder();
   if (border.Exists()) {
     return LayoutUnit(
         (border.Width() +
@@ -1274,8 +1225,12 @@ LayoutUnit LayoutTableCell::CollapsedBorderHalfEnd(bool outer) const {
 }
 
 LayoutUnit LayoutTableCell::CollapsedBorderHalfBefore(bool outer) const {
-  CollapsedBorderValue border =
-      ComputeCollapsedBeforeBorder(kDoNotIncludeBorderColor);
+  RecalcCollapsedBorderValuesIfNeeded();
+  const auto* collapsed_border_values = this->GetCollapsedBorderValues();
+  if (!collapsed_border_values)
+    return LayoutUnit();
+
+  const auto& border = collapsed_border_values->BeforeBorder();
   if (border.Exists()) {
     return LayoutUnit(
         (border.Width() +
@@ -1286,8 +1241,12 @@ LayoutUnit LayoutTableCell::CollapsedBorderHalfBefore(bool outer) const {
 }
 
 LayoutUnit LayoutTableCell::CollapsedBorderHalfAfter(bool outer) const {
-  CollapsedBorderValue border =
-      ComputeCollapsedAfterBorder(kDoNotIncludeBorderColor);
+  RecalcCollapsedBorderValuesIfNeeded();
+  const auto* collapsed_border_values = this->GetCollapsedBorderValues();
+  if (!collapsed_border_values)
+    return LayoutUnit();
+
+  const auto& border = collapsed_border_values->AfterBorder();
   if (border.Exists()) {
     return LayoutUnit(
         (border.Width() +
@@ -1302,6 +1261,59 @@ void LayoutTableCell::Paint(const PaintInfo& paint_info,
   TableCellPainter(*this).Paint(paint_info, paint_offset);
 }
 
+void LayoutTableCell::RecalcCollapsedBorderValuesIfNeeded() const {

[wkorman, 2017/04/27 20:11:15]

Same as previous -- believe prefer without ...IfNeeded suffix.

[Xianzhu, 2017/04/28 20:31:44]

Renamed to UpdateCollapsedBorderValues(). Simply removing IfNeeded may confuse
callers because "Recalc" means always "re-calculate".

+  Table()->InvalidateCollapsedBordersForAllCellsIfNeeded();
+  if (collapsed_border_values_valid_)
+    return;
+
+  collapsed_border_values_valid_ = true;
+
+  if (!Table()->CollapseBorders()) {
+    if (collapsed_border_values_) {
+      collapsed_borders_visually_changed_ = true;
+      collapsed_border_values_ = nullptr;
+    }
+    return;
+  }
+
+  CollapsedBorderValues new_values(
+      *this, ComputeCollapsedStartBorder(), ComputeCollapsedEndBorder(),
+      ComputeCollapsedBeforeBorder(), ComputeCollapsedAfterBorder());
+
+  // We need to save collapsed border if has a non-zero width even if it's
+  // invisible because the width affects table layout.
+  if (!new_values.StartBorder().Width() && !new_values.EndBorder().Width() &&
+      !new_values.BeforeBorder().Width() && !new_values.AfterBorder().Width()) {
+    if (collapsed_border_values_) {
+      collapsed_borders_visually_changed_ = true;
+      collapsed_border_values_ = nullptr;
+    }
+    return;
+  }
+
+  if (!collapsed_border_values_) {
+    collapsed_borders_visually_changed_ = true;
+    collapsed_border_values_ = WTF::WrapUnique(new CollapsedBorderValues(
+        *this, new_values.StartBorder(), new_values.EndBorder(),
+        new_values.BeforeBorder(), new_values.AfterBorder()));
+    return;
+  }
+
+  // We check VisuallyEquals so that the table cell is invalidated only if a
+  // changed collapsed border is visible in the first place.
+  if (!collapsed_border_values_->StartBorder().VisuallyEquals(
+          new_values.StartBorder()) ||
+      !collapsed_border_values_->EndBorder().VisuallyEquals(
+          new_values.EndBorder()) ||
+      !collapsed_border_values_->BeforeBorder().VisuallyEquals(
+          new_values.BeforeBorder()) ||
+      !collapsed_border_values_->AfterBorder().VisuallyEquals(
+          new_values.AfterBorder())) {
+    collapsed_border_values_->SetCollapsedBorderValues(new_values);
+    collapsed_borders_visually_changed_ = true;
+  }
+}
+
 static void AddBorderStyle(LayoutTable::CollapsedBorderValues& border_values,
                            CollapsedBorderValue border_value) {
   if (!border_value.IsVisible())
@@ -1316,51 +1328,26 @@ static void AddBorderStyle(LayoutTable::CollapsedBorderValues& border_values,
 
 void LayoutTableCell::CollectCollapsedBorderValues(
     LayoutTable::CollapsedBorderValues& border_values) {
-  CollapsedBorderValues new_values(
-      *this, ComputeCollapsedStartBorder(), ComputeCollapsedEndBorder(),
-      ComputeCollapsedBeforeBorder(), ComputeCollapsedAfterBorder());
-
-  bool changed = false;
-  if (!new_values.StartBorder().IsVisible() &&
-      !new_values.EndBorder().IsVisible() &&
-      !new_values.BeforeBorder().IsVisible() &&
-      !new_values.AfterBorder().IsVisible()) {
-    changed = !!collapsed_border_values_;
-    collapsed_border_values_ = nullptr;
-  } else if (!collapsed_border_values_) {
-    changed = true;
-    collapsed_border_values_ = WTF::WrapUnique(new CollapsedBorderValues(
-        *this, new_values.StartBorder(), new_values.EndBorder(),
-        new_values.BeforeBorder(), new_values.AfterBorder()));
-  } else {
-    // We check visuallyEquals so that the table cell is invalidated only if a
-    // changed collapsed border is visible in the first place.
-    changed = !collapsed_border_values_->StartBorder().VisuallyEquals(
-                  new_values.StartBorder()) ||
-              !collapsed_border_values_->EndBorder().VisuallyEquals(
-                  new_values.EndBorder()) ||
-              !collapsed_border_values_->BeforeBorder().VisuallyEquals(
-                  new_values.BeforeBorder()) ||
-              !collapsed_border_values_->AfterBorder().VisuallyEquals(
-                  new_values.AfterBorder());
-    if (changed)
-      collapsed_border_values_->SetCollapsedBorderValues(new_values);
-  }
+  RecalcCollapsedBorderValuesIfNeeded();
 
   // If collapsed borders changed, invalidate the cell's display item client on
   // the table's backing.
   // TODO(crbug.com/451090#c5): Need a way to invalidate/repaint the borders
   // only.
-  if (changed) {
+  if (collapsed_borders_visually_changed_) {
     ObjectPaintInvalidator(*Table())
         .SlowSetPaintingLayerNeedsRepaintAndInvalidateDisplayItemClient(
             *this, kPaintInvalidationStyleChange);
+    collapsed_borders_visually_changed_ = false;
   }
 
-  AddBorderStyle(border_values, new_values.StartBorder());
-  AddBorderStyle(border_values, new_values.EndBorder());
-  AddBorderStyle(border_values, new_values.BeforeBorder());
-  AddBorderStyle(border_values, new_values.AfterBorder());
+  if (!collapsed_border_values_)
+    return;
+
+  AddBorderStyle(border_values, collapsed_border_values_->StartBorder());
+  AddBorderStyle(border_values, collapsed_border_values_->EndBorder());
+  AddBorderStyle(border_values, collapsed_border_values_->BeforeBorder());
+  AddBorderStyle(border_values, collapsed_border_values_->AfterBorder());
 }
 
 void LayoutTableCell::SortCollapsedBorderValues(