Use generated getters/setters in ComputedStyle instead of directly accessing fields

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

Index: third_party/WebKit/Source/core/style/ComputedStyle.cpp
diff --git a/third_party/WebKit/Source/core/style/ComputedStyle.cpp b/third_party/WebKit/Source/core/style/ComputedStyle.cpp
index e62168104424eef3b4739ca41bcbf003abce0b94..ea0c024633f83b75d4cae5c438fc9a40b1a90aa9 100644
--- a/third_party/WebKit/Source/core/style/ComputedStyle.cpp
+++ b/third_party/WebKit/Source/core/style/ComputedStyle.cpp
@@ -345,9 +345,9 @@ void ComputedStyle::CopyNonInheritedFromCached(const ComputedStyle& other) {
 
   // These are not generated in ComputedStyleBase
   SetOriginalDisplay(other.OriginalDisplay());
-  SetVerticalAlign(other.VerticalAlign());
+  SetVerticalAlignInternal(other.VerticalAlignInternal());

[shend, 2017/04/19 06:17:30]

This is an interesting case. Both the previous and new code work.
I think we need to make a decision about which getter/setter to call when there
is a public getter that's the same as the internal getter.

IMO, we should only use internal getters/setters as a last resort, but I don't
mind as long as there is a consistent rule.

[nainar, 2017/04/19 06:31:14]

Replied in the next comment.Also done. 

   SetHasViewportUnits(other.HasViewportUnits());
-  has_rem_units_ = other.HasRemUnits();
+  SetHasRemUnitsInternal(other.HasRemUnits());
 
   // Correctly set during selector matching:
   // m_styleType
@@ -480,8 +480,9 @@ bool ComputedStyle::NonInheritedEqual(const ComputedStyle& other) const {
   return ComputedStyleBase::NonInheritedEqual(other) &&
          OriginalDisplay() ==
              other.OriginalDisplay() &&  // Not generated in ComputedStyleBase
-         VerticalAlign() ==
-             other.VerticalAlign() &&  // Not generated in ComputedStyleBase
+         VerticalAlignInternal() ==
+             other.VerticalAlignInternal() &&  // Not generated in
+                                               // ComputedStyleBase
          box_ == other.box_ &&
          visual_ == other.visual_ && background_ == other.background_ &&
          surround_ == other.surround_ &&
@@ -850,14 +851,14 @@ bool ComputedStyle::DiffNeedsFullLayout(const ComputedStyle& other) const {
         box_->MaxHeight() != other.box_->MaxHeight())
       return true;
 
-    if (box_->VerticalAlign() != other.box_->VerticalAlign())
+    if (box_->vertical_align_ != other.box_->vertical_align_)
       return true;
 
     if (box_->BoxSizing() != other.box_->BoxSizing())
       return true;
   }
 
-  if (VerticalAlign() != other.VerticalAlign() ||
+  if (VerticalAlignInternal() != other.VerticalAlignInternal() ||
       GetPosition() != other.GetPosition())
     return true;
 
@@ -1061,7 +1062,7 @@ void ComputedStyle::UpdatePropertySpecificDifferences(
     if (style_inherited_data_->color != other.style_inherited_data_->color ||
         style_inherited_data_->visited_link_color !=
             other.style_inherited_data_->visited_link_color ||
-        has_simple_underline_ != other.has_simple_underline_ ||
+        HasSimpleUnderlineInternal() != other.HasSimpleUnderlineInternal() ||
         visual_->text_decoration != other.visual_->text_decoration) {
       diff.SetTextDecorationOrColorChanged();
     } else if (rare_non_inherited_data_.Get() !=
@@ -1712,7 +1713,7 @@ FontStretch ComputedStyle::GetFontStretch() const {
 }
 
 TextDecoration ComputedStyle::TextDecorationsInEffect() const {
-  if (has_simple_underline_)
+  if (HasSimpleUnderlineInternal())
     return kTextDecorationUnderline;
   if (!rare_inherited_data_->applied_text_decorations)
     return kTextDecorationNone;
@@ -1729,7 +1730,7 @@ TextDecoration ComputedStyle::TextDecorationsInEffect() const {
 
 const Vector<AppliedTextDecoration>& ComputedStyle::AppliedTextDecorations()
     const {
-  if (has_simple_underline_) {
+  if (HasSimpleUnderlineInternal()) {
     DEFINE_STATIC_LOCAL(
         Vector<AppliedTextDecoration>, underline,
         (1, AppliedTextDecoration(
@@ -2016,18 +2017,19 @@ void ComputedStyle::OverrideTextDecorationColors(Color override_color) {
 void ComputedStyle::ApplyTextDecorations(
     const Color& parent_text_decoration_color,
     bool override_existing_colors) {
-  if (GetTextDecoration() == kTextDecorationNone && !has_simple_underline_ &&
+  if (GetTextDecoration() == kTextDecorationNone &&
+      !HasSimpleUnderlineInternal() &&
       !rare_inherited_data_->applied_text_decorations)
     return;
 
   // If there are any color changes or decorations set by this element, stop
-  // using m_hasSimpleUnderline.
+  // using m_hasSimpleUnderlineInternal.
   Color current_text_decoration_color =
       VisitedDependentColor(CSSPropertyTextDecorationColor);
-  if (has_simple_underline_ &&
+  if (HasSimpleUnderlineInternal() &&
       (GetTextDecoration() != kTextDecorationNone ||
        current_text_decoration_color != parent_text_decoration_color)) {
-    has_simple_underline_ = false;
+    SetHasSimpleUnderlineInternal(false);
     AddAppliedTextDecoration(AppliedTextDecoration(
         kTextDecorationUnderline, kTextDecorationStyleSolid,
         parent_text_decoration_color));
@@ -2037,7 +2039,7 @@ void ComputedStyle::ApplyTextDecorations(
     OverrideTextDecorationColors(current_text_decoration_color);
   if (GetTextDecoration() == kTextDecorationNone)
     return;
-  DCHECK(!has_simple_underline_);
+  DCHECK(!HasSimpleUnderlineInternal());
   // To save memory, we don't use AppliedTextDecoration objects in the common
   // case of a single simple underline of currentColor.
   TextDecoration decoration_lines = GetTextDecoration();
@@ -2046,7 +2048,7 @@ void ComputedStyle::ApplyTextDecorations(
                              decoration_style == kTextDecorationStyleSolid &&
                              TextDecorationColor().IsCurrentColor();
   if (is_simple_underline && !rare_inherited_data_->applied_text_decorations) {
-    has_simple_underline_ = true;
+    SetHasSimpleUnderlineInternal(true);
     return;
   }
 
@@ -2055,7 +2057,7 @@ void ComputedStyle::ApplyTextDecorations(
 }
 
 void ComputedStyle::ClearAppliedTextDecorations() {
-  has_simple_underline_ = false;
+  SetHasSimpleUnderlineInternal(false);
 
   if (rare_inherited_data_->applied_text_decorations)
     rare_inherited_data_.Access()->applied_text_decorations = nullptr;
@@ -2063,7 +2065,7 @@ void ComputedStyle::ClearAppliedTextDecorations() {
 
 void ComputedStyle::RestoreParentTextDecorations(
     const ComputedStyle& parent_style) {
-  has_simple_underline_ = parent_style.has_simple_underline_;
+  SetHasSimpleUnderlineInternal(parent_style.HasSimpleUnderlineInternal());
   if (rare_inherited_data_->applied_text_decorations !=
       parent_style.rare_inherited_data_->applied_text_decorations)
     rare_inherited_data_.Access()->applied_text_decorations =
@@ -2463,7 +2465,7 @@ void ComputedStyle::GetBorderEdgeInfo(BorderEdge edges[],
 }
 
 void ComputedStyle::CopyChildDependentFlagsFrom(const ComputedStyle& other) {
-  SetEmptyState(other.EmptyState());
+  SetEmptyStateInternal(other.EmptyStateInternal());

[shend, 2017/04/19 06:17:30]

Here is the problem with using internal getters. I don't think this code does
the same thing. Previously, we would have called SetUnique from the
SetEmptyState, but now we don't. So internal setters don't maintain class
invariants, which could subtly break code.

Surprisingly, tests don't break...

[nainar, 2017/04/19 06:31:14]

Yup, I see what you mean. Using public getters is best when they are available.
Made the change

Also yeah at lack of test coverage.

   if (other.HasExplicitlyInheritedProperties())
     SetHasExplicitlyInheritedProperties();
 }