Bug 32325

Summary: Filters contain some leaks in untested code
Product: WebKit Reporter: Nikolas Zimmermann <zimmermann>
Component: SVGAssignee: Nikolas Zimmermann <zimmermann>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, krit, mjs, staikos, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac (Intel)   
OS: OS X 10.5   
Attachments:
Description Flags
Initial patch
none
Updated patch oliver: review+, oliver: commit-queue-

Description Nikolas Zimmermann 2009-12-09 09:45:56 PST 
We currently don't have support for the lighting filters, though we already build filter effect objects for those.
DistantLightSource & PointLightSource are RefCounted objects w/o create() objects, leading to referennce counting bugs.

Just testing a patch that fixes those. Uploading later.
FYI: Besides that,  filters are up & running on win/mac/gtk/qt!
Comment 1 Nikolas Zimmermann 2009-12-09 17:14:50 PST 
Created attachment 44584 [details]
Initial patch
Comment 2 WebKit Review Bot 2009-12-09 17:18:25 PST 
Attachment 44584 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/svg/graphics/filters/SVGLightSource.h:28:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1
Comment 3 Nikolas Zimmermann 2009-12-09 17:28:31 PST 
Created attachment 44585 [details]
Updated patch

Picky Mrs. Stylebot...
Comment 4 WebKit Review Bot 2009-12-09 17:28:51 PST 
style-queue ran check-webkit-style on attachment 44585 [details] without any errors.
Comment 5 Oliver Hunt 2009-12-09 19:05:49 PST 
Comment on attachment 44585 [details]
Updated patch

Land your own patches :P :D

Looks good, it concerns me that we seem to repeatedly create new LightSource objects but that's a distinct issue
Comment 6 Nikolas Zimmermann 2009-12-10 04:00:44 PST 
Thanks Oliver. The patch only aims to fix the leaks - once the lighting filters will be implemented, these issues will be resolved.
Comment 7 Eric Seidel (no email) 2009-12-28 23:22:07 PST 
Ping?  Looks like this has been reviewed for a little over 2 weeks.  I assume the holiday season has just gotten in the way of landing?
Comment 8 Nikolas Zimmermann 2009-12-29 02:56:49 PST 
Oops, this has already been landed, a while ago. Forgot to close.