Simplify CSSParser::parseFont.
Created attachment 127166 [details] Patch
Created attachment 127169 [details] Patch
Comment on attachment 127169 [details] Patch Attachment 127169 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11515784 New failing tests: fast/inspector-support/style.html fast/parser/style-script-head-test.html
Created attachment 127190 [details] Patch
Attachment 127190 [details] did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit']" exit_code: 2 Updating OpenSource git.webkit.org[0: 17.254.20.231]: errno=Connection refused fatal: unable to connect a socket (Connection refused) Died at Tools/Scripts/update-webkit line 162. If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 136429 [details] Patch Rebased version
Comment on attachment 136429 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136429&action=review > Source/WebCore/css/CSSParser.cpp:4314 > + if (!fontStyleParsed && parseValue(CSSPropertyFontStyle, important)) > + fontStyleParsed = true; I think we should avoid calling to generic parseValue() from parseFont(). We already know what property this is so jumping through the huge switch seems like wasted work. Perhaps we could refactor this so that both parseValue and this can use the same helper functions. > Source/WebCore/css/CSSParser.cpp:-4594 > - if (m_valueList->size() != 1) > - return false; Only shorthand properties can have list longer than 1, right? You could assert for that.
Created attachment 138169 [details] Patch
(In reply to comment #7) > (From update of attachment 136429 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=136429&action=review > > > Source/WebCore/css/CSSParser.cpp:4314 > > + if (!fontStyleParsed && parseValue(CSSPropertyFontStyle, important)) > > + fontStyleParsed = true; > > I think we should avoid calling to generic parseValue() from parseFont(). We already know what property this is so jumping through the huge switch seems like wasted work. Fixed in the new version. > > Perhaps we could refactor this so that both parseValue and this can use the same helper functions. > > > Source/WebCore/css/CSSParser.cpp:-4594 > > - if (m_valueList->size() != 1) > > - return false; > > Only shorthand properties can have list longer than 1, right? You could assert for that. As discussed we don't really want to ASSERT in case of an invalid property, we just want to skip it.
Comment on attachment 138169 [details] Patch r=me
Comment on attachment 138169 [details] Patch Clearing flags on attachment: 138169 Committed r114895: <http://trac.webkit.org/changeset/114895>
All reviewed patches have been landed. Closing bug.