Bug 41347 - Cleanup SVGRenderSupport
Summary: Cleanup SVGRenderSupport
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nikolas Zimmermann
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-29 06:19 PDT by Nikolas Zimmermann
Modified: 2010-06-29 07:27 PDT (History)
1 user (show)

See Also:

Attachments
Initial patch (68.79 KB, patch)
2010-06-29 07:05 PDT, Nikolas Zimmermann 		krit: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Nikolas Zimmermann 2010-06-29 06:19:13 PDT 
SVGRenderSupport contains the SVGRenderBase class, which was meant to be a base class for all SVG renders until all inherit from RenderSVGModelObject, introduced by Eric a year ago.
RenderSVGModelObject inherits from RenderObject. That means classes like RenderSVGImage, which inherits from RenderImage, can NOT inherit from RenderSVGModelObject, as RenderImage already inherits from RenderObject.
The whole class is uncessary and confusing. I'll rename it to SVGRenderSupport, just like the file name, and make all methods static and make it impossible to construct the class.
This shrinks the size of all SVG renderers.
Comment 1 Nikolas Zimmermann 2010-06-29 07:05:23 PDT 
Created attachment 60019 [details]
Initial patch
Comment 2 WebKit Review Bot 2010-06-29 07:07:42 PDT 
Attachment 60019 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/rendering/RenderSVGResourcePattern.cpp:28:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 32 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Dirk Schulze 2010-06-29 07:17:48 PDT 
Comment on attachment 60019 [details]
Initial patch

LGTM. Fix the alphabetic order and you can land it. r=me.
Comment 4 Nikolas Zimmermann 2010-06-29 07:26:28 PDT 
Landed in r62117.
Comment 5 Nikolas Zimmermann 2010-06-29 07:27:34 PDT 
Oops, that should be: 62118.