Bug 83426

Summary: Don't expose internal CSSValues in API
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, kling, macpherson, menard, rniwa, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 77745, 83540    
Attachments:
Description Flags
patch
none
rebased
webkit-ews: commit-queue-
try to fix build
kling: review+, buildbot: commit-queue-
try to fix win build none

Description Antti Koivisto 2012-04-07 11:27:06 PDT
The CSSValues returned from functions like CSSStyleDeclaration.getPropertyCSSValue() are currently the same instances we use internally. This creates various problems. The values can't be shared between documents as the wrappers would be shared too. Having to maintain per-document CSSValuePools complicate the architecture and increase memory usage. This also prevents sharing style sheet data structures between documents.
Comment 1 Antti Koivisto 2012-04-07 11:45:23 PDT
Created attachment 136135 [details]
patch
Comment 2 Antti Koivisto 2012-04-07 11:57:50 PDT
Created attachment 136136 [details]
rebased
Comment 3 Early Warning System Bot 2012-04-07 12:11:09 PDT
Comment on attachment 136136 [details]
rebased

Attachment 136136 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12362468
Comment 4 Early Warning System Bot 2012-04-07 12:13:28 PDT
Comment on attachment 136136 [details]
rebased

Attachment 136136 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12364444
Comment 5 Build Bot 2012-04-07 12:35:11 PDT
Comment on attachment 136136 [details]
rebased

Attachment 136136 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12358494
Comment 6 Antti Koivisto 2012-04-07 12:41:58 PDT
Created attachment 136137 [details]
try to fix build
Comment 7 Build Bot 2012-04-07 13:06:21 PDT
Comment on attachment 136137 [details]
try to fix build

Attachment 136137 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12367042
Comment 8 Andreas Kling 2012-04-07 14:13:10 PDT
Comment on attachment 136137 [details]
try to fix build

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

Very cool. r=me.
(Though you'll need to do something about the win failure.)

> Source/WebCore/css/CSSImageSetValue.cpp:146
> +    // Non-CSSValueList data is not accesible through CSS OM, no need to clone.

Typo accessible.

> Source/WebCore/css/CSSValue.h:38
> +// Please don't expose more CSSValue types to the web.

Hear hear. Perhaps we could even get rid of WebKitCSSTransformValue.idl and WebKitCSSFilterValue.idl.

> Source/WebCore/css/RGBColor.cpp:42
> +    // Unique instances are safe.

Don't think this comment is needed.
Comment 9 Antti Koivisto 2012-04-07 14:28:49 PDT
Created attachment 136140 [details]
try to fix win build
Comment 10 WebKit Review Bot 2012-04-09 03:03:50 PDT
Comment on attachment 136140 [details]
try to fix win build

Rejecting attachment 136140 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

ERROR: /mnt/git/webkit-commit-queue/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/12372107
Comment 11 WebKit Review Bot 2012-04-09 10:01:26 PDT
Comment on attachment 136140 [details]
try to fix win build

Clearing flags on attachment: 136140

Committed r113588: <http://trac.webkit.org/changeset/113588>
Comment 12 WebKit Review Bot 2012-04-09 10:01:31 PDT
All reviewed patches have been landed.  Closing bug.