Cleanup CSSStyleApplyProperty.cpp, it's pretty hard to read as is, and uses too many Base classes. As first step remove ApplyPropertyColorBase, merge it with ApplyPropertyColor. Use typedefs for the function pointers where possible!
Created attachment 91316 [details] Patch
r=me
Landed in r85069.
Was this just the typedef code taken from my patch https://bug-59414-attachments.webkit.org/attachment.cgi?id=91221 ?
View in context: https://bugs.webkit.org/attachment.cgi?id=91316&action=review Overall I like the cleanup. Thanks. > Source/WebCore/css/CSSStyleApplyProperty.cpp:147 > + (selector->style()->*m_setter)(m_initial ? m_initial() : Color()); Looks good. > Source/WebCore/css/CSSStyleApplyProperty.cpp:155 > + if (m_initial && primitiveValue->getIdent() == CSSValueCurrentcolor) Use of m_initial here seems to add an odd side-effect to that parameter. Perhaps this would be better handled by adding another constructor value "inheritColorFromParent = true".
(In reply to comment #4) > Was this just the typedef code taken from my patch https://bug-59414-attachments.webkit.org/attachment.cgi?id=91221 ? I started from scratch, I forgot that you partly fixed that problem in your patch! (In reply to comment #5) > View in context: https://bugs.webkit.org/attachment.cgi?id=91316&action=review > > Overall I like the cleanup. Thanks. > > > Source/WebCore/css/CSSStyleApplyProperty.cpp:147 > > + (selector->style()->*m_setter)(m_initial ? m_initial() : Color()); > > Looks good. > > > Source/WebCore/css/CSSStyleApplyProperty.cpp:155 > > + if (m_initial && primitiveValue->getIdent() == CSSValueCurrentcolor) > > Use of m_initial here seems to add an odd side-effect to that parameter. Perhaps this would be better handled by adding another constructor value "inheritColorFromParent = true". That's true, if you want to modify it I'm happy to review!