[webkit-reviews] review granted: [Bug 100753] Pack immutable StylePropertySets harder on 64-bit. : [Attachment 171656] Oh yes, it is a patch.

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted Andreas Kling <kling at webkit.org>'s
request for review:
Bug 100753: Pack immutable StylePropertySets harder on 64-bit.
https://bugs.webkit.org/show_bug.cgi?id=100753

Attachment 171656: Oh yes, it is a patch.
https://bugs.webkit.org/attachment.cgi?id=171656&action=review

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


r=me, good idea!

> Source/WebCore/css/CSSProperty.h:58
> -    {
> -    }
> +    { }

I think the original was the correct style.

> Source/WebCore/css/StylePropertySet.cpp:1195
> +String StylePropertySet::PropertyReference::cssName() const
> +{
> +#if ENABLE(CSS_VARIABLES)
> +    if (id() == CSSPropertyVariable) {
> +	   ASSERT(propertyValue()->isVariableValue());
> +	   return "-webkit-var-" + static_cast<const
CSSVariableValue*>(propertyValue())->name();
> +    }
> +#endif
> +    return getPropertyNameString(id());
> +}
> +
> +String StylePropertySet::PropertyReference::cssText() const
> +{
> +    StringBuilder result;
> +    result.append(cssName());
> +    result.appendLiteral(": ");
> +    result.append(propertyValue()->cssText());
> +    if (isImportant())
> +	   result.appendLiteral(" !important");
> +    result.append(';');
> +    return result.toString();
> +}

I think it would be slightly better if PropertyReference would focus on being a
reference and would not have complex functions with CSS semantics hanging from
it. Maybe these could just be free standing?

> Source/WebCore/css/StylePropertySet.h:72
> +	   String cssName() const;
> +	   String cssText() const;

It might be nicer if S