Bug 86574 – Avoid reparsing the style attribute when cloning elements.

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Andreas Kling]

We should avoid unnecessarily reparsing the 'style' attribute when cloning DOM elements. This would help the DOM/CloneNodes performance test, and makes sense in general.

[Andreas Kling]

Created attachment 142147 [details]
Patch

Let's see how EWS feels about this..

[Early Warning System Bot]

Comment on attachment 142147 [details]
Patch

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

[Build Bot]

Comment on attachment 142147 [details]
Patch

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

[Early Warning System Bot]

Comment on attachment 142147 [details]
Patch

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

[Andreas Kling]

Created attachment 142156 [details]
Patch

[Andreas Kling]

Created attachment 142165 [details]
Patch

[Gustavo Noronha (kov)]

Comment on attachment 142165 [details]
Patch

Attachment 142165 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12720058

[Build Bot]

Comment on attachment 142165 [details]
Patch

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

[Build Bot]

Comment on attachment 142165 [details]
Patch

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

[Early Warning System Bot]

Comment on attachment 142165 [details]
Patch

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

[Andreas Kling]

Created attachment 142168 [details]
Patch

From the making-this-harder-than-it-needs-to-be department..

[Antti Koivisto]

Comment on attachment 142168 [details]
Patch

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

r=me

> Source/WebCore/dom/Element.cpp:2109
> +    // Transfer additional data if the we are cloning from another element of the same type.
> +    if (tagQName() == other.tagQName())
> +        copyNonAttributePropertiesFromElement(other);

Would it be better to have a different function for cloning to an element with a different tag name? That seems like a quite different case (why do we do it?) from regular cloning.

> Source/WebCore/dom/StyledElement.h:36
> +enum ShouldReparseStyleAttribute { DoNotReparseStyleAttribute = 0, ReparseStyleAttribute = 1 };

I would scope this to StyledElement.

[Andreas Kling]

Committed r117323: <http://trac.webkit.org/changeset/117323>

[Ryosuke Niwa]

This improved our score on DOM/CreateNodes by roughly 50% on Chromium port:
http://webkit-perf.appspot.com/graph.html#tests=[[3116,2001,173262],[3116,2001,2389050],[3116,2001,3001]]&sel=1337201364787.342,1337244049479.191&displayrange=7&datatype=running