Bug 81587 - Increase code sharing between CSSParser and CSSPropertyLonghand.
Summary: Increase code sharing between CSSParser and CSSPropertyLonghand.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Alexis Menard (darktears)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-19 16:21 PDT by Alexis Menard (darktears)
Modified: 2012-03-22 10:55 PDT (History)
7 users (show)

See Also:


Attachments
Patch (27.06 KB, patch)
2012-03-19 16:23 PDT, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
Patch (50.12 KB, patch)
2012-03-22 05:52 PDT, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
Patch (49.81 KB, patch)
2012-03-22 06:12 PDT, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
Patch for landing (50.27 KB, patch)
2012-03-22 06:51 PDT, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
Patch for landing (50.29 KB, patch)
2012-03-22 07:14 PDT, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
Patch for landing (50.40 KB, patch)
2012-03-22 07:39 PDT, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
Patch for landing (51.01 KB, patch)
2012-03-22 08:43 PDT, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
Patch for landing (50.96 KB, patch)
2012-03-22 10:10 PDT, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexis Menard (darktears) 2012-03-19 16:21:28 PDT
Remove the map of CSSPropertyLonghand.
Comment 1 Alexis Menard (darktears) 2012-03-19 16:23:46 PDT
Created attachment 132707 [details]
Patch
Comment 2 Alexis Menard (darktears) 2012-03-19 16:33:11 PDT
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
Comment 3 Luke Macpherson 2012-03-20 17:01:23 PDT
See also https://bugs.webkit.org/show_bug.cgi?id=81611
Comment 4 Tony Chang 2012-03-21 14:17:48 PDT
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?
Comment 5 Tony Chang 2012-03-21 14:19:56 PDT
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?
Comment 6 Alexis Menard (darktears) 2012-03-22 05:52:38 PDT
Created attachment 133235 [details]
Patch
Comment 7 Alexis Menard (darktears) 2012-03-22 06:12:26 PDT
Created attachment 133243 [details]
Patch
Comment 8 Build Bot 2012-03-22 06:34:19 PDT
Comment on attachment 133243 [details]
Patch

Attachment 133243 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12070991
Comment 9 Antti Koivisto 2012-03-22 06:39:01 PDT
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&
Comment 10 Alexis Menard (darktears) 2012-03-22 06:51:45 PDT
Created attachment 133247 [details]
Patch for landing
Comment 11 Build Bot 2012-03-22 07:08:26 PDT
Comment on attachment 133247 [details]
Patch for landing

Attachment 133247 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12066959
Comment 12 Alexis Menard (darktears) 2012-03-22 07:14:38 PDT
Created attachment 133253 [details]
Patch for landing
Comment 13 Build Bot 2012-03-22 07:28:26 PDT
Comment on attachment 133253 [details]
Patch for landing

Attachment 133253 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12067018
Comment 14 Alexis Menard (darktears) 2012-03-22 07:39:50 PDT
Created attachment 133261 [details]
Patch for landing
Comment 15 Build Bot 2012-03-22 08:21:37 PDT
Comment on attachment 133261 [details]
Patch for landing

Attachment 133261 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12114041
Comment 16 WebKit Review Bot 2012-03-22 08:26:01 PDT
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
Comment 17 Alexis Menard (darktears) 2012-03-22 08:43:28 PDT
Created attachment 133273 [details]
Patch for landing
Comment 18 Alexis Menard (darktears) 2012-03-22 08:44:03 PDT
(In reply to comment #17)
> Created an attachment (id=133273) [details]
> Patch for landing

Should fix the warnings about integer vs unsigned comparison.
Comment 19 Alexis Menard (darktears) 2012-03-22 10:10:15 PDT
Created attachment 133287 [details]
Patch for landing
Comment 20 WebKit Review Bot 2012-03-22 10:55:08 PDT
Comment on attachment 133287 [details]
Patch for landing

Clearing flags on attachment: 133287

Committed r111731: <http://trac.webkit.org/changeset/111731>
Comment 21 WebKit Review Bot 2012-03-22 10:55:14 PDT
All reviewed patches have been landed.  Closing bug.