Move border-*-width out of BorderValue and store on SurroundData in ComputedStyle instead

third_party/WebKit/Source/core/style/ComputedStyle.h

Index: third_party/WebKit/Source/core/style/ComputedStyle.h
diff --git a/third_party/WebKit/Source/core/style/ComputedStyle.h b/third_party/WebKit/Source/core/style/ComputedStyle.h
index c6033b5c88d6be9c45320c2ed0013d840fb9b8b5..c88679624a44cc953f6e07a86898a1efac0cb0fb 100644
--- a/third_party/WebKit/Source/core/style/ComputedStyle.h
+++ b/third_party/WebKit/Source/core/style/ComputedStyle.h
@@ -294,6 +294,12 @@ class CORE_EXPORT ComputedStyle : public ComputedStyleBase,
     return cached_pseudo_styles_.get();
   }
 
+  bool BorderWidthEquals(float border_width_first,
+                         float border_width_second) const {
+    return WidthToFixedPoint(border_width_first) ==
+           WidthToFixedPoint(border_width_second);
+  }
+
   /**
      * ComputedStyle properties
      *
@@ -502,34 +508,50 @@ class CORE_EXPORT ComputedStyle : public ComputedStyleBase,
 
   // border-top-width
   float BorderTopWidth() const {
-    return surround_data_->border_.BorderTopWidth();
+    if (surround_data_->border_.top_.Style() == kBorderStyleNone ||
+        surround_data_->border_.top_.Style() == kBorderStyleHidden)
+      return 0;
+    return static_cast<float>(BorderTopWidthInternal()) /
+           kBorderWidthDenominator;
   }
   void SetBorderTopWidth(float v) {
-    SET_BORDER_WIDTH(surround_data_, border_.top_, v);
+    surround_data_.Access()->border_top_width_ = WidthToFixedPoint(v);
   }
 
   // border-bottom-width
   float BorderBottomWidth() const {
-    return surround_data_->border_.BorderBottomWidth();
+    if (surround_data_->border_.bottom_.Style() == kBorderStyleNone ||
+        surround_data_->border_.bottom_.Style() == kBorderStyleHidden)
+      return 0;
+    return static_cast<float>(BorderBottomWidthInternal()) /
+           kBorderWidthDenominator;
   }
   void SetBorderBottomWidth(float v) {
-    SET_BORDER_WIDTH(surround_data_, border_.bottom_, v);
+    surround_data_.Access()->border_bottom_width_ = WidthToFixedPoint(v);
   }
 
   // border-left-width
   float BorderLeftWidth() const {
-    return surround_data_->border_.BorderLeftWidth();
+    if (surround_data_->border_.left_.Style() == kBorderStyleNone ||
+        surround_data_->border_.left_.Style() == kBorderStyleHidden)
+      return 0;
+    return static_cast<float>(BorderLeftWidthInternal()) /
+           kBorderWidthDenominator;
   }
   void SetBorderLeftWidth(float v) {
-    SET_BORDER_WIDTH(surround_data_, border_.left_, v);
+    surround_data_.Access()->border_left_width_ = WidthToFixedPoint(v);
   }
 
   // border-right-width
   float BorderRightWidth() const {
-    return surround_data_->border_.BorderRightWidth();
+    if (surround_data_->border_.right_.Style() == kBorderStyleNone ||
+        surround_data_->border_.right_.Style() == kBorderStyleHidden)
+      return 0;
+    return static_cast<float>(BorderRightWidthInternal()) /

[shend, 2017/05/07 23:08:31]

The fixed point stuff feels like it should be a class. Maybe as future work, we
could make a FixedPoint class that handles this stuff in a generic way?

[nainar, 2017/05/08 00:59:57]

Yes. Added a todo here on me to take a look at this next.

+           kBorderWidthDenominator;
   }
   void SetBorderRightWidth(float v) {
-    SET_BORDER_WIDTH(surround_data_, border_.right_, v);
+    surround_data_.Access()->border_right_width_ = WidthToFixedPoint(v);
   }
 
   // Border style properties.
@@ -2853,14 +2875,30 @@ class CORE_EXPORT ComputedStyle : public ComputedStyleBase,
   void SetBorderImageSlicesFill(bool);
   const BorderData& Border() const { return surround_data_->border_; }
   const BorderValue& BorderLeft() const {
-    return surround_data_->border_.Left();
+    BorderValue* border_value =

[shend, 2017/05/07 23:08:30]

Have we measured the perf impact of this dynamic allocation?
Also, I'm not familiar with Blink GC magic, but isn't this a memory leak?
Could we just do:

BorderValue BorderLeft() const {
  return BorderValue(surround_data_->border_.Left(), BorderLeftWidth());
}

[nainar, 2017/05/08 00:59:57]

Done.

+        new BorderValue(surround_data_->border_.Left(), BorderLeftWidth());
+    return *border_value;
   }
   const BorderValue& BorderRight() const {
-    return surround_data_->border_.Right();
+    BorderValue* border_value =
+        new BorderValue(surround_data_->border_.Right(), BorderRightWidth());
+    return *border_value;
+  }
+  const BorderValue& BorderTop() const {
+    BorderValue* border_value =
+        new BorderValue(surround_data_->border_.Top(), BorderTopWidth());
+    return *border_value;
   }
-  const BorderValue& BorderTop() const { return surround_data_->border_.Top(); }
   const BorderValue& BorderBottom() const {
-    return surround_data_->border_.Bottom();
+    BorderValue* border_value =
+        new BorderValue(surround_data_->border_.Bottom(), BorderBottomWidth());
+    return *border_value;
+  }
+  bool BorderSizeEquals(const ComputedStyle& o) const {
+    return BorderWidthEquals(BorderLeftWidth(), o.BorderLeftWidth()) &&
+           BorderWidthEquals(BorderTopWidth(), o.BorderTopWidth()) &&
+           BorderWidthEquals(BorderRightWidth(), o.BorderRightWidth()) &&
+           BorderWidthEquals(BorderBottomWidth(), o.BorderBottomWidth());
   }
   const BorderValue& BorderBefore() const;
   const BorderValue& BorderAfter() const;
@@ -2874,7 +2912,10 @@ class CORE_EXPORT ComputedStyle : public ComputedStyleBase,
   float BorderUnderWidth() const;
 
   bool HasBorderFill() const { return Border().HasBorderFill(); }
-  bool HasBorder() const { return Border().HasBorder(); }
+  bool HasBorder() const {
+    return Border().HasBorder() || BorderLeftWidth() || BorderRightWidth() ||
+           BorderTopWidth() || BorderBottomWidth();
+  }
   bool HasBorderDecoration() const { return HasBorder() || HasBorderFill(); }
   bool HasBorderRadius() const {
     if (!BorderTopLeftRadius().Width().IsZero())
@@ -2910,16 +2951,16 @@ class CORE_EXPORT ComputedStyle : public ComputedStyleBase,
     ResetBorderBottomRightRadius();
   }
   void ResetBorderTop() {
-    SET_VAR(surround_data_, border_.top_, BorderValue());
+    SET_VAR(surround_data_, border_.top_, BorderColorAndStyle());

[nainar, 2017/05/05 07:16:50]

hmm on second thought this should probably also reset BorderTopWidth.

Best to do it in this function or in all callsites?

[shend, 2017/05/07 23:08:31]

Hmm, good question. I would do it here, since BorderLeft() returns a BorderValue
(color + style + width), I would expect ResetBorderLeft() to reset all three
things.

[nainar, 2017/05/08 00:59:57]

Yup. Agreed. Done.

   }
   void ResetBorderRight() {
-    SET_VAR(surround_data_, border_.right_, BorderValue());
+    SET_VAR(surround_data_, border_.right_, BorderColorAndStyle());
   }
   void ResetBorderBottom() {
-    SET_VAR(surround_data_, border_.bottom_, BorderValue());
+    SET_VAR(surround_data_, border_.bottom_, BorderColorAndStyle());
   }
   void ResetBorderLeft() {
-    SET_VAR(surround_data_, border_.left_, BorderValue());
+    SET_VAR(surround_data_, border_.left_, BorderColorAndStyle());
   }
   void ResetBorderImage() {
     SET_VAR(surround_data_, border_.image_, NinePieceImage());