Bug 81587 – Increase code sharing between CSSParser and CSSPropertyLonghand.

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Alexis Menard (darktears)]

Remove the map of CSSPropertyLonghand.

[Alexis Menard (darktears)]

Created attachment 132707 [details]
Patch

[Alexis Menard (darktears)]

Without the patch :

Running CSS/CSSPropertySetterGetter.html (1 of 1)
RESULT CSS: CSSPropertySetterGetter= 1140.55 ms
median= 1139.5 ms, stdev= 5.11346262331 ms, min= 1134.0 ms, max= 1151.0 ms
Finished: 24.360030 s

With the patch :

Running CSS/CSSPropertySetterGetter.html (1 of 1)
RESULT CSS: CSSPropertySetterGetter= 1125.6 ms
median= 1123.0 ms, stdev= 7.052659073 ms, min= 1119.0 ms, max= 1147.0 ms
Finished: 24.047662 s

[Luke Macpherson]

See also https://bugs.webkit.org/show_bug.cgi?id=81611

[Tony Chang]

Comment on attachment 132707 [details]
Patch

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

> Source/WebCore/css/CSSPropertyLonghand.cpp:371
> +static inline CSSPropertyLonghand& webkitWrapLonghands()
> +{
> +    static const int webkitWrapProperties[] = {

Do we actually need all these functions?  How does it look to just inlines these in longhandForProperty?

[Tony Chang]

Comment on attachment 132707 [details]
Patch

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

> Source/WebCore/css/CSSPropertyLonghand.cpp:380
> +static CSSPropertyLonghand defaultLonghand;

This looks like a global static. Can you put this in longhandForProperty?

[Alexis Menard (darktears)]

Created attachment 133235 [details]
Patch

[Alexis Menard (darktears)]

Created attachment 133243 [details]
Patch

[Build Bot]

Comment on attachment 133243 [details]
Patch

Attachment 133243 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12070991

[Antti Koivisto]

Comment on attachment 133243 [details]
Patch

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

r=me

> Source/WebCore/css/CSSPropertyLonghand.cpp:31
> +CSSPropertyLonghand& backgroundLonghand()
> +{

These should be 

const CSSPropertyLonghand&

> Source/WebCore/css/CSSPropertyLonghand.cpp:59
> +CSSPropertyLonghand& backgroundRepeatLonghand()
> +{
> +    static const int backgroundRepeatProperties[] = { CSSPropertyBackgroundRepeatX, CSSPropertyBackgroundRepeatY };
> +    DEFINE_STATIC_LOCAL(CSSPropertyLonghand, backgroundRepeatLonghand, (backgroundRepeatProperties, WTF_ARRAY_LENGTH(backgroundRepeatProperties)));
> +    return backgroundRepeatLonghand;
> +}

These should be 

const CSSPropertyLonghand&

[Alexis Menard (darktears)]

Created attachment 133247 [details]
Patch for landing

[Build Bot]

Comment on attachment 133247 [details]
Patch for landing

Attachment 133247 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12066959

[Alexis Menard (darktears)]

Created attachment 133253 [details]
Patch for landing

[Build Bot]

Comment on attachment 133253 [details]
Patch for landing

Attachment 133253 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12067018

[Alexis Menard (darktears)]

Created attachment 133261 [details]
Patch for landing

[Build Bot]

Comment on attachment 133261 [details]
Patch for landing

Attachment 133261 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12114041

[WebKit Review Bot]

Comment on attachment 133261 [details]
Patch for landing

Attachment 133261 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12117050

[Alexis Menard (darktears)]

Created attachment 133273 [details]
Patch for landing

[Alexis Menard (darktears)]

(In reply to comment #17)
> Created an attachment (id=133273) [details]
> Patch for landing

Should fix the warnings about integer vs unsigned comparison.

[Alexis Menard (darktears)]

Created attachment 133287 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 133287 [details]
Patch for landing

Clearing flags on attachment: 133287

Committed r111731: <http://trac.webkit.org/changeset/111731>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.