[webkit-reviews] review granted: [Bug 88240] Make ElementAttributeData a variable-sized object to reduce memory use. : [Attachment 145604] Pøtch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted Andreas Kling <kling at webkit.org>'s
request for review:
Bug 88240: Make ElementAttributeData a variable-sized object to reduce memory
use.
https://bugs.webkit.org/show_bug.cgi?id=88240

Attachment 145604: Pøtch
https://bugs.webkit.org/attachment.cgi?id=145604&action=review

------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
View in context: https://bugs.webkit.org/attachment.cgi?id=145604&action=review


r=me, nice!

> Source/WebCore/dom/Element.cpp:684
>  inline void Element::setAttributeInternal(size_t index, const QualifiedName&
name, const AtomicString& value, EInUpdateStyleAttribute
inUpdateStyleAttribute)
>  {
> +    mutableAttributeData();

This looks lonely. It would perhaps be nicer to use the returned value instead
of accessing attributes directly.

> Source/WebCore/dom/Element.cpp:2123
> +void Element::createMutableAttributeData()
> +{
> +    if (!m_attributeData)
> +	   m_attributeData = ElementAttributeData::create();

It would be nicer if this was called ElementAttributeData::createMutable()
(same for StylePropertySet::create() too)

> Source/WebCore/dom/ElementAttributeData.cpp:54
> +ElementAttributeData::ElementAttributeData(const Vector<Attribute>&
attributes)
> +    : m_isMutable(false)
> +    , m_arraySize(attributes.size())
> +{
> +    for (unsigned i = 0; i < attributes.size(); ++i)
> +	   new (&array()[i]) Attribute(attributes[i]);
> +}

Anders tells that we should be able to generalize this pattern into a template.
Would be nice to do that at some point as we can probably find other uses.