[webkit-reviews] review granted: [Bug 78381] Attribute styles should be created lazily. : [Attachment 126628] Porpoise 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 78381: Attribute styles should be created lazily.
https://bugs.webkit.org/show_bug.cgi?id=78381

Attachment 126628: Porpoise patch
https://bugs.webkit.org/attachment.cgi?id=126628&action=review

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


r=me

Clearly there would be stuff to tweak here but it is good to get this in first.


> Source/WebCore/dom/ElementAttributeData.cpp:56
> +StylePropertySet*
ElementAttributeData::ensureNewAttributeStyle(StyledElement* element)

"ensureNew" sounds bit strange though I don't have good suggestions.

> Source/WebCore/dom/StyledElement.h:72
> +    static void addCSSLengthToStyle(StylePropertySet*, int propertyID, const
String& value);
> +    static void addCSSColorToStyle(StylePropertySet*, int propertyID, const
String& color);

Since these are HTML specific maybe they could move down to HTMLElement

> Source/WebCore/html/HTMLTableColElement.cpp:71
> +	   setNeedsAttributeStyleUpdate();
>	   if (!attr->value().isEmpty()) {
> -	       addCSSLength(CSSPropertyWidth, attr->value());
>	       if (renderer() && renderer()->isTableCol()) {
>		   RenderTableCol* col = toRenderTableCol(renderer());
>		   int newWidth = width().toInt();
>		   if (newWidth != col->width())
>		       col->setNeedsLayoutAndPrefWidthsRecalc();

I suppose width() still gets calculated here?