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+

Andreas Kling
Reported 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.
Attachments
Patch (19.66 KB, patch)
2012-05-15 22:19 PDT, Andreas Kling 		webkit-ews: commit-queue-
DetailsFormatted DiffDiff
Patch (19.66 KB, patch)
2012-05-15 22:54 PDT, Andreas Kling 		no flags
DetailsFormatted DiffDiff
Patch (19.71 KB, patch)
2012-05-15 23:41 PDT, Andreas Kling 		gustavo: commit-queue-
DetailsFormatted DiffDiff
Patch (19.80 KB, patch)
2012-05-16 00:11 PDT, Andreas Kling 		koivisto: review+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Andreas Kling
Comment 1 2012-05-15 22:19:37 PDT
Created attachment 142147 [details] Patch Let's see how EWS feels about this..
Early Warning System Bot
Comment 2 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
Build Bot
Comment 3 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
Early Warning System Bot
Comment 4 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
Andreas Kling
Comment 5 2012-05-15 22:54:13 PDT
Created attachment 142156 [details] Patch
Andreas Kling
Comment 6 2012-05-15 23:41:09 PDT
Created attachment 142165 [details] Patch
Gustavo Noronha (kov)
Comment 7 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
Build Bot
Comment 8 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
Build Bot
Comment 9 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
Early Warning System Bot
Comment 10 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
Andreas Kling
Comment 11 2012-05-16 00:11:13 PDT
Created attachment 142168 [details] Patch From the making-this-harder-than-it-needs-to-be department..
Antti Koivisto
Comment 12 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.
Andreas Kling
Comment 13 2012-05-16 12:31:27 PDT
Committed r117323: <http://trac.webkit.org/changeset/117323>
Note You need to log in before you can comment on or make changes to this bug.