Bug 83426 - Don't expose internal CSSValues in API
Summary: Don't expose internal CSSValues in API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 77745 83540
  Show dependency tree / graph
 
Reported: 2012-04-07 11:27 PDT by Antti Koivisto
Modified: 2012-04-11 20:53 PDT (History)
6 users (show)

See Also:

Attachments
patch (35.75 KB, patch)
2012-04-07 11:45 PDT, Antti Koivisto 		no flags Details | Formatted Diff | Diff
rebased (35.67 KB, patch)
2012-04-07 11:57 PDT, Antti Koivisto 		webkit-ews: commit-queue-
Details | Formatted Diff | Diff
try to fix build (35.71 KB, patch)
2012-04-07 12:41 PDT, Antti Koivisto 		kling: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
try to fix win build (36.13 KB, patch)
2012-04-07 14:28 PDT, Antti Koivisto 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.