2009-12-09 Nikolas Zimmermann <nzimmermann@rim.com>
authorzimmermann@webkit.org <zimmermann@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 10 Dec 2009 12:05:33 +0000 (12:05 +0000)
committerzimmermann@webkit.org <zimmermann@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 10 Dec 2009 12:05:33 +0000 (12:05 +0000)
        Reviewed by Oliver Hunt.

        Filters contain some leaks in untested code
        https://bugs.webkit.org/show_bug.cgi?id=32325

        Fix obvious leak in SVGFE*Lighting classes. Implement the create() idiom for
        all classes in svg/graphics, that were missing it. The lighting filters aren't
        implemented so far, but the associated FilterEffect objects are build, which created
        these leaks.

        This removes the SVG related failures in the leaks bot.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@51945 268f45cc-cd09-0410-ab3c-d52691b4dbfc

20 files changed:
WebCore/ChangeLog
WebCore/svg/SVGFEDiffuseLightingElement.cpp
WebCore/svg/SVGFEDiffuseLightingElement.h
WebCore/svg/SVGFEDistantLightElement.cpp
WebCore/svg/SVGFEDistantLightElement.h
WebCore/svg/SVGFELightElement.h
WebCore/svg/SVGFEPointLightElement.cpp
WebCore/svg/SVGFEPointLightElement.h
WebCore/svg/SVGFESpecularLightingElement.cpp
WebCore/svg/SVGFESpecularLightingElement.h
WebCore/svg/SVGFESpotLightElement.cpp
WebCore/svg/SVGFESpotLightElement.h
WebCore/svg/graphics/filters/SVGDistantLightSource.h
WebCore/svg/graphics/filters/SVGFEDiffuseLighting.cpp
WebCore/svg/graphics/filters/SVGFEDiffuseLighting.h
WebCore/svg/graphics/filters/SVGFESpecularLighting.cpp
WebCore/svg/graphics/filters/SVGFESpecularLighting.h
WebCore/svg/graphics/filters/SVGLightSource.h
WebCore/svg/graphics/filters/SVGPointLightSource.h
WebCore/svg/graphics/filters/SVGSpotLightSource.h

index d82d26a..3bd01c9 100644 (file)
@@ -1,3 +1,54 @@
+2009-12-09  Nikolas Zimmermann  <nzimmermann@rim.com>
+
+        Reviewed by Oliver Hunt.
+
+        Filters contain some leaks in untested code
+        https://bugs.webkit.org/show_bug.cgi?id=32325
+
+        Fix obvious leak in SVGFE*Lighting classes. Implement the create() idiom for 
+        all classes in svg/graphics, that were missing it. The lighting filters aren't
+        implemented so far, but the associated FilterEffect objects are build, which created
+        these leaks.
+
+        This removes the SVG related failures in the leaks bot.
+
+        * svg/SVGFEDiffuseLightingElement.cpp:
+        (WebCore::SVGFEDiffuseLightingElement::findLights):
+        * svg/SVGFEDiffuseLightingElement.h:
+        * svg/SVGFEDistantLightElement.cpp:
+        (WebCore::SVGFEDistantLightElement::lightSource):
+        * svg/SVGFEDistantLightElement.h:
+        * svg/SVGFELightElement.h:
+        * svg/SVGFEPointLightElement.cpp:
+        (WebCore::SVGFEPointLightElement::lightSource):
+        * svg/SVGFEPointLightElement.h:
+        * svg/SVGFESpecularLightingElement.cpp:
+        (WebCore::SVGFESpecularLightingElement::findLights):
+        * svg/SVGFESpecularLightingElement.h:
+        * svg/SVGFESpotLightElement.cpp:
+        (WebCore::SVGFESpotLightElement::lightSource):
+        * svg/SVGFESpotLightElement.h:
+        * svg/graphics/filters/SVGDistantLightSource.h:
+        (WebCore::DistantLightSource::create):
+        (WebCore::DistantLightSource::DistantLightSource):
+        * svg/graphics/filters/SVGFEDiffuseLighting.cpp:
+        (WebCore::FEDiffuseLighting::FEDiffuseLighting):
+        (WebCore::FEDiffuseLighting::create):
+        (WebCore::FEDiffuseLighting::setLightSource):
+        * svg/graphics/filters/SVGFEDiffuseLighting.h:
+        * svg/graphics/filters/SVGFESpecularLighting.cpp:
+        (WebCore::FESpecularLighting::FESpecularLighting):
+        (WebCore::FESpecularLighting::create):
+        (WebCore::FESpecularLighting::setLightSource):
+        * svg/graphics/filters/SVGFESpecularLighting.h:
+        * svg/graphics/filters/SVGLightSource.h:
+        * svg/graphics/filters/SVGPointLightSource.h:
+        (WebCore::PointLightSource::create):
+        (WebCore::PointLightSource::PointLightSource):
+        * svg/graphics/filters/SVGSpotLightSource.h:
+        (WebCore::SpotLightSource::create):
+        (WebCore::SpotLightSource::SpotLightSource):
+
 2009-12-10  Kenneth Russell  <kbr@google.com>
 
         Reviewed by Oliver Hunt.
index edbd852..ed6e353 100644 (file)
@@ -88,20 +88,18 @@ bool SVGFEDiffuseLightingElement::build(SVGResourceFilter* filterResource)
     return true;
 }
 
-LightSource* SVGFEDiffuseLightingElement::findLights() const
+PassRefPtr<LightSource> SVGFEDiffuseLightingElement::findLights() const
 {
-    LightSource* light = 0;
     for (Node* n = firstChild(); n; n = n->nextSibling()) {
         if (n->hasTagName(SVGNames::feDistantLightTag) ||
             n->hasTagName(SVGNames::fePointLightTag) ||
             n->hasTagName(SVGNames::feSpotLightTag)) {
             SVGFELightElement* lightNode = static_cast<SVGFELightElement*>(n); 
-            light = lightNode->lightSource();
-            break;
+            return lightNode->lightSource();
         }
     }
 
-    return light;
+    return 0;
 }
 
 }
index fcb5eee..ed117a4 100644 (file)
@@ -49,7 +49,7 @@ namespace WebCore {
         ANIMATED_PROPERTY_DECLARATIONS(SVGFEDiffuseLightingElement, SVGNames::feDiffuseLightingTagString, SVGKernelUnitLengthXIdentifier, float, KernelUnitLengthX, kernelUnitLengthX)
         ANIMATED_PROPERTY_DECLARATIONS(SVGFEDiffuseLightingElement, SVGNames::feDiffuseLightingTagString, SVGKernelUnitLengthYIdentifier, float, KernelUnitLengthY, kernelUnitLengthY)
 
-        LightSource* findLights() const;
+        PassRefPtr<LightSource> findLights() const;
     };
 
 } // namespace WebCore
index 35c487b..8322a3a 100644 (file)
@@ -34,9 +34,9 @@ SVGFEDistantLightElement::~SVGFEDistantLightElement()
 {
 }
 
-LightSource* SVGFEDistantLightElement::lightSource() const
+PassRefPtr<LightSource> SVGFEDistantLightElement::lightSource() const
 {
-    return new DistantLightSource(azimuth(), elevation());
+    return DistantLightSource::create(azimuth(), elevation());
 }
 
 }
index 95f45c8..3d0c039 100644 (file)
@@ -29,7 +29,7 @@ namespace WebCore {
         SVGFEDistantLightElement(const QualifiedName&, Document*);
         virtual ~SVGFEDistantLightElement();
 
-        virtual LightSource* lightSource() const;
+        virtual PassRefPtr<LightSource> lightSource() const;
     };
 
 } // namespace WebCore
index 92947a1..705eeaa 100644 (file)
@@ -36,7 +36,7 @@ namespace WebCore {
         SVGFELightElement(const QualifiedName&, Document*);
         virtual ~SVGFELightElement();
         
-        virtual LightSource* lightSource() const = 0;
+        virtual PassRefPtr<LightSource> lightSource() const = 0;
         virtual void parseMappedAttribute(MappedAttribute*);
 
     private:
index 088f878..36ed2e9 100644 (file)
@@ -34,10 +34,10 @@ SVGFEPointLightElement::~SVGFEPointLightElement()
 {
 }
 
-LightSource* SVGFEPointLightElement::lightSource() const
+PassRefPtr<LightSource> SVGFEPointLightElement::lightSource() const
 {
     FloatPoint3D pos(x(), y(), z());
-    return new PointLightSource(pos);
+    return PointLightSource::create(pos);
 }
 
 }
index b39fffa..9b1f997 100644 (file)
@@ -29,7 +29,7 @@ namespace WebCore {
         SVGFEPointLightElement(const QualifiedName&, Document*);
         virtual ~SVGFEPointLightElement();
 
-        virtual LightSource* lightSource() const;
+        virtual PassRefPtr<LightSource> lightSource() const;
     };
 
 } // namespace WebCore
index 7afa4cc..90e9cb3 100644 (file)
@@ -70,20 +70,18 @@ void SVGFESpecularLightingElement::parseMappedAttribute(MappedAttribute* attr)
         SVGFilterPrimitiveStandardAttributes::parseMappedAttribute(attr);
 }
 
-LightSource* SVGFESpecularLightingElement::findLights() const
+PassRefPtr<LightSource> SVGFESpecularLightingElement::findLights() const
 {
-    LightSource* light = 0;
     for (Node* n = firstChild(); n; n = n->nextSibling()) {
         if (n->hasTagName(SVGNames::feDistantLightTag) ||
             n->hasTagName(SVGNames::fePointLightTag) ||
             n->hasTagName(SVGNames::feSpotLightTag)) {
             SVGFELightElement* lightNode = static_cast<SVGFELightElement*>(n); 
-            light = lightNode->lightSource();
-            break;
+            return lightNode->lightSource();
         }
     }
 
-    return light;
+    return 0;
 }
 
 bool SVGFESpecularLightingElement::build(SVGResourceFilter* filterResource)
index 8ef490a..b3771fe 100644 (file)
@@ -48,8 +48,8 @@ namespace WebCore {
         ANIMATED_PROPERTY_DECLARATIONS(SVGFESpecularLightingElement, SVGNames::feSpecularLightingTagString, SVGNames::surfaceScaleAttrString, float, SurfaceScale, surfaceScale)
         ANIMATED_PROPERTY_DECLARATIONS(SVGFESpecularLightingElement, SVGNames::feSpecularLightingTagString, SVGKernelUnitLengthXIdentifier, float, KernelUnitLengthX, kernelUnitLengthX)
         ANIMATED_PROPERTY_DECLARATIONS(SVGFESpecularLightingElement, SVGNames::feSpecularLightingTagString, SVGKernelUnitLengthYIdentifier, float, KernelUnitLengthY, kernelUnitLengthY)
-        
-        LightSource* findLights() const;
+
+        PassRefPtr<LightSource> findLights() const;
     };
 
 } // namespace WebCore
index 980a3bb..01fe54e 100644 (file)
@@ -34,7 +34,7 @@ SVGFESpotLightElement::~SVGFESpotLightElement()
 {
 }
 
-LightSource* SVGFESpotLightElement::lightSource() const
+PassRefPtr<LightSource> SVGFESpotLightElement::lightSource() const
 {
     FloatPoint3D pos(x(), y(), z());
 
@@ -44,7 +44,7 @@ LightSource* SVGFESpotLightElement::lightSource() const
                            pointsAtZ() - pos.z());
 
     direction.normalize();
-    return new SpotLightSource(pos, direction, specularExponent(), limitingConeAngle());
+    return SpotLightSource::create(pos, direction, specularExponent(), limitingConeAngle());
 }
 
 }
index 440c664..d54e232 100644 (file)
@@ -29,7 +29,7 @@ namespace WebCore {
         SVGFESpotLightElement(const QualifiedName&, Document*);
         virtual ~SVGFESpotLightElement();
 
-        virtual LightSource* lightSource() const;
+        virtual PassRefPtr<LightSource> lightSource() const;
     };
 
 } // namespace WebCore
index 07ac16c..db9b59d 100644 (file)
@@ -30,11 +30,10 @@ namespace WebCore {
 
     class DistantLightSource : public LightSource {
     public:
-        DistantLightSource(float azimuth, float elevation)
-            : LightSource(LS_DISTANT)
-            , m_azimuth(azimuth)
-            , m_elevation(elevation)
-        { }
+        static PassRefPtr<DistantLightSource> create(float azimuth, float elevation)
+        {
+            return adoptRef(new DistantLightSource(azimuth, elevation));
+        }
 
         float azimuth() const { return m_azimuth; }
         float elevation() const { return m_elevation; }
@@ -42,6 +41,13 @@ namespace WebCore {
         virtual TextStream& externalRepresentation(TextStream&) const;
 
     private:
+        DistantLightSource(float azimuth, float elevation)
+            : LightSource(LS_DISTANT)
+            , m_azimuth(azimuth)
+            , m_elevation(elevation)
+        {
+        }
+
         float m_azimuth;
         float m_elevation;
     };
index c536478..93921df 100644 (file)
@@ -30,7 +30,7 @@
 namespace WebCore {
 
 FEDiffuseLighting::FEDiffuseLighting(FilterEffect* in , const Color& lightingColor, const float& surfaceScale,
-    const float& diffuseConstant, const float& kernelUnitLengthX, const float& kernelUnitLengthY, LightSource* lightSource)
+    const float& diffuseConstant, const float& kernelUnitLengthX, const float& kernelUnitLengthY, PassRefPtr<LightSource> lightSource)
     : FilterEffect()
     , m_in(in)
     , m_lightingColor(lightingColor)
@@ -44,7 +44,7 @@ FEDiffuseLighting::FEDiffuseLighting(FilterEffect* in , const Color& lightingCol
 
 PassRefPtr<FEDiffuseLighting> FEDiffuseLighting::create(FilterEffect* in , const Color& lightingColor,
     const float& surfaceScale, const float& diffuseConstant, const float& kernelUnitLengthX,
-    const float& kernelUnitLengthY, LightSource* lightSource)
+    const float& kernelUnitLengthY, PassRefPtr<LightSource> lightSource)
 {
     return adoptRef(new FEDiffuseLighting(in, lightingColor, surfaceScale, diffuseConstant, kernelUnitLengthX, kernelUnitLengthY, lightSource));
 }
@@ -108,7 +108,7 @@ const LightSource* FEDiffuseLighting::lightSource() const
     return m_lightSource.get();
 }
 
-void FEDiffuseLighting::setLightSource(LightSource* lightSource)
+void FEDiffuseLighting::setLightSource(PassRefPtr<LightSource> lightSource)
 {    
     m_lightSource = lightSource;
 }
index 71f8e22..bf7f28e 100644 (file)
@@ -34,7 +34,7 @@ namespace WebCore {
     class FEDiffuseLighting : public FilterEffect {
     public:
         static PassRefPtr<FEDiffuseLighting> create(FilterEffect*, const Color&, const float&, const float&,
-                const float&, const float&, LightSource*);
+                const float&, const float&, PassRefPtr<LightSource>);
         virtual ~FEDiffuseLighting();
 
         Color lightingColor() const;
@@ -53,7 +53,7 @@ namespace WebCore {
         void setKernelUnitLengthY(float);
 
         const LightSource* lightSource() const;
-        void setLightSource(LightSource*);
+        void setLightSource(PassRefPtr<LightSource>);
 
         virtual FloatRect uniteChildEffectSubregions(Filter* filter) { return calculateUnionOfChildEffectSubregions(filter, m_in.get()); }
         void apply(Filter*);
@@ -62,7 +62,7 @@ namespace WebCore {
         
     private:
         FEDiffuseLighting(FilterEffect*, const Color&, const float&, const float&,
-                const float&, const float&, LightSource*);
+                const float&, const float&, PassRefPtr<LightSource>);
 
         RefPtr<FilterEffect> m_in;
         Color m_lightingColor;
index eb0c280..0b43aba 100644 (file)
@@ -30,7 +30,7 @@ namespace WebCore {
 
 FESpecularLighting::FESpecularLighting(FilterEffect* in, const Color& lightingColor, const float& surfaceScale,
     const float& specularConstant, const float& specularExponent, const float& kernelUnitLengthX,
-    const float& kernelUnitLengthY, LightSource* lightSource)
+    const float& kernelUnitLengthY, PassRefPtr<LightSource> lightSource)
     : FilterEffect()
     , m_in(in)
     , m_lightingColor(lightingColor)
@@ -45,7 +45,7 @@ FESpecularLighting::FESpecularLighting(FilterEffect* in, const Color& lightingCo
 
 PassRefPtr<FESpecularLighting> FESpecularLighting::create(FilterEffect* in, const Color& lightingColor,
     const float& surfaceScale, const float& specularConstant, const float& specularExponent,
-    const float& kernelUnitLengthX, const float& kernelUnitLengthY, LightSource* lightSource)
+    const float& kernelUnitLengthX, const float& kernelUnitLengthY, PassRefPtr<LightSource> lightSource)
 {
     return adoptRef(new FESpecularLighting(in, lightingColor, surfaceScale, specularConstant, specularExponent,
         kernelUnitLengthX, kernelUnitLengthY, lightSource));
@@ -120,7 +120,7 @@ const LightSource* FESpecularLighting::lightSource() const
     return m_lightSource.get();
 }
 
-void FESpecularLighting::setLightSource(LightSource* lightSource)
+void FESpecularLighting::setLightSource(PassRefPtr<LightSource> lightSource)
 {
     m_lightSource = lightSource;
 }
index dec5163..f4947fd 100644 (file)
@@ -33,7 +33,7 @@ namespace WebCore {
     class FESpecularLighting : public FilterEffect {
     public:
         static PassRefPtr<FESpecularLighting> create(FilterEffect*, const Color&, const float&, const float&,
-            const float&, const float&, const float&, LightSource*);
+            const float&, const float&, const float&, PassRefPtr<LightSource>);
         virtual ~FESpecularLighting();
 
         Color lightingColor() const;
@@ -55,7 +55,7 @@ namespace WebCore {
         void setKernelUnitLengthY(float);
 
         const LightSource* lightSource() const;
-        void setLightSource(LightSource*);
+        void setLightSource(PassRefPtr<LightSource>);
 
         virtual FloatRect uniteEffectRect(Filter* filter) { return calculateUnionOfChildEffectSubregions(filter, m_in.get()); }
         void apply(Filter*);
@@ -64,7 +64,7 @@ namespace WebCore {
 
     private:
         FESpecularLighting(FilterEffect*, const Color&, const float&, const float&, const float&,
-            const float&, const float&, LightSource*);
+            const float&, const float&, PassRefPtr<LightSource>);
 
         RefPtr<FilterEffect> m_in;
         Color m_lightingColor;
index 22b43c8..6f0075c 100644 (file)
@@ -24,6 +24,7 @@
 #define SVGLightSource_h
 
 #if ENABLE(SVG) && ENABLE(FILTERS)
+#include <wtf/PassRefPtr.h>
 #include <wtf/RefCounted.h>
 
 namespace WebCore {
index 772e278..1e966cc 100644 (file)
@@ -31,16 +31,22 @@ namespace WebCore {
 
     class PointLightSource : public LightSource {
     public:
-        PointLightSource(const FloatPoint3D& position)
-            : LightSource(LS_POINT)
-            , m_position(position)
-        }
+        static PassRefPtr<PointLightSource> create(const FloatPoint3D& position)
+        {
+            return adoptRef(new PointLightSource(position));
+        }
 
         const FloatPoint3D& position() const { return m_position; }
 
         virtual TextStream& externalRepresentation(TextStream&) const;
 
     private:
+        PointLightSource(const FloatPoint3D& position)
+            : LightSource(LS_POINT)
+            , m_position(position)
+        {
+        }
+
         FloatPoint3D m_position;
     };
 
index 9a787fb..05280d2 100644 (file)
@@ -31,13 +31,11 @@ namespace WebCore {
 
     class SpotLightSource : public LightSource {
     public:
-        SpotLightSource(const FloatPoint3D& position, const FloatPoint3D& direction, float specularExponent, float limitingConeAngle)
-            : LightSource(LS_SPOT)
-            , m_position(position)
-            , m_direction(direction)
-            , m_specularExponent(specularExponent)
-            , m_limitingConeAngle(limitingConeAngle)
-        { }
+        static PassRefPtr<SpotLightSource> create(const FloatPoint3D& position, const FloatPoint3D& direction,
+                                                  float specularExponent, float limitingConeAngle)
+        {
+            return adoptRef(new SpotLightSource(position, direction, specularExponent, limitingConeAngle));
+        }
 
         const FloatPoint3D& position() const { return m_position; }
         const FloatPoint3D& direction() const { return m_direction; }
@@ -48,6 +46,16 @@ namespace WebCore {
         virtual TextStream& externalRepresentation(TextStream&) const;
 
     private:
+        SpotLightSource(const FloatPoint3D& position, const FloatPoint3D& direction,
+                        float specularExponent, float limitingConeAngle)
+            : LightSource(LS_SPOT)
+            , m_position(position)
+            , m_direction(direction)
+            , m_specularExponent(specularExponent)
+            , m_limitingConeAngle(limitingConeAngle)
+        {
+        }
+
         FloatPoint3D m_position;
         FloatPoint3D m_direction;