Bug 125762

Summary:

CTTE: Convert more of SVG to use references

Product:

WebKit

Reporter:

Sam Weinig <sam>

Component:

New Bugs

Assignee:

Sam Weinig <sam>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, dino, d-r, esprehn+autocc, fmalita, glenn, gyuyoung.kim, kondapallykalyan, pdr, schenney

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none

Sam Weinig

Reported 2013-12-15 19:19:10 PST

CTTE: Convert more of SVG to use references

Attachments
Patch (51.61 KB, patch) 2013-12-15 19:19 PST, Sam Weinig	no flags	Details Formatted Diff Diff
Patch (51.50 KB, patch) 2013-12-15 20:38 PST, Sam Weinig	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Sam Weinig

Comment 1 2013-12-15 19:19:42 PST

Created attachment 219289 [details] Patch

Darin Adler

Comment 2 2013-12-15 20:28:42 PST

Comment on attachment 219289 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=219289&action=review > Source/WebCore/rendering/svg/RenderSVGResource.cpp:179 > + if (RenderElement* renderer = element->renderer()) Could use auto instead of specifying RenderElement. Other places in the patch, too.

Sam Weinig

Comment 3 2013-12-15 20:38:20 PST

Created attachment 219292 [details] Patch

Darin Adler

Comment 4 2013-12-16 09:37:41 PST

Comment on attachment 219292 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=219292&action=review Looks great. I spotted 6 “other places” you didn’t change. > Source/WebCore/svg/SVGAnimateMotionElement.cpp:292 > + if (RenderElement* renderer = targetElement->renderer()) One of the “other places”. > Source/WebCore/svg/SVGAnimateMotionElement.cpp:307 > + if (RenderElement* renderer = shadowTreeElement->renderer()) { Another of the “other places”. > Source/WebCore/svg/SVGFEImageElement.cpp:202 > + RenderElement* parentRenderer = parent->renderer(); Another of the “other places”. > Source/WebCore/svg/SVGFELightElement.cpp:217 > + RenderElement* renderer = parent->renderer(); Another of the “other places”. > Source/WebCore/svg/SVGFilterPrimitiveStandardAttributes.cpp:168 > + RenderElement* renderer = parent->renderer(); Another of the “other places”. > Source/WebCore/svg/SVGSVGElement.cpp:726 > + if (RenderElement* renderer = svg->renderer()) Another of the “other places”.

Sam Weinig

Comment 5 2013-12-16 10:53:08 PST

(In reply to comment #4) > (From update of attachment 219292 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=219292&action=review > > Looks great. I spotted 6 “other places” you didn’t change. > > > Source/WebCore/svg/SVGAnimateMotionElement.cpp:292 > > + if (RenderElement* renderer = targetElement->renderer()) > > One of the “other places”. > > > Source/WebCore/svg/SVGAnimateMotionElement.cpp:307 > > + if (RenderElement* renderer = shadowTreeElement->renderer()) { > > Another of the “other places”. > > > Source/WebCore/svg/SVGFEImageElement.cpp:202 > > + RenderElement* parentRenderer = parent->renderer(); > > Another of the “other places”. > > > Source/WebCore/svg/SVGFELightElement.cpp:217 > > + RenderElement* renderer = parent->renderer(); > > Another of the “other places”. > > > Source/WebCore/svg/SVGFilterPrimitiveStandardAttributes.cpp:168 > > + RenderElement* renderer = parent->renderer(); > > Another of the “other places”. > > > Source/WebCore/svg/SVGSVGElement.cpp:726 > > + if (RenderElement* renderer = svg->renderer()) > > Another of the “other places”. Doh. I'll get them in another pass.

WebKit Commit Bot

Comment 6 2013-12-16 11:20:12 PST

Comment on attachment 219292 [details] Patch Clearing flags on attachment: 219292 Committed r160651: <http://trac.webkit.org/changeset/160651>

WebKit Commit Bot

Comment 7 2013-12-16 11:20:15 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.