Optimize collapsed border calculation (step 1)

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

Index: third_party/WebKit/Source/core/layout/LayoutTableCell.h
diff --git a/third_party/WebKit/Source/core/layout/LayoutTableCell.h b/third_party/WebKit/Source/core/layout/LayoutTableCell.h
index d9e7a613e30d3f9331e6a649deca4c6f8bd775b1..1dbd0dee212ce66d5c31ce3e4920cf239ab9d799 100644
--- a/third_party/WebKit/Source/core/layout/LayoutTableCell.h
+++ b/third_party/WebKit/Source/core/layout/LayoutTableCell.h
@@ -35,10 +35,8 @@
 
 namespace blink {
 
-static const unsigned kUnsetColumnIndex = 0x1FFFFFFF;
-static const unsigned kMaxColumnIndex = 0x1FFFFFFE;  // 536,870,910
-
-enum IncludeBorderColorOrNot { kDoNotIncludeBorderColor, kIncludeBorderColor };
+static const unsigned kUnsetColumnIndex = 0x07FFFFFF;

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

How are these numbers here and below determined and why do they change as part
of this patch? Worth adding comment for future readers as well.

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

Changed to self-explanation code:
#define BITS_OF_ABSOLUTE_COLUMN_INDEX 27
static const unsigned kUnsetColumnIndex =
    (1u << BITS_OF_ABSOLUTE_COLUMN_INDEX) - 1;
static const unsigned kMaxColumnIndex = kUnsetColumnIndex - 1;

and in LayoutTableCell:
  unsigned absolute_column_index_ : BITS_OF_ABSOLUTE_COLUMN_INDEX;

  // When adding or removing bits here, we should also adjust
  // BITS_OF_ABSOLUTE_COLUMN_INDEX to use remaining bits of a 32-bit word.

add added a test LayoutTableCellTest.AbsoluteColumnIndex.

+static const unsigned kMaxColumnIndex = 0x07FFFFFE;  // 134,217,726
 
 class SubtreeLayoutScope;
 
@@ -327,8 +325,12 @@ class CORE_EXPORT LayoutTableCell final : public LayoutBlockFlow {
 
   bool UsesCompositedCellDisplayItemClients() const;
   const CollapsedBorderValues* GetCollapsedBorderValues() const {
+    DCHECK(collapsed_border_values_valid_);
     return collapsed_border_values_.get();
   }
+  void InvalidateCollapsedBorderValues() {
+    collapsed_border_values_valid_ = false;

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

When are they valid/invalid? Worth documenting in a comment somewhere, perhaps
on the member data? I wondered same re: visually_changed member data.

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

Done.

+  }
 
   LayoutRect DebugRect() const override;
 
@@ -404,16 +406,13 @@ class CORE_EXPORT LayoutTableCell final : public LayoutBlockFlow {
   // See also https://code.google.com/p/chromium/issues/detail?id=128227 for
   // some history.
   //
-  // Those functions are called when the cache (m_collapsedBorders) is
-  // invalidated on LayoutTable.
-  CollapsedBorderValue ComputeCollapsedStartBorder(
-      IncludeBorderColorOrNot = kIncludeBorderColor) const;
-  CollapsedBorderValue ComputeCollapsedEndBorder(
-      IncludeBorderColorOrNot = kIncludeBorderColor) const;
-  CollapsedBorderValue ComputeCollapsedBeforeBorder(
-      IncludeBorderColorOrNot = kIncludeBorderColor) const;
-  CollapsedBorderValue ComputeCollapsedAfterBorder(
-      IncludeBorderColorOrNot = kIncludeBorderColor) const;
+  // Those functions are called during recalcCollapsedBorderValuesIfNeeded().
+  CollapsedBorderValue ComputeCollapsedStartBorder() const;
+  CollapsedBorderValue ComputeCollapsedEndBorder() const;
+  CollapsedBorderValue ComputeCollapsedBeforeBorder() const;
+  CollapsedBorderValue ComputeCollapsedAfterBorder() const;
+
+  void RecalcCollapsedBorderValuesIfNeeded() const;
 
   Length LogicalWidthFromColumns(LayoutTableCol* first_col_for_this_cell,
                                  Length width_from_style) const;
@@ -428,11 +427,15 @@ class CORE_EXPORT LayoutTableCell final : public LayoutBlockFlow {
 
   // Note MSVC will only pack members if they have identical types, hence we use
   // unsigned instead of bool here.
-  unsigned absolute_column_index_ : 29;
+  unsigned absolute_column_index_ : 27;
   unsigned cell_width_changed_ : 1;
   unsigned has_col_span_ : 1;
   unsigned has_row_span_ : 1;
 
+  mutable unsigned collapsed_border_values_valid_ : 1;
+  mutable unsigned collapsed_borders_visually_changed_ : 1;
+  mutable std::unique_ptr<CollapsedBorderValues> collapsed_border_values_;
+
   // The intrinsic padding.
   // See class comment for what they are.
   //
@@ -440,8 +443,6 @@ class CORE_EXPORT LayoutTableCell final : public LayoutBlockFlow {
   // because we don't do fractional arithmetic on tables.
   int intrinsic_padding_before_;
   int intrinsic_padding_after_;
-
-  std::unique_ptr<CollapsedBorderValues> collapsed_border_values_;
 };
 
 DEFINE_LAYOUT_OBJECT_TYPE_CASTS(LayoutTableCell, IsTableCell());