[webkit-reviews] review granted: [Bug 87506] Make StylePropertySet a variable-sized object to reduce memory use. : [Attachment 144548] I met my wife on patch.com

[bugzilla-daemon at webkit.org]

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

Attachment 144548: I met my wife on patch.com
https://bugs.webkit.org/attachment.cgi?id=144548&action=review

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


Nice, r=me.

> Source/WebCore/css/StylePropertySet.cpp:64
> +    RefPtr<StylePropertySet> result = adoptRef(new
StylePropertySet(CSSStrictMode));
> +    *result->m_vector = properties;
> +    return result.release();
> +}

Usually the vector would be passed in as constructor parameter instead of being
set in create(). Mutability bit could be passed in as enum.

> Source/WebCore/css/StylePropertySet.cpp:120
> +    if (other.isMutable())
> +	   *m_vector = *other.m_vector;
> +    else {

Use early return.

> Source/WebCore/css/StylePropertySet.h:58
> +    inline unsigned propertyCount() const;
> +    inline bool isEmpty() const;
> +    inline const CSSProperty& propertyAt(unsigned index) const;
> +    inline CSSProperty& propertyAt(unsigned index);

The inline keywords should go to the definition. They are not needed on
declaration.

> Source/WebCore/css/StylePropertySet.h:152
> +    unsigned m_arraySize : 28;
> +
> +    union {
> +	   Vector<CSSProperty>* m_vector;
> +	   void* m_properties;
> +    };

m_vector could use a more descriptive name. m_mutablePropertyVector or
something?