Add methods to CounterDirectives to clean up StyleBuilder and RenderCounter.
Created attachment 159653 [details] Patch
Comment on attachment 159653 [details] Patch 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.
This fixes 94642 because it's more careful about initializing the values.
(In reply to comment #2) > (From update of attachment 159653 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=159653&action=review > ... > > > 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. Indeed it is something you can just add to the "current value", the catch being that on a reset CounterNode the current value is 0, while on other nodes the current value is the value of our ancestor node. I changed this to combinedValue(). Overall it would be nice to get rid of CounterDirectives entirely, it's a confusing container for what should just be two maps and RenderStyle should have something like getCounterReset(String& identifier) and getCounterIncrement(String& identifier) that return a struct CounterValue() { bool isDefined(); int value(); } or something like that that.
Created attachment 162643 [details] Patch for landing
Comment on attachment 162643 [details] Patch for landing Clearing flags on attachment: 162643 Committed r127826: <http://trac.webkit.org/changeset/127826>
All reviewed patches have been landed. Closing bug.