Summary: | CSSParser::parseValue() copies the m_parsedProperties vector at addParsedProperties() | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||
Component: | CSS | Assignee: | Chris Dumez <cdumez> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cdumez, commit-queue, darin, kling, koivisto, simon.fraser | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=143970 | ||||||||
Attachments: |
|
Description
Simon Fraser (smfr)
2015-04-18 14:58:26 PDT
Very nice find :) We really supposed to use ParsedPropertyVector type here. Created attachment 251171 [details]
Patch
Comment on attachment 251171 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=251171&action=review > Source/WebCore/css/CSSParser.h:84 > + using ParsedPropertyVector = Vector<CSSProperty, 256>; Does the compiler enforce a match between this and the typedef? > Source/WebCore/css/StyleProperties.cpp:770 > + for (auto& property : properties) const auto&? Comment on attachment 251171 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=251171&action=review >> Source/WebCore/css/CSSParser.h:84 >> + using ParsedPropertyVector = Vector<CSSProperty, 256>; > > Does the compiler enforce a match between this and the typedef? Which typedef? I just made the typedef public so it could be used in StyleProperties. I used a 'using' statement but it is equivalent to a typedef in this case. >> Source/WebCore/css/StyleProperties.cpp:770 >> + for (auto& property : properties) > > const auto&? auto will be a const CSSProperty so it is equivalent but OK. Created attachment 251179 [details]
Patch
To avoid mistakes like this in the future, I think we should make the following constructor explicit: template<size_t otherCapacity, typename otherOverflowBehaviour> Vector(const Vector<T, otherCapacity, otherOverflowBehaviour>&); I can look into this in a follow-up patch as it likely is a larger change. Comment on attachment 251179 [details] Patch Clearing flags on attachment: 251179 Committed r183024: <http://trac.webkit.org/changeset/183024> All reviewed patches have been landed. Closing bug. (In reply to comment #6) > To avoid mistakes like this in the future, I think we should make the > following constructor explicit: > template<size_t otherCapacity, typename otherOverflowBehaviour> > Vector(const Vector<T, otherCapacity, otherOverflowBehaviour>&); > > I can look into this in a follow-up patch as it likely is a larger change. -> https://bugs.webkit.org/show_bug.cgi?id=143970 |