[webkit-reviews] review granted: [Bug 76600] DynamicNodeList: Simplify internal Caches object. : [Attachment 123060] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Andreas Kling <kling at webkit.org>'s
request for review:
Bug 76600: DynamicNodeList: Simplify internal Caches object.
https://bugs.webkit.org/show_bug.cgi?id=76600

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

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=123060&action=review


Excellent! Saves 4 bytes per node list!

>> Source/WebCore/dom/DynamicNodeList.cpp:99
>> +	    if (offset == m_caches.lastItemOffset)
> 
> An else if statement should be written as an if statement when the prior "if"
concludes with a return, break, continue or goto statement. 
[readability/control_flow] [4]

Please address this style warning before landing the patch.

> Source/WebCore/dom/DynamicNodeList.h:69
> +	   unsigned cachedLength;
> +	   unsigned lastItemOffset;
> +	   bool isLengthCacheValid : 1;
> +	   bool isItemCacheValid : 1;

You should steal 2 bits from lastItemOffset. Be sure to add a compile assert &
change bools to unsigned to work-around MSVC behavior.