Partial implementation of inline StylePropertyMap.

third_party/WebKit/Source/core/css/cssom/InlineStylePropertyMap.cpp

+// Copyright 2016 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 fil
+
+#include "core/css/cssom/InlineStylePropertyMap.h"
+
+#include "core/CSSPropertyNames.h"
+#include "core/css/CSSPrimitiveValue.h"
+#include "core/css/CSSValueList.h"
+#include "core/css/StylePropertySet.h"
+#include "core/css/cssom/CSSOMTypes.h"
+#include "core/css/cssom/SimpleLength.h"
+
+namespace blink {
+
+StyleValue* InlineStylePropertyMap::get(CSSPropertyID propertyID)
+{
+    if (!m_cleanStyles.get(propertyID)) {

[Timothy Loh, 2016/02/09 07:29:32]

Maybe better to move the duplicated logic here and in getAll to updateProperty
and rename to (e.g.) updatePropertyIfNeeded

[meade_UTC10, 2016/02/10 05:55:39]

Done.

+        // First update the property if it is not clean.
+        updateProperty(propertyID, m_ownerElement->ensureMutableInlineStyle().getPropertyCSSValue(propertyID));
+    }
+
+    if (!m_styles.contains(propertyID)) {
+        return nullptr;
+    }
+    HeapVector<Member<StyleValue>>& styleVector = ensurePropertyList(propertyID);
+    if (styleVector.size() < 1) {

[Timothy Loh, 2016/02/09 07:29:32]

There's probably an empty() or isEmpty() function to use here and everywhere
else. BTW I'm not sure this was the intention of the get/getAll split, it might
have been that this always returns the first value if there are any.

[meade_UTC10, 2016/02/10 05:55:39]

Done. AFAIK it's supposed to return the first value if there is one or greater.

+        return nullptr;
+    }
+    return styleVector.at(0);
+}
+
+HeapVector<Member<StyleValue>> InlineStylePropertyMap::getAll(CSSPropertyID propertyID)
+{
+    if (!m_cleanStyles.get(propertyID)) {
+        // First update the property if it is not clean.
+        updateProperty(propertyID, m_ownerElement->ensureMutableInlineStyle().getPropertyCSSValue(propertyID));
+    }
+
+    if (!m_styles.contains(propertyID))
+        return HeapVector<Member<StyleValue>>();
+
+    return ensurePropertyList(propertyID);
+}
+
+bool InlineStylePropertyMap::has(CSSPropertyID propertyID)
+{
+    return m_styles.contains(propertyID) && (ensurePropertyList(propertyID).size() > 0);
+}
+
+Vector<String> InlineStylePropertyMap::getProperties()
+{
+    Vector<String> result;
+    for (const auto key : m_styles.keys()) {

[Timothy Loh, 2016/02/09 07:29:32]

Using a "const auto" type does a copy of the value without telling that the type
is, so my default reaction is always "what's the actual type that we're
copying". Better to just write out the type here (and elsewhere in the patch).
You can use "for (const auto& x :" if the object has a copying cost and the type
is particularly long.

[meade_UTC10, 2016/02/10 05:55:39]

Done.

+        result.append(getPropertyNameString(key));
+    }
+    return result;
+}
+
+void InlineStylePropertyMap::set(CSSPropertyID propertyID, StyleValueOrStyleValueSequenceOrString& item, ExceptionState& exceptionState)
+{
+    if (!item.isNull()) {
+        if (m_styles.contains(propertyID))

[Timothy Loh, 2016/02/09 07:29:32]

I think remove() does this check, so we don't need to explicitly do it (it costs
an extra hash lookup)

[meade_UTC10, 2016/02/10 05:55:39]

Done.

+            m_styles.remove(propertyID);
+        append(propertyID, item, exceptionState);

[Timothy Loh, 2016/02/09 07:29:32]

doesn't this not work for non-lists? because append() checks
propertySupportsMultiple

[meade_UTC10, 2016/02/10 05:55:39]

As implemented, it only does that if it's a singlular StyleValue and it's not
the first value appended. So it does work, however append will always work the
first time for non-list values, which may be confusing. As implemented it's
convenient because I can avoid having similar code paths, but this probably
needs a short discussion.

+    } else {
+        // Clear it.
+        remove(propertyID, exceptionState);
+    }
+}
+
+void InlineStylePropertyMap::append(CSSPropertyID propertyID, StyleValueOrStyleValueSequenceOrString& item, ExceptionState& exceptionState)
+{
+    ASSERT(propertyID != CSSPropertyInvalid);
+
+    // TODO other pre-validation?
+    HeapVector<Member<StyleValue>>& values = ensurePropertyList(propertyID);
+
+    if (item.isStyleValue()) {
+        // Setting a single value.
+        StyleValue* value = item.getAsStyleValue();
+        if (!CSSOMTypes::propertyCanTake(propertyID, value->type())) {
+            exceptionState.throwTypeError("Invalid type for property");
+            return;
+        }
+        if (values.size() != 0 && !CSSOMTypes::propertySupportsMultiple(propertyID)) {
+            exceptionState.throwTypeError("Property does not support multiple values");
+            return;
+        }
+        values.append(value);
+
+    } else if (item.isStyleValueSequence()) {
+        if (!CSSOMTypes::propertySupportsMultiple(propertyID)) {
+            exceptionState.throwTypeError("Property does not support multiple values");
+        }
+        // Check all the values in the sequence to make sure they're valid.
+        const HeapVector<Member<StyleValue>> specifiedValues = item.getAsStyleValueSequence();
+        for (const auto value : specifiedValues) {
+            if (!CSSOMTypes::propertyCanTake(propertyID, value->type())) {
+                exceptionState.throwTypeError("Invalid type for property");
+                return;
+            }
+        }
+        values.appendVector(specifiedValues);
+
+    } else if (item.isString()) {
+        // Parse it.
+        // TODO(meade): Implement this.
+        exceptionState.throwTypeError("Not implemented yet");
+    }
+
+    if (values.size() == 1) {
+        m_ownerElement->setInlineStyleProperty(propertyID, values[0]->toCSSValue());

[Timothy Loh, 2016/02/09 07:29:32]

Properties which accept list-of-T probably wouldn't work if you just had a T.

[meade_UTC10, 2016/02/10 05:55:39]

Done.

+    } else {
+        RefPtrWillBeRawPtr<CSSValueList> valueList = CSSValueList::createSpaceSeparated();
+        for (const auto value : values) {
+            valueList->append(value->toCSSValue());
+        }
+        m_ownerElement->setInlineStyleProperty(propertyID, valueList);
+    }
+    m_cleanStyles.set(propertyID, true);
+}
+
+void InlineStylePropertyMap::remove(CSSPropertyID propertyID, ExceptionState& exceptionState)
+{
+    if (!m_styles.contains(propertyID)) {

[Timothy Loh, 2016/02/09 07:29:32]

Better to do something like below and avoid an extra hash lookup:

foo::iterator iterator = m_styles.find(propertyID);
if (iterator == m_styles.end())
    return;
m_styles.remove(iterator);
...

[meade_UTC10, 2016/02/10 05:55:39]

Done.

+        return;
+    }
+    m_styles.remove(propertyID);
+    m_ownerElement->removeInlineStyleProperty(propertyID);
+    m_cleanStyles.set(propertyID, true);
+}
+
+void InlineStylePropertyMap::updateProperty(CSSPropertyID propertyID, PassRefPtrWillBeRawPtr<CSSValue> cssValue)
+{
+    HeapVector<Member<StyleValue>>& values = ensurePropertyList(propertyID);
+    values.clear();
+
+    if (cssValue && !cssValue->isValueList() && !cssValue->isValuePair()) {

[Timothy Loh, 2016/02/09 07:29:32]

I think <position> types get stored as pairs, might not want that here.

[meade_UTC10, 2016/02/10 05:55:39]

hmm yeah. Removed.

+        StyleValue* styleValue = StyleValue::create(*cssValue);
+        if (styleValue) {
+            values.append(*styleValue);
+        }
+    }

[Timothy Loh, 2016/02/09 07:29:32]

missing a TODO to handle the other case(s)?

[meade_UTC10, 2016/02/10 05:55:39]

Done.

+
+    m_cleanStyles.set(propertyID, true);
+}
+
+void InlineStylePropertyMap::updateCustomProperty(const String& propertyName, PassRefPtrWillBeRawPtr<CSSValue> cssValue)
+{
+    // TODO(meade): Implement.
+}
+
+HeapVector<Member<StyleValue>>& InlineStylePropertyMap::ensurePropertyList(CSSPropertyID propertyID)
+{
+    if (!m_styles.contains(propertyID)) {
+        m_styles.set(propertyID, HeapVector<Member<StyleValue>>());
+    }
+    // Note: If you write return m_style.get(propertyID) here, you get a copy.
+    return m_styles.find(propertyID)->value;
+}
+
+} // namespace blink
+