A cleanup.
Created attachment 111587 [details] patch
Comment on attachment 111587 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=111587&action=review r=me. I suspect that this code is vulnerable to some integer overflows with all the assignments of unsigned/size_t values to int variables (the MatchResult members.) Still, r+ since you're just moving the code around. > Source/WebCore/css/CSSStyleSelector.cpp:752 > + if (!resolveForRootDefault) { We should use early return here. > Source/WebCore/css/CSSStyleSelector.cpp:760 > + // Ask if the HTML element has mapped attributes. This comment implies that m_styledElement is an HTML element, but given the isHTMLElement() check further down, is that always the case? > Source/WebCore/css/CSSStyleSelector.cpp:764 > + for (unsigned i = 0; i < map->length(); i++) { Style, ++i. NamedNodeMap::length() returns size_t, not unsigned. > Source/WebCore/css/CSSStyleSelector.cpp:769 > + result.firstAuthorRule =result.lastAuthorRule; Also style, should have space after =. > Source/WebCore/css/CSSStyleSelector.cpp:786 > + for (unsigned i = 0; i < additionalDeclsSize; i++) Style, ++i. > Source/WebCore/css/CSSStyleSelector.cpp:1653 > + // If we have first-letter pseudo style, do not share this style Might as well tack on a period at the end of this comment. > Source/WebCore/css/CSSStyleSelector.cpp:2189 > + // Line-height is set when we are sure we decided on the font-size Ditto.
Comment on attachment 111587 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=111587&action=review > Source/WebCore/css/CSSStyleSelector.cpp:782 > + unsigned additionalDeclsSize = m_additionalAttributeStyleDecls.size(); Vector::size() returns size_t.
Created attachment 111632 [details] new patch Expanded the refactoring a bit so requesting re-review. This patch uses the MatchResult struct in more places and gets rid of the resolveForRootDefault parameter for the new functions.
Comment on attachment 111632 [details] new patch r=me, though let's fight about size_t vs unsigned indexing at some point.
http://trac.webkit.org/changeset/97962