[Webkit-unassigned] [Bug 54707] Introduce lookup-table based approach for applying CSS properties.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=54707


Eric Seidel <eric at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #83271|review?                     |review-
               Flag|                            |




--- Comment #14 from Eric Seidel <eric at webkit.org>  2011-02-24 02:31:30 PST ---
(From update of attachment 83271)
View in context: https://bugs.webkit.org/attachment.cgi?id=83271&action=review

> Source/WebCore/css/CSSStyleApplyProperty.cpp:48
> +        : m_prop(prop)

I would have written out m_prop as m_property.

> Source/WebCore/css/CSSStyleApplyProperty.cpp:54
> +    virtual void inherit(CSSStyleSelector *instance) const = 0;

arg names aren't useful here and should be removed.

> Source/WebCore/css/CSSStyleApplyProperty.cpp:67
> +    virtual void inherit(CSSStyleSelector *instance) const {UNUSED_PARAM(instance);}

Better to not name the instance parameter at all.  Also the * is in the wrong palce.

> Source/WebCore/css/CSSStyleApplyProperty.cpp:134
> +        if (value->isPrimitiveValue()) {

I would have used early return.

> Source/WebCore/css/CSSStyleApplyProperty.cpp:184
> +    m_propertyMap[PROPERTY_INDEX(CSSPropertyDirection)] = m_invalid;

This should just be a helper function.

markInvalid(CSSPropertyName);  Or you could put them in a static array and use a for loop.  your choice.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.