[webkit-reviews] review denied: [Bug 93574] Convert CSSParser's m_reusableSelectorVector to OwnPtr and rename to m_selectorVector. : [Attachment 157381] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Aug 8 22:03:15 PDT 2012
Tony Chang <tony at chromium.org> has denied Shane Stephens
<shanestephens at google.com>'s request for review:
Bug 93574: Convert CSSParser's m_reusableSelectorVector to OwnPtr and rename to
m_selectorVector.
https://bugs.webkit.org/show_bug.cgi?id=93574
Attachment 157381: Patch
https://bugs.webkit.org/attachment.cgi?id=157381&action=review
------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=157381&action=review
r- for the ChangeLog.
After you upload a new patch, let's wait a day so people in other times zones
can look and provide feedback.
> Source/WebCore/ChangeLog:22
> + * css/CSSGrammar.y:
> + * css/CSSParser.cpp:
> + (WebCore::CSSParser::CSSParser):
> + (WebCore::CSSParser::parseValue):
> + (WebCore::CSSParser::parseColor):
Please annotate some of the major changes here. For example, renaming
reusableSelectorVector to selectorVector or where the typedef is added.
> Source/WebCore/css/CSSParser.cpp:1159
> + declaration->addParsedProperties(*m_parsedProperties.get());
Nit: I don't think you need the .get().
> Source/WebCore/css/CSSParser.cpp:1250
> - declaration->addParsedProperties(m_parsedProperties);
> + declaration->addParsedProperties(*m_parsedProperties.get());
Nit: I don't think you need the .get().
> Source/WebCore/css/CSSParserValues.h:109
> +typedef Vector<OwnPtr<CSSParserSelector> > CSSSelectorVector;
If we moved this typedef into CSSParserSelector, would we have to say
CSSParserSelector::CSSSelectorVector in many places?
More information about the webkit-reviews
mailing list