[Variables] Enable get/setProperty and similar APIs from the CSSOM

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

Index: third_party/WebKit/Source/core/css/StylePropertySet.cpp
diff --git a/third_party/WebKit/Source/core/css/StylePropertySet.cpp b/third_party/WebKit/Source/core/css/StylePropertySet.cpp
index 62200c2b375ab05ed69fe1f11569e1eacd91c292..e0eaf6b7ba7c5f72b5a65eae77c77bed7f827454 100644
--- a/third_party/WebKit/Source/core/css/StylePropertySet.cpp
+++ b/third_party/WebKit/Source/core/css/StylePropertySet.cpp
@@ -119,6 +119,21 @@ int ImmutableStylePropertySet::findPropertyIndex(CSSPropertyID propertyID) const
     return -1;
 }
 
+int ImmutableStylePropertySet::findCustomPropertyIndex(const AtomicString& propertyName) const
+{
+    // Convert here propertyID into an uint16_t to compare it with the metadata's m_propertyID to avoid
+    // the compiler converting it to an int multiple times in the loop.
+    const uint16_t variableId = static_cast<uint16_t>(CSSPropertyVariable);
+
+    for (int n = m_arraySize - 1 ; n >= 0; --n) {
+        if (metadataArray()[n].m_propertyID == variableId
+            && toCSSCustomPropertyDeclaration(valueArray()[n])->name() == propertyName)
+            return n;
+    }
+
+    return -1;
+}
+
 DEFINE_TRACE_AFTER_DISPATCH(ImmutableStylePropertySet)
 {
     const RawPtrWillBeMember<CSSValue>* values = valueArray();
@@ -148,6 +163,12 @@ String StylePropertySet::getPropertyValue(CSSPropertyID propertyID) const
     return StylePropertySerializer(*this).getPropertyValue(propertyID);
 }
 
+String StylePropertySet::getCustomPropertyValue(const AtomicString& propertyName) const
+{
+    RefPtrWillBeRawPtr<CSSValue> value = getCustomPropertyCSSValue(propertyName);
+    return value ? value->cssText() : "";
+}
+
 PassRefPtrWillBeRawPtr<CSSValue> StylePropertySet::getPropertyCSSValue(CSSPropertyID propertyID) const
 {
     int foundPropertyIndex = findPropertyIndex(propertyID);
@@ -156,6 +177,14 @@ PassRefPtrWillBeRawPtr<CSSValue> StylePropertySet::getPropertyCSSValue(CSSProper
     return propertyAt(foundPropertyIndex).value();
 }
 
+PassRefPtrWillBeRawPtr<CSSCustomPropertyDeclaration> StylePropertySet::getCustomPropertyCSSValue(const AtomicString& propertyName) const
+{
+    int foundPropertyIndex = findCustomPropertyIndex(propertyName);
+    if (foundPropertyIndex == -1)
+        return nullptr;
+    return toCSSCustomPropertyDeclaration(propertyAt(foundPropertyIndex).value());
+}
+
 DEFINE_TRACE(StylePropertySet)
 {
     if (m_isMutable)
@@ -183,32 +212,43 @@ bool MutableStylePropertySet::removeShorthandProperty(CSSPropertyID propertyID)
     return removePropertiesInSet(shorthand.properties(), shorthand.length());
 }
 
-bool MutableStylePropertySet::removeProperty(CSSPropertyID propertyID, String* returnText)
+bool MutableStylePropertySet::removePropertyAtIndex(int propertyIndex, String* returnText)
 {
-    if (removeShorthandProperty(propertyID)) {
-        // FIXME: Return an equivalent shorthand when possible.
-        if (returnText)
-            *returnText = "";
-        return true;
-    }
-
-    int foundPropertyIndex = findPropertyIndex(propertyID);
-    if (foundPropertyIndex == -1) {
+    if (propertyIndex == -1) {
         if (returnText)
             *returnText = "";
         return false;
     }
 
     if (returnText)
-        *returnText = propertyAt(foundPropertyIndex).value()->cssText();
+        *returnText = propertyAt(propertyIndex).value()->cssText();
 
     // A more efficient removal strategy would involve marking entries as empty
     // and sweeping them when the vector grows too big.
-    m_propertyVector.remove(foundPropertyIndex);
+    m_propertyVector.remove(propertyIndex);
 
     return true;
 }
 
+bool MutableStylePropertySet::removeProperty(CSSPropertyID propertyID, String* returnText)
+{
+    if (removeShorthandProperty(propertyID)) {
+        // FIXME: Return an equivalent shorthand when possible.
+        if (returnText)
+            *returnText = "";
+        return true;
+    }
+
+    int foundPropertyIndex = findPropertyIndex(propertyID);
+    return removePropertyAtIndex(foundPropertyIndex, returnText);
+}
+
+bool MutableStylePropertySet::removeCustomProperty(const AtomicString& propertyName, String* returnText)
+{
+    int foundPropertyIndex = findCustomPropertyIndex(propertyName);
+    return removePropertyAtIndex(foundPropertyIndex, returnText);
+}
+
 bool StylePropertySet::propertyIsImportant(CSSPropertyID propertyID) const
 {
     int foundPropertyIndex = findPropertyIndex(propertyID);
@@ -226,6 +266,14 @@ bool StylePropertySet::propertyIsImportant(CSSPropertyID propertyID) const
     return true;
 }
 
+bool StylePropertySet::customPropertyIsImportant(const AtomicString& propertyName) const
+{
+    int foundPropertyIndex = findCustomPropertyIndex(propertyName);
+    if (foundPropertyIndex != -1)
+        return propertyAt(foundPropertyIndex).isImportant();
+    return false;
+}
+
 CSSPropertyID StylePropertySet::getPropertyShorthand(CSSPropertyID propertyID) const
 {
     int foundPropertyIndex = findPropertyIndex(propertyID);
@@ -254,6 +302,13 @@ bool MutableStylePropertySet::setProperty(CSSPropertyID unresolvedProperty, cons
     return CSSParser::parseValue(this, unresolvedProperty, value, important, contextStyleSheet);
 }
 
+bool MutableStylePropertySet::setCustomProperty(const AtomicString& propertyName, const String& value, bool important, StyleSheetContents* contextStyleSheet)
+{
+    if (value.isEmpty())
+        return removeCustomProperty(propertyName);
+    return CSSParser::parseValueForCustomProperty(this, propertyName, value, important, contextStyleSheet);
+}
+
 void MutableStylePropertySet::setProperty(CSSPropertyID propertyID, PassRefPtrWillBeRawPtr<CSSValue> prpValue, bool important)
 {
     StylePropertyShorthand shorthand = shorthandForProperty(propertyID);
@@ -330,6 +385,7 @@ void MutableStylePropertySet::mergeAndOverrideOnConflict(const StylePropertySet*
     unsigned size = other->propertyCount();
     for (unsigned n = 0; n < size; ++n) {
         PropertyReference toMerge = other->propertyAt(n);
+        // TODO(leviw): This probably doesn't work correctly with Custom Properties
         CSSProperty* old = findCSSPropertyWithID(toMerge.id());
         if (old)
             setProperty(toMerge.toCSSProperty(), old);
@@ -386,6 +442,12 @@ bool MutableStylePropertySet::removePropertiesInSet(const CSSPropertyID* set, un
 
 CSSProperty* MutableStylePropertySet::findCSSPropertyWithID(CSSPropertyID propertyID)
 {
+    // TODO(leviw): Calling this with a custom property should probably assert, or this
+    // method should alternatively take a string used for custom properties and check it
+    // in that case.
+    if (propertyID == CSSPropertyVariable)
+        return nullptr;
+
     int foundPropertyIndex = findPropertyIndex(propertyID);
     if (foundPropertyIndex == -1)
         return nullptr;
@@ -480,6 +542,26 @@ int MutableStylePropertySet::findPropertyIndex(CSSPropertyID propertyID) const
     return (it == end) ? -1 : it - begin;
 }
 
+int MutableStylePropertySet::findCustomPropertyIndex(const AtomicString& propertyName) const
+{
+    const CSSProperty* begin = m_propertyVector.data();
+    const CSSProperty* end = begin + m_propertyVector.size();
+    // Convert here propertyID into an uint16_t to compare it with the metadata's m_propertyID to avoid
+    // the compiler converting it to an int multiple times in the loop.
+    const uint16_t variableId = static_cast<uint16_t>(CSSPropertyVariable);
+
+    auto compare = [variableId, propertyName](const CSSProperty& property) -> bool {
+        if (property.metadata().m_propertyID == variableId) {
+            return toCSSCustomPropertyDeclaration(property.value())->name() == propertyName;
+        }
+        return false;
+    };
+
+    const CSSProperty* it = std::find_if(begin, end, compare);
+
+    return (it == end) ? -1 : it - begin;
+}
+

[alancutter (OOO until 2018), 2015/11/17 07:10:42]

The copy paste in this file is pretty unappealing. If instead of duplicating
StylePropertySet's interface for CSSPropertyID/AtomicString the parameter was
templated we would avoid code duplication and also avoid touching the callsites
to these methods. StylePropertySet.cpp can explicitly instantiate the
CSSPropertyID and AtomicString specialisations to avoid pushing implementations
up into StylePropertySet.h.

WDYT?

[leviw_travelin_and_unemployed, 2015/11/17 07:23:44]

If that saved us code duplication I'd agree. I'm not entirely sure I understand
your suggestion, but if you're saying you'd prefer template specialization to
function overloading in this case, I'd disagree, but I'm not dogmatic about it.

[alancutter (OOO until 2018), 2015/11/18 21:22:51]

Explicit template specialisation isn't something I've done before so I tried it
out to be sure that it avoided code duplication.
I think it turned out okay, the differences between native and custom property
handling is captured in function overloads while everything else stays the same
only templated: https://codereview.chromium.org/1455053003
leviw, timloh, WDYT?

 DEFINE_TRACE_AFTER_DISPATCH(MutableStylePropertySet)
 {
 #if ENABLE(OILPAN)