Fix size keyword comparison in CSSRadialGradientValue::Equals

Said function would ignore any size keyword or explicit size
specification if there was a shape keyword.
Adjust the comparisons so that first any explicit size checked, followed
by checking the size keyword and the shape value.

Also rewrite the if-ladder for the center point (|first_x_| and
|first_y_|) to just use two DataEquivalent.

Bug: 775201
Change-Id: I44686b54d3e02e1fd4682c4288eec3aca0e3e441
Reviewed-on: https://chromium-review.googlesource.com/723460Reviewed-by: meade_UTC10 <meade@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#509718}

third_party/WebKit/Source/core/BUILD.gn

@@ -1342,6 +1342,7 @@ jumbo_source_set("unit_tests") {
     "css/AffectedByPseudoTest.cpp",
     "css/CSSCalculationValueTest.cpp",
     "css/CSSFontFaceSourceTest.cpp",
+    "css/CSSGradientValueTest.cpp",
     "css/CSSPageRuleTest.cpp",
     "css/CSSSelectorTest.cpp",
     "css/CSSSelectorWatchTest.cpp",

third_party/WebKit/Source/core/css/CSSGradientValue.cpp

@@ -1332,6 +1332,18 @@ RefPtr<Gradient> CSSRadialGradientValue::CreateGradient(
   return gradient;
 }
 
+namespace {
+
+bool EqualIdentifiersWithDefault(const CSSIdentifierValue* id_a,
+                                 const CSSIdentifierValue* id_b,
+                                 CSSValueID default_id) {
+  CSSValueID value_a = id_a ? id_a->GetValueID() : default_id;
+  CSSValueID value_b = id_b ? id_b->GetValueID() : default_id;
+  return value_a == value_b;
+}
+
+}  // namespace
+
 bool CSSRadialGradientValue::Equals(const CSSRadialGradientValue& other) const {
   if (gradient_type_ == kCSSDeprecatedRadialGradient)
     return other.gradient_type_ == gradient_type_ &&
@@ -1346,42 +1358,29 @@ bool CSSRadialGradientValue::Equals(const CSSRadialGradientValue& other) const {
   if (repeating_ != other.repeating_)
     return false;
 
-  bool equal_xand_y = false;
-  if (first_x_ && first_y_) {
-    equal_xand_y = DataEquivalent(first_x_, other.first_x_) &&
-                   DataEquivalent(first_y_, other.first_y_);
-  } else if (first_x_) {
-    equal_xand_y = DataEquivalent(first_x_, other.first_x_) && !other.first_y_;
-  } else if (first_y_) {
-    equal_xand_y = DataEquivalent(first_y_, other.first_y_) && !other.first_x_;
-  } else {
-    equal_xand_y = !other.first_x_ && !other.first_y_;
-  }
-
-  if (!equal_xand_y)
+  if (!DataEquivalent(first_x_, other.first_x_) ||
+      !DataEquivalent(first_y_, other.first_y_))
     return false;
 
-  bool equal_shape = true;
-  bool equal_sizing_behavior = true;
-  bool equal_horizontal_and_vertical_size = true;
-
-  if (shape_) {
-    equal_shape = DataEquivalent(shape_, other.shape_);
-  } else if (sizing_behavior_) {
-    equal_sizing_behavior =
-        DataEquivalent(sizing_behavior_, other.sizing_behavior_);
-  } else if (end_horizontal_size_ && end_vertical_size_) {
-    equal_horizontal_and_vertical_size =
-        DataEquivalent(end_horizontal_size_, other.end_horizontal_size_) &&
-        DataEquivalent(end_vertical_size_, other.end_vertical_size_);
+  // There's either a size keyword or an explicit size specification.
+  if (end_horizontal_size_) {
+    // Explicit size specification. One <length> or two <length-percentage>.
+    if (!DataEquivalent(end_horizontal_size_, other.end_horizontal_size_))
+      return false;
+    if (!DataEquivalent(end_vertical_size_, other.end_vertical_size_))
+      return false;
   } else {
-    equal_shape = !other.shape_;
-    equal_sizing_behavior = !other.sizing_behavior_;
-    equal_horizontal_and_vertical_size =
-        !other.end_horizontal_size_ && !other.end_vertical_size_;
+    if (other.end_horizontal_size_)
+      return false;
+    // There's a size keyword.
+    if (!EqualIdentifiersWithDefault(sizing_behavior_, other.sizing_behavior_,
+                                     CSSValueFarthestCorner))
+      return false;
+    // Here the shape is 'ellipse' unless explicitly set to 'circle'.
+    if (!EqualIdentifiersWithDefault(shape_, other.shape_, CSSValueEllipse))
+      return false;
   }
-  return equal_shape && equal_sizing_behavior &&
-         equal_horizontal_and_vertical_size && stops_ == other.stops_;
+  return stops_ == other.stops_;
 }
 
 DEFINE_TRACE_AFTER_DISPATCH(CSSRadialGradientValue) {

third_party/WebKit/Source/core/css/CSSGradientValueTest.cpp

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/css/CSSGradientValue.h"
+
+#include "core/css/parser/CSSParser.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace blink {
+
+namespace {
+
+bool CompareGradients(const char* gradient1, const char* gradient2) {
+  const CSSValue* value1 = CSSParser::ParseSingleValue(
+      CSSPropertyBackgroundImage, gradient1, StrictCSSParserContext());
+  const CSSValue* value2 = CSSParser::ParseSingleValue(
+      CSSPropertyBackgroundImage, gradient2, StrictCSSParserContext());
+  return *value1 == *value2;
+}
+
+TEST(CSSGradientValueTest, RadialGradient_Equals) {
+  // Trivially identical.
+  EXPECT_TRUE(CompareGradients(
+      "radial-gradient(circle closest-corner at 100px 60px, blue, red)",
+      "radial-gradient(circle closest-corner at 100px 60px, blue, red)"));
+  EXPECT_TRUE(CompareGradients(
+      "radial-gradient(100px 150px at 100px 60px, blue, red)",
+      "radial-gradient(100px 150px at 100px 60px, blue, red)"));
+
+  // Identical with differing parameterization.
+  EXPECT_TRUE(CompareGradients(
+      "radial-gradient(100px 150px at 100px 60px, blue, red)",
+      "radial-gradient(ellipse 100px 150px at 100px 60px, blue, red)"));
+  EXPECT_TRUE(CompareGradients(
+      "radial-gradient(100px at 100px 60px, blue, red)",
+      "radial-gradient(circle 100px at 100px 60px, blue, red)"));
+  EXPECT_TRUE(CompareGradients(
+      "radial-gradient(closest-corner at 100px 60px, blue, red)",
+      "radial-gradient(ellipse closest-corner at 100px 60px, blue, red)"));
+  EXPECT_TRUE(CompareGradients(
+      "radial-gradient(ellipse at 100px 60px, blue, red)",
+      "radial-gradient(ellipse farthest-corner at 100px 60px, blue, red)"));
+
+  // Different.
+  EXPECT_FALSE(CompareGradients(
+      "radial-gradient(circle closest-corner at 100px 60px, blue, red)",
+      "radial-gradient(circle farthest-side  at 100px 60px, blue, red)"));
+  EXPECT_FALSE(CompareGradients(
+      "radial-gradient(circle at 100px 60px, blue, red)",
+      "radial-gradient(circle farthest-side  at 100px 60px, blue, red)"));
+  EXPECT_FALSE(CompareGradients(
+      "radial-gradient(100px 150px at 100px 60px, blue, red)",
+      "radial-gradient(circle farthest-side  at 100px 60px, blue, red)"));
+  EXPECT_FALSE(
+      CompareGradients("radial-gradient(100px 150px at 100px 60px, blue, red)",
+                       "radial-gradient(100px at 100px 60px, blue, red)"));
+}
+
+}  // namespace
+
+}  // namespace blink