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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Mar 30 19:23:47 PDT 2011


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





--- Comment #55 from James Robinson <jamesr at chromium.org>  2011-03-30 19:23:45 PST ---
(From update of attachment 85506)
View in context: https://bugs.webkit.org/attachment.cgi?id=85506&action=review

>> Source/WebCore/css/CSSStyleApplyProperty.cpp:48
>> +    ApplyPropertyDefault(T (RenderStyle::*getter)() const, void (RenderStyle::*setter)(T), T (*initial)())
> 
> Use typedefs to avoid repeating the function pointers.
> 
> typedef T (RenderStyle::*GetterMethod)() const;
> typedef T void (RenderStyle::*SetterMethod)(T);
> typedef T (*initialMethod)();
> 
> ..
> protected:
> GetterMethod m_getter;
> ..

Yeah, that seems significantly easier to handle.

>> Source/WebCore/css/CSSStyleApplyProperty.cpp:57
>> +        (selector->style()->*m_setter)((selector->parentStyle()->*m_getter)());
> 
> While you're at it: I don't think ASSERTIONS() will hurt here.

What would you ASSERT() here?

>> Source/WebCore/css/CSSStyleApplyProperty.cpp:67
>> +        if (value->isPrimitiveValue())
> 
> Early return style is prefereed:
> if (!value->isPrimitiveValue())
>     return;

Early return for a 2-line function is not beneficial.  It turns a 2-line function into a 3-line function that is no easier to understand.

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



More information about the webkit-unassigned mailing list