[webkit-reviews] review granted: [Bug 131685] std::bitset<>::test() does unnecessary bounds checks on CSSPropertyID bitsets : [Attachment 230197] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sun Apr 27 19:58:33 PDT 2014
Darin Adler <darin at apple.com> has granted Zan Dobersek
<zandobersek at gmail.com>'s request for review:
Bug 131685: std::bitset<>::test() does unnecessary bounds checks on
CSSPropertyID bitsets
https://bugs.webkit.org/show_bug.cgi?id=131685
Attachment 230197: Patch
https://bugs.webkit.org/attachment.cgi?id=230197&action=review
------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=230197&action=review
> Source/WebCore/css/StyleProperties.cpp:928
> + ASSERT(!shorthandPropertyID || shortPropertyIndex <
shorthandPropertyUsed.size());
Wouldn’t it be better to put this assertion inside the if just below to avoid
the "||" here?
> Source/WebCore/css/StyleResolver.cpp:191
> - bool hasProperty(CSSPropertyID id) const { return
m_propertyIsPresent.test(id); }
> + bool hasProperty(CSSPropertyID id) const
> + {
> + ASSERT(id < m_propertyIsPresent.size());
> + return m_propertyIsPresent[id];
> + }
Normally when a function body grows to be nontrivial like this we like to put
it in the header after the class definition, with the inline keyword, rather
than leaving it inside the class definition.
More information about the webkit-reviews
mailing list