Bug 86574

Summary: Avoid reparsing the style attribute when cloning elements.
Product: WebKit Reporter: Andreas Kling <kling>
Component: DOMAssignee: Andreas Kling <kling>
Status: RESOLVED FIXED    
Severity: Normal CC: gustavo, koivisto, philn, rniwa, tkent, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
webkit-ews: commit-queue-
Patch
none
Patch
gustavo: commit-queue-
Patch koivisto: review+

Description Andreas Kling 2012-05-15 21:57:28 PDT 
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.
Comment 1 Andreas Kling 2012-05-15 22:19:37 PDT 
Created attachment 142147 [details]
Patch

Let's see how EWS feels about this..
Comment 2 Early Warning System Bot 2012-05-15 22:39:42 PDT 
Comment on attachment 142147 [details]
Patch

Attachment 142147 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12709313
Comment 3 Build Bot 2012-05-15 22:42:53 PDT 
Comment on attachment 142147 [details]
Patch

Attachment 142147 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12706555
Comment 4 Early Warning System Bot 2012-05-15 22:50:20 PDT 
Comment on attachment 142147 [details]
Patch

Attachment 142147 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12720031
Comment 5 Andreas Kling 2012-05-15 22:54:13 PDT 
Created attachment 142156 [details]
Patch
Comment 6 Andreas Kling 2012-05-15 23:41:09 PDT 
Created attachment 142165 [details]
Patch
Comment 7 Gustavo Noronha (kov) 2012-05-16 00:03:00 PDT 
Comment on attachment 142165 [details]
Patch

Attachment 142165 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12720058
Comment 8 Build Bot 2012-05-16 00:03:01 PDT 
Comment on attachment 142165 [details]
Patch

Attachment 142165 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12721015
Comment 9 Build Bot 2012-05-16 00:05:14 PDT 
Comment on attachment 142165 [details]
Patch

Attachment 142165 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12720055
Comment 10 Early Warning System Bot 2012-05-16 00:06:42 PDT 
Comment on attachment 142165 [details]
Patch

Attachment 142165 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12718078
Comment 11 Andreas Kling 2012-05-16 00:11:13 PDT 
Created attachment 142168 [details]
Patch

From the making-this-harder-than-it-needs-to-be department..
Comment 12 Antti Koivisto 2012-05-16 05:25:23 PDT 
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.
Comment 13 Andreas Kling 2012-05-16 12:31:27 PDT 
Committed r117323: <http://trac.webkit.org/changeset/117323>