ChangeLog

2012-06-11  Xianzhu Wang  <wangxianzhu@chromium.org>

        SVGImageCache leaks image data

        https://bugs.webkit.org/show_bug.cgi?id=87792

        There are two functions to remove a client from a CachedImage:

        - CachedResource::removeClient()

        - CachedImage::removeClientForRenderer().

        It's easy to make error to call the former which will leak the cached

        image buffers in SVGImageCache.

        This change combined the two by adding the virtual 

        CachedResource::didRemoveClient(). CachedImage will do SVGImageCache

        cleanup in the function.

        Reviewed by Nikolas Zimmermann.

        Test: svg/as-image/svg-image-leak-cached-data.html

        * loader/cache/CachedFont.h:

        (WebCore::CachedFontClient::resourceClientType): Added 'const'.

        * loader/cache/CachedImage.cpp:

        (WebCore):

        (WebCore::CachedImage::didRemoveClient): Removes the client from SVGImageCache.

        (WebCore::CachedImage::lookupOrCreateImageForRenderer):

        * loader/cache/CachedImage.h:

        (CachedImage):

        (WebCore::CachedImageClient::resourceClientType): Added 'const'.

        * loader/cache/CachedRawResource.h:

        (WebCore::CachedRawResourceClient::resourceClientType): Added 'const'.

        * loader/cache/CachedResource.cpp:

        (WebCore::CachedResource::removeClient): Added invocation of didRemoveClient().

        * loader/cache/CachedResource.h:

        (WebCore::CachedResource::didRemoveClient): Added for subclasses to do additional works.

        * loader/cache/CachedResourceClient.h:

        (WebCore::CachedResourceClient::resourceClientType): Added 'const'.

        * loader/cache/CachedSVGDocument.h:

        (WebCore::CachedSVGDocumentClient::resourceClientType): Added 'const'.

        * loader/cache/CachedStyleSheetClient.h:

        (WebCore::CachedStyleSheetClient::resourceClientType): Added 'const'.

        * rendering/style/StyleCachedImage.cpp:

        (WebCore::StyleCachedImage::removeClient):

        * rendering/style/StyleCachedImageSet.cpp:

        (WebCore::StyleCachedImageSet::removeClient):

        * svg/graphics/SVGImageCache.cpp:

        (WebCore::SVGImageCache::~SVGImageCache): Added checking for leaks.

        (WebCore::SVGImageCache::removeClientFromCache):

        (WebCore::SVGImageCache::setRequestedSizeAndScales): 

        (WebCore::SVGImageCache::requestedSizeAndScales):

        (WebCore::SVGImageCache::lookupOrCreateBitmapImageForClient):

        * svg/graphics/SVGImageCache.h:

        (WebCore):

        (SVGImageCache):

2012-06-11  David Barr  <davidbarr@chromium.org>

        Add css3-images image-resolution (dppx only)

119986

Source/WebCore/loader/cache/CachedFont.h

@@class CachedFontClient : public CachedRe

public:

    virtual ~CachedFontClient() { }

    static CachedResourceClientType expectedType() { return FontType; }

88 virtual CachedResourceClientType resourceClientType() { return expectedType(); }

88 virtual CachedResourceClientType resourceClientType() const { return expectedType(); }

    virtual void fontLoaded(CachedFont*) { }

};

119986

Source/WebCore/loader/cache/CachedImage.cpp

@@void CachedImage::load(CachedResourceLoa

        setLoading(false);

}

void CachedImage::removeClientForRenderer(RenderObject* renderer)

{

#if ENABLE(SVG)

    if (m_svgImageCache)

        m_svgImageCache->removeRendererFromCache(renderer);

#endif

    removeClient(renderer);

}

void CachedImage::didAddClient(CachedResourceClient* c)

{

    if (m_decodedDataDeletionTimer.isActive())

@@void CachedImage::didAddClient(CachedRes

    CachedResource::didAddClient(c);

}

void CachedImage::didRemoveClient(CachedResourceClient* c)

{

    ASSERT(c->resourceClientType() == CachedImageClient::expectedType());

#if ENABLE(SVG)

    if (m_svgImageCache)

        m_svgImageCache->removeClientFromCache(static_cast<CachedImageClient*>(c));

#endif

    CachedResource::didRemoveClient(c);

}

void CachedImage::allClientsRemoved()

{

    if (m_image && !errorOccurred())

@@inline Image* CachedImage::lookupOrCreat

        return 0;

    if (!m_image->isSVGImage())

        return m_image.get();

152 Image* useImage = m_svgImageCache->lookupOrCreateBitmapImageForR~~enderer~~(renderer);

154 Image* useImage = m_svgImageCache->lookupOrCreateBitmapImageForClient(renderer);

    if (useImage == Image::nullImage())

        return m_image.get();

    return useImage;

119986

Source/WebCore/loader/cache/CachedImage.h

@@public:

    IntSize imageSizeForRenderer(const RenderObject*, float multiplier); // returns the size of the complete image.

    void computeIntrinsicDimensions(Length& intrinsicWidth, Length& intrinsicHeight, FloatSize& intrinsicRatio);

70 void removeClientForRenderer(RenderObject*);

7170 virtual void didAddClient(CachedResourceClient*);

72

71 virtual void didRemoveClient(CachedResourceClient*);

72

    virtual void allClientsRemoved();

    virtual void destroyDecodedData();

@@class CachedImageClient : public CachedR

public:

    virtual ~CachedImageClient() { }

    static CachedResourceClientType expectedType() { return ImageType; }

121 virtual CachedResourceClientType resourceClientType() { return expectedType(); }

121 virtual CachedResourceClientType resourceClientType() const { return expectedType(); }

    // Called whenever a frame of an image changes, either because we got more data from the network or

    // because we are animating. If not null, the IntRect is the changed rect of the image.

119986

Source/WebCore/loader/cache/CachedRawResource.h

@@class CachedRawResourceClient : public C

public:

    virtual ~CachedRawResourceClient() { }

    static CachedResourceClientType expectedType() { return RawResourceType; }

69 virtual CachedResourceClientType resourceClientType() { return expectedType(); }

69 virtual CachedResourceClientType resourceClientType() const { return expectedType(); }

    virtual void dataSent(CachedResource*, unsigned long long /* bytesSent */, unsigned long long /* totalBytesToBeSent */) { }

    virtual void responseReceived(CachedResource*, const ResourceResponse&) { }

119986

Source/WebCore/loader/cache/CachedResource.cpp

@@void CachedResource::removeClient(Cached

    } else {

        ASSERT(m_clients.contains(client));

        m_clients.remove(client);

429 didRemoveClient(client);

    }

    if (canDelete() && !inCache())

119986

Source/WebCore/loader/cache/CachedResource.h

@@public:

    PreloadResult preloadResult() const { return static_cast<PreloadResult>(m_preloadResult); }

    virtual void didAddClient(CachedResourceClient*);

129 virtual void didRemoveClient(CachedResourceClient*) { }

    virtual void allClientsRemoved() { }

    unsigned count() const { return m_clients.size(); }

119986

Source/WebCore/loader/cache/CachedResourceClient.h

@@public:

    virtual void didReceiveData(CachedResource*) { };

    static CachedResourceClientType expectedType() { return BaseResourceType; }

53 virtual CachedResourceClientType resourceClientType() { return expectedType(); }

53 virtual CachedResourceClientType resourceClientType() const { return expectedType(); }

protected:

    CachedResourceClient() { }

119986

Source/WebCore/loader/cache/CachedSVGDocument.h

@@class CachedSVGDocumentClient : public C

public:

    virtual ~CachedSVGDocumentClient() { }

    static CachedResourceClientType expectedType() { return SVGDocumentType; }

55 virtual CachedResourceClientType resourceClientType() { return expectedType(); }

55 virtual CachedResourceClientType resourceClientType() const { return expectedType(); }

};

}

119986

Source/WebCore/loader/cache/CachedStyleSheetClient.h

@@class CachedStyleSheetClient : public Ca

public:

    virtual ~CachedStyleSheetClient() { }

    static CachedResourceClientType expectedType() { return StyleSheetType; }

39 virtual CachedResourceClientType resourceClientType() { return expectedType(); }

39 virtual CachedResourceClientType resourceClientType() const { return expectedType(); }

    virtual void setCSSStyleSheet(const String& /* href */, const KURL& /* baseURL */, const String& /* charset */, const CachedCSSStyleSheet*) { }

    virtual void setXSLStyleSheet(const String& /* href */, const KURL& /* baseURL */, const String& /* sheet */) { }

};

119986

Source/WebCore/rendering/style/StyleCachedImage.cpp

@@void StyleCachedImage::addClient(RenderO

void StyleCachedImage::removeClient(RenderObject* renderer)

{

100 m_image->removeClient~~ForRenderer~~(renderer);

100 m_image->removeClient(renderer);

}

PassRefPtr<Image> StyleCachedImage::image(RenderObject* renderer, const IntSize&) const

119986

Source/WebCore/rendering/style/StyleCachedImageSet.cpp

@@void StyleCachedImageSet::addClient(Rend

void StyleCachedImageSet::removeClient(RenderObject* renderer)

{

111 m_bestFitImage->removeClient~~ForRenderer~~(renderer);

111 m_bestFitImage->removeClient(renderer);

}

PassRefPtr<Image> StyleCachedImageSet::image(RenderObject* renderer, const IntSize&) const

119986

Source/WebCore/svg/graphics/SVGImageCache.cpp

#include "SVGImageCache.h"

#if ENABLE(SVG)

24#include "CachedImage.h"

#include "FrameView.h"

#include "GraphicsContext.h"

#include "ImageBuffer.h"

@@SVGImageCache::~SVGImageCache()

    m_sizeAndScalesMap.clear();

    ImageDataMap::iterator end = m_imageDataMap.end();

44 for (ImageDataMap::iterator it = m_imageDataMap.begin(); it != end; ++it)

    for (ImageDataMap::iterator it = m_imageDataMap.begin(); it != end; ++it) {

        // Checks if the client (it->first) is still valid. The client should remove itself from this

        // cache before its end of life, otherwise the following ASSERT will crash on pure virtual

        // function call or a general crash.

        ASSERT(it->first->resourceClientType() == CachedImageClient::expectedType());

4550 delete it->second.buffer;

51 }

    m_imageDataMap.clear();

}

50 void SVGImageCache::removeR~~enderer~~FromCache(const R~~enderObjec~~t* r~~enderer~~)

56void SVGImageCache::removeClientFromCache(const CachedImageClient* client)

5157{

52 ASSERT(r~~enderer~~);

53 m_sizeAndScalesMap.remove(r~~enderer~~);

58 ASSERT(client);

59 m_sizeAndScalesMap.remove(client);

5460

55 ImageDataMap::iterator it = m_imageDataMap.find(r~~enderer~~);

61 ImageDataMap::iterator it = m_imageDataMap.find(client);

    if (it == m_imageDataMap.end())

        return;

@@void SVGImageCache::removeRendererFromCa

    m_imageDataMap.remove(it);

}

63 void SVGImageCache::setRequestedSizeAndScales(const R~~enderObjec~~t* r~~enderer~~, const SizeAndScales& sizeAndScales)

69void SVGImageCache::setRequestedSizeAndScales(const CachedImageClient* client, const SizeAndScales& sizeAndScales)

6470{

65 ASSERT(r~~enderer~~);

71 ASSERT(client);

6672 ASSERT(!sizeAndScales.size.isEmpty());

67 m_sizeAndScalesMap.set(r~~enderer~~, sizeAndScales);

73 m_sizeAndScalesMap.set(client, sizeAndScales);

6874}

6975

70 SVGImageCache::SizeAndScales SVGImageCache::requestedSizeAndScales(const R~~enderObjec~~t* r~~enderer~~) const

76SVGImageCache::SizeAndScales SVGImageCache::requestedSizeAndScales(const CachedImageClient* client) const

7177{

72 ASSERT(r~~enderer~~);

73 SizeAndScalesMap::const_iterator it = m_sizeAndScalesMap.find(r~~enderer~~);

78 ASSERT(client);

79 SizeAndScalesMap::const_iterator it = m_sizeAndScalesMap.find(client);

    if (it == m_sizeAndScalesMap.end())

        return SizeAndScales();

    return it->second;

@@void SVGImageCache::redrawTimerFired(Tim

       redraw();

}

125 Image* SVGImageCache::lookupOrCreateBitmapImageForR~~enderer~~(const R~~enderObjec~~t* r~~enderer~~)

131Image* SVGImageCache::lookupOrCreateBitmapImageForClient(const CachedImageClient* client)

126132{

127 ASSERT(r~~enderer~~);

133 ASSERT(client);

128134

129 // The cache needs to know the size of the r~~enderer~~ before querying an image for it.

130 SizeAndScalesMap::iterator sizeIt = m_sizeAndScalesMap.find(r~~enderer~~);

135 // The cache needs to know the size of the client before querying an image for it.

136 SizeAndScalesMap::iterator sizeIt = m_sizeAndScalesMap.find(client);

    if (sizeIt == m_sizeAndScalesMap.end())

        return Image::nullImage();

@@Image* SVGImageCache::lookupOrCreateBitm

    float scale = sizeIt->second.scale;

    ASSERT(!size.isEmpty());

139 // Lookup image for r~~enderer~~ in cache and eventually update it.

140 ImageDataMap::iterator it = m_imageDataMap.find(r~~enderer~~);

145 // Lookup image for client in cache and eventually update it.

146 ImageDataMap::iterator it = m_imageDataMap.find(client);

    if (it != m_imageDataMap.end()) {

        ImageData& data = it->second;

@@Image* SVGImageCache::lookupOrCreateBitm

        if (data.sizeAndScales.size == size && data.sizeAndScales.zoom == zoom && data.sizeAndScales.scale == scale)

            return data.image.get();

148 // If the image size for the r~~enderer~~ changed, we have to delete the buffer, remove the item from the cache and recreate it.

154 // If the image size for the client changed, we have to delete the buffer, remove the item from the cache and recreate it.

        delete data.buffer;

        m_imageDataMap.remove(it);

    }

@@Image* SVGImageCache::lookupOrCreateBitm

    Image* newImagePtr = newImage.get();

    ASSERT(newImagePtr);

167 m_imageDataMap.add(r~~enderer~~, ImageData(newBuffer.leakPtr(), newImage.release(), sizeIt->second));

173 m_imageDataMap.add(client, ImageData(newBuffer.leakPtr(), newImage.release(), sizeIt->second));

    return newImagePtr;

}

119986

Source/WebCore/svg/graphics/SVGImageCache.h

namespace WebCore {

class CachedImage;

34class CachedImageClient;

3435class ImageBuffer;

35 class RenderObject;

class SVGImage;

class SVGImageCache {

@@public:

        float scale;

    };

66 void removeR~~enderer~~FromCache(const R~~enderObjec~~t*);

66 void removeClientFromCache(const CachedImageClient*);

6767

68 void setRequestedSizeAndScales(const R~~enderObjec~~t*, const SizeAndScales&);

69 SizeAndScales requestedSizeAndScales(const R~~enderObjec~~t*) const;

68 void setRequestedSizeAndScales(const CachedImageClient*, const SizeAndScales&);

69 SizeAndScales requestedSizeAndScales(const CachedImageClient*) const;

7070

71 Image* lookupOrCreateBitmapImageForR~~enderer~~(const R~~enderObjec~~t*);

71 Image* lookupOrCreateBitmapImageForClient(const CachedImageClient*);

    void imageContentChanged();

private:

@@private:

        RefPtr<Image> image;

    };

101 typedef HashMap<const R~~enderObjec~~t*, SizeAndScales> SizeAndScalesMap;

102 typedef HashMap<const R~~enderObjec~~t*, ImageData> ImageDataMap;

101 typedef HashMap<const CachedImageClient*, SizeAndScales> SizeAndScalesMap;

102 typedef HashMap<const CachedImageClient*, ImageData> ImageDataMap;

    SVGImage* m_svgImage;

    SizeAndScalesMap m_sizeAndScalesMap;

119986

LayoutTests/ChangeLog

2012-06-11  Xianzhu Wang  <wangxianzhu@chromium.org>

        SVGImageCache leaks image data

        https://bugs.webkit.org/show_bug.cgi?id=87792

        Reviewed by Nikolas Zimmermann.

        New test case.

        * svg/as-image/svg-image-leak-cached-data-expected.txt: Added.

        * svg/as-image/svg-image-leak-cached-data.html: Added.

2012-06-11  Ryosuke Niwa  <rniwa@webkit.org>

        Use testRunner instead of layoutTestController in animations tests

119986

LayoutTests/svg/as-image/svg-image-leak-cached-data-expected.txt

This test checks if SVGImageCache leaks SVG image data as reported in https://bugs.webkit.org/show_bug.cgi?id=87792. Its layout has no particular meaning. The test will cause crash of debug version when leaks of SVG image data is detected.

Note: the code detects leaks of SVG image data on destruction of SVGImageCache, which doesn't work on platforms that DumpRenderTree leaks the cache itself.

0

LayoutTests/svg/as-image/svg-image-leak-cached-data.html

<html>

<head>

<script>

if (window.layoutTestController) {

    layoutTestController.dumpAsText();

    layoutTestController.waitUntilDone();

}

var count = 0;

function test() {

    var img = document.getElementById('img');

    document.body.replaceChild(img.cloneNode(), img);

    if (++count < 10)

        setTimeout(test, 0);

    else if (window.layoutTestController)

        layoutTestController.notifyDone();

}

</script>

</head>

<body onload='test()'>

    <p>This test checks if SVGImageCache leaks SVG image data as reported in

    https://bugs.webkit.org/show_bug.cgi?id=87792. Its layout has no particular meaning.

    The test will cause crash of debug version when leaks of SVG image data is detected.</p>

    <p>Note: the code detects leaks of SVG image data on destruction of SVGImageCache,

    which doesn't work on platforms that DumpRenderTree leaks the cache itself.</p>

    <img id='img' src='resources/circle.svg'>

</body>

</html>

0