Bug 102608

Summary:

Element::parseAttribute() should take name & value as separate arguments.

Product:

WebKit

Reporter:

Andreas Kling <kling>

Component:

DOM

Assignee:

Andreas Kling <kling>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, cmarcelo, dbarton, dglazkov, d-r, eric.carlson, feature-media-reviews, fmalita, haraken, japhet, mifenton, pdr, schenney, tkent, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

102598

Attachments:

Description	Flags
Patch	none

Andreas Kling

Reported 2012-11-17 22:35:43 PST

parseAttribute() currently takes a const Attribute&, so we're forced to create a temporary Attribute at the call site in Element::attributeChanged(). This causes measurable refcount churn on the DOM/CreateNodes performance test (~1% of samples.)

Attachments
Patch (346.59 KB, patch) 2012-11-17 22:38 PST, Andreas Kling	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Andreas Kling

Comment 1 2012-11-17 22:38:36 PST

Created attachment 174841 [details] Patch Let's see how EWS likes this.

WebKit Review Bot

Comment 2 2012-11-17 22:41:23 PST

Attachment 174841 [details] did not pass style-queue: Source/WebCore/svg/SVGLineElement.cpp:99: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGLineElement.cpp:100: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGMarkerElement.cpp:155: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGMarkerElement.cpp:156: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGCursorElement.cpp:92: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGCursorElement.cpp:93: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGMaskElement.cpp:122: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGMaskElement.cpp:123: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGPatternElement.cpp:144: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGPatternElement.cpp:145: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGPatternElement.cpp:146: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGPatternElement.cpp:147: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGImageElement.cpp:130: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGImageElement.cpp:131: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGImageElement.cpp:132: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGSVGElement.cpp:272: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGSVGElement.cpp:273: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGSVGElement.cpp:274: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGSVGElement.cpp:275: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGUseElement.cpp:171: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGUseElement.cpp:172: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGUseElement.cpp:173: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/html/HTMLElement.cpp:239: An else should appear on the same line as the preceding } [whitespace/newline] [4] Source/WebCore/svg/SVGEllipseElement.cpp:100: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGEllipseElement.cpp:101: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGCircleElement.cpp:96: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGCircleElement.cpp:97: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGFilterElement.cpp:153: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGFilterElement.cpp:154: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGRecFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 tElement.cpp:113: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGRectElement.cpp:114: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGTextContentElement.cpp:266: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGForeignObjectElement.cpp:103: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/svg/SVGForeignObjectElement.cpp:104: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 34 in 229 files If any of these errors are false positives, please file a bug against check-webkit-style.

WebKit Review Bot

Comment 3 2012-11-17 23:56:59 PST

Comment on attachment 174841 [details] Patch Attachment 174841 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14861797 New failing tests: inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html

Andreas Kling

Comment 4 2012-11-18 08:26:52 PST

Comment on attachment 174841 [details] Patch Pretty sure that test failure is a flake, so marking r?

Antti Koivisto

Comment 5 2012-11-18 08:28:33 PST

Comment on attachment 174841 [details] Patch r=me

WebKit Review Bot

Comment 6 2012-11-18 08:55:48 PST

Comment on attachment 174841 [details] Patch Clearing flags on attachment: 174841 Committed r135069: <http://trac.webkit.org/changeset/135069>

WebKit Review Bot

Comment 7 2012-11-18 08:55:53 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.