Implement color stop position syntax from CSS Image Values 4

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

Index: third_party/WebKit/Source/core/css/CSSGradientValue.cpp
diff --git a/third_party/WebKit/Source/core/css/CSSGradientValue.cpp b/third_party/WebKit/Source/core/css/CSSGradientValue.cpp
index 0bed19512a45267777c92044beaa6f14bdc2a1f7..5cdcaf44038153fc163556d387273a218b2686f6 100644
--- a/third_party/WebKit/Source/core/css/CSSGradientValue.cpp
+++ b/third_party/WebKit/Source/core/css/CSSGradientValue.cpp
@@ -1074,14 +1074,21 @@ DEFINE_TRACE_AFTER_DISPATCH(CSSLinearGradientValue) {
 void CSSGradientValue::appendCSSTextForColorStops(
     StringBuilder& result,
     bool requiresSeparator) const {
-  for (const auto& stop : m_stops) {
+  for (size_t i = 0; i < m_stops.size(); ++i) {
+    const auto& stop = m_stops[i];
+    const bool isColorRepeat =
+        (i > 0) && !stop.isHint() && stop.m_color == m_stops[i - 1].m_color;
+    // At most two consecutive stops can share the same CSS value for color.

[fs, 2017/04/06 19:58:48]

Could we add a test for this as well - both the "compressing" behavior ("blue
10%, blue 20%" serializing as "blue 10% 20%") and that more than 2 stops are not
joined ("blue 10%, blue 20%, blue 30%" -> "blue 10% 20%, blue 30%" or so I
presume...) It might be easier to write this in "forward consuming" fashion
rather than "backward merge" (like this). Also, another case to test: "blue 10%,
blue 20% 30%".

[f(malita), 2017/04/06 20:29:42]

Actually, the current impl bends over backwards to preserve the original format:

  "blue 10%, blue 20%" -> "blue 10%, blue 20%"
  "blue 10% 20%"       -> "blue 10% 20%"

(we use the CSSValue pointer to detect whether the color value is shared by two
consecutive stops, and only compress if that's the case)

The reason is this serializer appears to feed getComputedValue(), and the
computed value for background-image is supposed to be "as specified, but with
URIs made absolute"
(https://www.w3.org/TR/css3-background/#the-background-image).

I don't feel strong either way, but based on the spec language it seemed more
prudent to preserve the input as much as possible.  WDYT?

[fs, 2017/04/06 20:44:36]

Oh, yeah, right, the stop.color == stops[i - 1].color was comparing the
objects...
 
I was looking around to find relevant serialization rules, and the only thing I
found was the ones in CSS Image L3:
https://drafts.csswg.org/css-images-3/#serialization ; which I guess essentially
says to emit "as little as possible". I'm not sure if I think this "as
specified" in css3-background actually makes sense (it does for <url> but that
not much to go on), since at least 'currentColor' stops ought to compute to the
actual color (not that it does in Blink.) Anyway, we can keep it as-is (this is
fuzzy territory after all), but then having the tests would be even more
valuable I think =).

+    DCHECK(!isColorRepeat || i < 2 || stop.m_color != m_stops[i - 2].m_color);
+
     if (requiresSeparator) {
-      result.append(", ");
+      if (!isColorRepeat)
+        result.append(", ");
     } else {
       requiresSeparator = true;
     }
 
-    if (stop.m_color)
+    if (stop.m_color && !isColorRepeat)
       result.append(stop.m_color->cssText());
     if (stop.m_color && stop.m_offset)
       result.append(' ');