Bug 80653

Summary: CSSParser: Use Vector for intermediate property storage.
Product: WebKit Reporter: Andreas Kling <kling>
Component: CSSAssignee: Andreas Kling <kling>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, koivisto, macpherson, menard, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Andreas Kling 2012-03-08 16:13:22 PST
CSSParser is doing a bunch of strange things to manage its internal CSSProperty storage. We should switch to using a normal Vector and remove the custom allocation code.
Comment 1 Andreas Kling 2012-03-08 16:16:06 PST
Created attachment 130934 [details]
Patch
Comment 2 WebKit Review Bot 2012-03-08 16:19:24 PST
Attachment 130934 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/css/CSSParser.cpp:2738:  Declaration has space between type name and * in CSSValue *value  [whitespace/declaration] [3]
Source/WebCore/css/CSSParser.cpp:2748:  Declaration has space between type name and * in CSSValue *value  [whitespace/declaration] [3]
Source/WebCore/css/CSSParser.cpp:2758:  Declaration has space between type name and * in CSSValue *value  [whitespace/declaration] [3]
Total errors found: 3 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Alexis Menard (darktears) 2012-03-08 17:05:01 PST
Comment on attachment 130934 [details]
Patch

Nice patch. LGTM
Comment 4 Luke Macpherson 2012-03-08 20:41:58 PST
Comment on attachment 130934 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=130934&action=review

> Source/WebCore/css/CSSParser.cpp:8871
> +        CSSProperty& property = m_parsedProperties[i];

looks like an iterator would drop in here.

> Source/WebCore/css/CSSParser.cpp:9017
> +        ++i;

Could you use reverse iteration to get around having to retry removed indicies?
Comment 5 Andreas Kling 2012-03-09 11:05:47 PST
Comment on attachment 130934 [details]
Patch

Clearing flags on attachment: 130934

Committed r110305: <http://trac.webkit.org/changeset/110305>
Comment 6 Andreas Kling 2012-03-09 11:05:56 PST
All reviewed patches have been landed.  Closing bug.
Comment 7 Andras Becsi 2012-03-10 05:03:31 PST
*** Bug 19417 has been marked as a duplicate of this bug. ***