Bug 32325 - Filters contain some leaks in untested code
Summary: Filters contain some leaks in untested code
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh Intel OS X 10.5
: P2 Normal
Assignee: Nikolas Zimmermann
Depends on:
Reported: 2009-12-09 09:45 PST by Nikolas Zimmermann
Modified: 2009-12-29 02:56 PST (History)
5 users (show)

See Also:

Initial patch (20.82 KB, patch)
2009-12-09 17:14 PST, Nikolas Zimmermann
no flags Details | Formatted Diff | Diff
Updated patch (20.84 KB, patch)
2009-12-09 17:28 PST, Nikolas Zimmermann
oliver: review+
oliver: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.