[webkit-reviews] review granted: [Bug 94587] Add methods to CounterDirectives to clean up StyleBuilder and RenderCounter. : [Attachment 159653] Patch

[bugzilla-daemon at webkit.org]

Julien Chaffraix <jchaffraix at webkit.org> has granted Elliott Sprehn
<esprehn at chromium.org>'s request for review:
Bug 94587: Add methods to CounterDirectives to clean up StyleBuilder and
RenderCounter.
https://bugs.webkit.org/show_bug.cgi?id=94587

Attachment 159653: Patch
https://bugs.webkit.org/attachment.cgi?id=159653&action=review

------- Additional Comments from Julien Chaffraix <jchaffraix at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=159653&action=review


> Source/WebCore/rendering/style/CounterDirectives.h:40
>	   : m_reset(false)
>	   , m_increment(false)

This naming should be improved while we are touching this code. Some better
names:
* m_isResetCounter / m_hasCounterResetSet
* m_isIncrementCounter / m_hasCounterIncrementSet

> Source/WebCore/rendering/style/CounterDirectives.h:86
> +    int value() const

This value name is dangerous as you have lost the information whether you
actually reset. I would have anticipated that value would return something I
could just add to the current value.

Maybe valueForResetOrIncrement would better convey this idea.