Bug 83426 – Don't expose internal CSSValues in API

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

[Antti Koivisto]

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.

[Antti Koivisto]

Created attachment 136135 [details]
patch

[Antti Koivisto]

Created attachment 136136 [details]
rebased

[Early Warning System Bot]

Comment on attachment 136136 [details]
rebased

Attachment 136136 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12362468

[Early Warning System Bot]

Comment on attachment 136136 [details]
rebased

Attachment 136136 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12364444

[Build Bot]

Comment on attachment 136136 [details]
rebased

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

[Antti Koivisto]

Created attachment 136137 [details]
try to fix build

[Build Bot]

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

[Andreas Kling]

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.

[Antti Koivisto]

Created attachment 136140 [details]
try to fix win build

[WebKit Review Bot]

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

[WebKit Review Bot]

Comment on attachment 136140 [details]
try to fix win build

Clearing flags on attachment: 136140

Committed r113588: <http://trac.webkit.org/changeset/113588>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Ryosuke Niwa]

It seems like this patch regressed some perf. tests:
http://webkit-perf.appspot.com/graph.html#tests=[[314224,2001,32196]]&sel=1333978732699.9312,1334010760286.138,1137.9310344827586,1896.5517241379312&displayrange=7&datatype=running