Handle initial values in background-repeat introduced by the background shorthand

Source/core/css/StylePropertySerializer.cpp

 /*
  * (C) 1999-2003 Lars Knoll (knoll@kde.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2011 Research In Motion Limited. All rights reserved.
  * Copyright (C) 2013 Intel Corporation. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
@@ skipping 684 matching lines @@
             continue;
         if (!result.isEmpty())
             result.append(' ');
         result.append(value);
     }
     if (isInitialOrInherit(commonValue))
         return commonValue;
     return result.isEmpty() ? String() : result.toString();
 }
 
-static void buildSingleBackgroundRepeatValue(StringBuilder& builder, const CSSValue& repeatXValue, const CSSValue& repeatYValue)
+static void appendBackgroundRepeatValue(StringBuilder& builder, const CSSValue& repeatXCSSValue, const CSSValue& repeatYCSSValue)
 {
-    CSSValueID repeatXValueId = toCSSPrimitiveValue(repeatXValue).getValueID();
-    CSSValueID repeatYValueId = toCSSPrimitiveValue(repeatYValue).getValueID();
+    // FIXME: Ensure initial values do not appear in CSS_VALUE_LISTS.

[rune, 2014/05/31 22:36:01]

Shouldn't this be fixed instead?

This seems to be broken for background-attachment as well.

[alancutter (OOO until 2018), 2014/06/01 03:57:37]

That was my initial approach but the change was getting too large for the type
of bug I'm fixing.
It also has implications for the way we serialise shorthand properties as we
would no longer know whether an initial value was specified as initial or the
equivalent initial value. Too much yak shave for this particular bug.

[rune, 2014/06/02 06:54:50]

I cannot access the issue for this CL in bugs.chromium.org, btw. So I don't know
the importance for it.

I reckon this it's a larger piece of work to fix this properly. Also,
getLayeredShorthandValue have FIXME and Bug references. This code needs some
proper fixing in general it seems.

I'd really love to see this code reworked.

As a local fix this lgtm, but perhaps there are other owners who have stronger
opinions and more knowledge in this area.

+    DEFINE_STATIC_REF_WILL_BE_PERSISTENT(CSSPrimitiveValue, initialRepeatValue, CSSPrimitiveValue::create(CSSValueRepeat));
+    const CSSPrimitiveValue& repeatX = repeatXCSSValue.isInitialValue() ? *initialRepeatValue : toCSSPrimitiveValue(repeatXCSSValue);
+    const CSSPrimitiveValue& repeatY = repeatYCSSValue.isInitialValue() ? *initialRepeatValue : toCSSPrimitiveValue(repeatYCSSValue);
+    CSSValueID repeatXValueId = repeatX.getValueID();
+    CSSValueID repeatYValueId = repeatY.getValueID();
     if (repeatXValueId == repeatYValueId) {
-        builder.append(repeatXValue.cssText());
+        builder.append(repeatX.cssText());
     } else if (repeatXValueId == CSSValueNoRepeat && repeatYValueId == CSSValueRepeat) {
         builder.append("repeat-y");
     } else if (repeatXValueId == CSSValueRepeat && repeatYValueId == CSSValueNoRepeat) {
         builder.append("repeat-x");
     } else {
-        builder.append(repeatXValue.cssText());
+        builder.append(repeatX.cssText());
         builder.append(" ");
-        builder.append(repeatYValue.cssText());
+        builder.append(repeatY.cssText());
     }
 }
 
 String StylePropertySerializer::backgroundRepeatPropertyValue() const
 {
     RefPtrWillBeRawPtr<CSSValue> repeatX = m_propertySet.getPropertyCSSValue(CSSPropertyBackgroundRepeatX);
     RefPtrWillBeRawPtr<CSSValue> repeatY = m_propertySet.getPropertyCSSValue(CSSPropertyBackgroundRepeatY);
     if (!repeatX || !repeatY)
         return String();
     if (m_propertySet.propertyIsImportant(CSSPropertyBackgroundRepeatX) != m_propertySet.propertyIsImportant(CSSPropertyBackgroundRepeatY))
@@ skipping 21 matching lines @@
         repeatYList = toCSSValueList(repeatY.get());
     } else {
         return String();
     }
 
     size_t shorthandLength = lowestCommonMultiple(repeatXList->length(), repeatYList->length());
     StringBuilder builder;
     for (size_t i = 0; i < shorthandLength; ++i) {
         if (i)
             builder.append(", ");
-        buildSingleBackgroundRepeatValue(builder,
+        appendBackgroundRepeatValue(builder,
             *repeatXList->item(i % repeatXList->length()),
             *repeatYList->item(i % repeatYList->length()));
     }
     return builder.toString();
 }
 
 void StylePropertySerializer::appendBackgroundPropertyAsText(StringBuilder& result, unsigned& numDecls) const
 {
     if (isPropertyShorthandAvailable(backgroundShorthand())) {
         String backgroundValue = getPropertyValue(CSSPropertyBackground);
@@ skipping 89 matching lines @@
             isInitialValue = false;
         if (!value->isInheritedValue())
             isInheritedValue = false;
         if (isImportant != m_propertySet.propertyIsImportant(shorthand.properties()[i]))
             return false;
     }
     return isInitialValue || isInheritedValue;
 }
 
 }