Diff
Modified: trunk/LayoutTests/ChangeLog (119999 => 120000)
--- trunk/LayoutTests/ChangeLog 2012-06-11 19:50:00 UTC (rev 119999)
+++ trunk/LayoutTests/ChangeLog 2012-06-11 20:15:47 UTC (rev 120000)
@@ -1,3 +1,15 @@
+2012-06-11 Xianzhu Wang <[email protected]>
+
+ 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 Sudarsana Nagineni <[email protected]>
[EFL] REGRESSION (r119788): tests rely on pathToLocalResource are failing after r119788
Added: trunk/LayoutTests/svg/as-image/svg-image-leak-cached-data-expected.txt (0 => 120000)
--- trunk/LayoutTests/svg/as-image/svg-image-leak-cached-data-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/as-image/svg-image-leak-cached-data-expected.txt 2012-06-11 20:15:47 UTC (rev 120000)
@@ -0,0 +1,5 @@
+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.
+
+
Added: trunk/LayoutTests/svg/as-image/svg-image-leak-cached-data.html (0 => 120000)
--- trunk/LayoutTests/svg/as-image/svg-image-leak-cached-data.html (rev 0)
+++ trunk/LayoutTests/svg/as-image/svg-image-leak-cached-data.html 2012-06-11 20:15:47 UTC (rev 120000)
@@ -0,0 +1,29 @@
+<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=''>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (119999 => 120000)
--- trunk/Source/WebCore/ChangeLog 2012-06-11 19:50:00 UTC (rev 119999)
+++ trunk/Source/WebCore/ChangeLog 2012-06-11 20:15:47 UTC (rev 120000)
@@ -1,3 +1,57 @@
+2012-06-11 Xianzhu Wang <[email protected]>
+
+ 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 Mark Pilgrim <[email protected]>
[Chromium] Call shared timer functions directly
Modified: trunk/Source/WebCore/loader/cache/CachedFont.h (119999 => 120000)
--- trunk/Source/WebCore/loader/cache/CachedFont.h 2012-06-11 19:50:00 UTC (rev 119999)
+++ trunk/Source/WebCore/loader/cache/CachedFont.h 2012-06-11 20:15:47 UTC (rev 120000)
@@ -85,7 +85,7 @@
public:
virtual ~CachedFontClient() { }
static CachedResourceClientType expectedType() { return FontType; }
- virtual CachedResourceClientType resourceClientType() { return expectedType(); }
+ virtual CachedResourceClientType resourceClientType() const { return expectedType(); }
virtual void fontLoaded(CachedFont*) { }
};
Modified: trunk/Source/WebCore/loader/cache/CachedImage.cpp (119999 => 120000)
--- trunk/Source/WebCore/loader/cache/CachedImage.cpp 2012-06-11 19:50:00 UTC (rev 119999)
+++ trunk/Source/WebCore/loader/cache/CachedImage.cpp 2012-06-11 20:15:47 UTC (rev 120000)
@@ -92,15 +92,6 @@
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())
@@ -118,6 +109,17 @@
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())
@@ -149,7 +151,7 @@
return 0;
if (!m_image->isSVGImage())
return m_image.get();
- Image* useImage = m_svgImageCache->lookupOrCreateBitmapImageForRenderer(renderer);
+ Image* useImage = m_svgImageCache->lookupOrCreateBitmapImageForClient(renderer);
if (useImage == Image::nullImage())
return m_image.get();
return useImage;
Modified: trunk/Source/WebCore/loader/cache/CachedImage.h (119999 => 120000)
--- trunk/Source/WebCore/loader/cache/CachedImage.h 2012-06-11 19:50:00 UTC (rev 119999)
+++ trunk/Source/WebCore/loader/cache/CachedImage.h 2012-06-11 20:15:47 UTC (rev 120000)
@@ -67,9 +67,9 @@
IntSize imageSizeForRenderer(const RenderObject*, float multiplier); // returns the size of the complete image.
void computeIntrinsicDimensions(Length& intrinsicWidth, Length& intrinsicHeight, FloatSize& intrinsicRatio);
- void removeClientForRenderer(RenderObject*);
virtual void didAddClient(CachedResourceClient*);
-
+ virtual void didRemoveClient(CachedResourceClient*);
+
virtual void allClientsRemoved();
virtual void destroyDecodedData();
@@ -118,7 +118,7 @@
public:
virtual ~CachedImageClient() { }
static CachedResourceClientType expectedType() { return ImageType; }
- virtual CachedResourceClientType resourceClientType() { return expectedType(); }
+ 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.
Modified: trunk/Source/WebCore/loader/cache/CachedRawResource.h (119999 => 120000)
--- trunk/Source/WebCore/loader/cache/CachedRawResource.h 2012-06-11 19:50:00 UTC (rev 119999)
+++ trunk/Source/WebCore/loader/cache/CachedRawResource.h 2012-06-11 20:15:47 UTC (rev 120000)
@@ -66,7 +66,7 @@
public:
virtual ~CachedRawResourceClient() { }
static CachedResourceClientType expectedType() { return RawResourceType; }
- virtual CachedResourceClientType resourceClientType() { return expectedType(); }
+ virtual CachedResourceClientType resourceClientType() const { return expectedType(); }
virtual void dataSent(CachedResource*, unsigned long long /* bytesSent */, unsigned long long /* totalBytesToBeSent */) { }
virtual void responseReceived(CachedResource*, const ResourceResponse&) { }
Modified: trunk/Source/WebCore/loader/cache/CachedResource.cpp (119999 => 120000)
--- trunk/Source/WebCore/loader/cache/CachedResource.cpp 2012-06-11 19:50:00 UTC (rev 119999)
+++ trunk/Source/WebCore/loader/cache/CachedResource.cpp 2012-06-11 20:15:47 UTC (rev 120000)
@@ -426,6 +426,7 @@
} else {
ASSERT(m_clients.contains(client));
m_clients.remove(client);
+ didRemoveClient(client);
}
if (canDelete() && !inCache())
Modified: trunk/Source/WebCore/loader/cache/CachedResource.h (119999 => 120000)
--- trunk/Source/WebCore/loader/cache/CachedResource.h 2012-06-11 19:50:00 UTC (rev 119999)
+++ trunk/Source/WebCore/loader/cache/CachedResource.h 2012-06-11 20:15:47 UTC (rev 120000)
@@ -126,6 +126,7 @@
PreloadResult preloadResult() const { return static_cast<PreloadResult>(m_preloadResult); }
virtual void didAddClient(CachedResourceClient*);
+ virtual void didRemoveClient(CachedResourceClient*) { }
virtual void allClientsRemoved() { }
unsigned count() const { return m_clients.size(); }
Modified: trunk/Source/WebCore/loader/cache/CachedResourceClient.h (119999 => 120000)
--- trunk/Source/WebCore/loader/cache/CachedResourceClient.h 2012-06-11 19:50:00 UTC (rev 119999)
+++ trunk/Source/WebCore/loader/cache/CachedResourceClient.h 2012-06-11 20:15:47 UTC (rev 120000)
@@ -50,7 +50,7 @@
virtual void didReceiveData(CachedResource*) { };
static CachedResourceClientType expectedType() { return BaseResourceType; }
- virtual CachedResourceClientType resourceClientType() { return expectedType(); }
+ virtual CachedResourceClientType resourceClientType() const { return expectedType(); }
protected:
CachedResourceClient() { }
Modified: trunk/Source/WebCore/loader/cache/CachedSVGDocument.h (119999 => 120000)
--- trunk/Source/WebCore/loader/cache/CachedSVGDocument.h 2012-06-11 19:50:00 UTC (rev 119999)
+++ trunk/Source/WebCore/loader/cache/CachedSVGDocument.h 2012-06-11 20:15:47 UTC (rev 120000)
@@ -52,7 +52,7 @@
public:
virtual ~CachedSVGDocumentClient() { }
static CachedResourceClientType expectedType() { return SVGDocumentType; }
- virtual CachedResourceClientType resourceClientType() { return expectedType(); }
+ virtual CachedResourceClientType resourceClientType() const { return expectedType(); }
};
}
Modified: trunk/Source/WebCore/loader/cache/CachedStyleSheetClient.h (119999 => 120000)
--- trunk/Source/WebCore/loader/cache/CachedStyleSheetClient.h 2012-06-11 19:50:00 UTC (rev 119999)
+++ trunk/Source/WebCore/loader/cache/CachedStyleSheetClient.h 2012-06-11 20:15:47 UTC (rev 120000)
@@ -36,7 +36,7 @@
public:
virtual ~CachedStyleSheetClient() { }
static CachedResourceClientType expectedType() { return StyleSheetType; }
- virtual CachedResourceClientType resourceClientType() { return expectedType(); }
+ 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 */) { }
};
Modified: trunk/Source/WebCore/rendering/style/StyleCachedImage.cpp (119999 => 120000)
--- trunk/Source/WebCore/rendering/style/StyleCachedImage.cpp 2012-06-11 19:50:00 UTC (rev 119999)
+++ trunk/Source/WebCore/rendering/style/StyleCachedImage.cpp 2012-06-11 20:15:47 UTC (rev 120000)
@@ -97,7 +97,7 @@
void StyleCachedImage::removeClient(RenderObject* renderer)
{
- m_image->removeClientForRenderer(renderer);
+ m_image->removeClient(renderer);
}
PassRefPtr<Image> StyleCachedImage::image(RenderObject* renderer, const IntSize&) const
Modified: trunk/Source/WebCore/rendering/style/StyleCachedImageSet.cpp (119999 => 120000)
--- trunk/Source/WebCore/rendering/style/StyleCachedImageSet.cpp 2012-06-11 19:50:00 UTC (rev 119999)
+++ trunk/Source/WebCore/rendering/style/StyleCachedImageSet.cpp 2012-06-11 20:15:47 UTC (rev 120000)
@@ -108,7 +108,7 @@
void StyleCachedImageSet::removeClient(RenderObject* renderer)
{
- m_bestFitImage->removeClientForRenderer(renderer);
+ m_bestFitImage->removeClient(renderer);
}
PassRefPtr<Image> StyleCachedImageSet::image(RenderObject* renderer, const IntSize&) const
Modified: trunk/Source/WebCore/svg/graphics/SVGImageCache.cpp (119999 => 120000)
--- trunk/Source/WebCore/svg/graphics/SVGImageCache.cpp 2012-06-11 19:50:00 UTC (rev 119999)
+++ trunk/Source/WebCore/svg/graphics/SVGImageCache.cpp 2012-06-11 20:15:47 UTC (rev 120000)
@@ -21,6 +21,7 @@
#include "SVGImageCache.h"
#if ENABLE(SVG)
+#include "CachedImage.h"
#include "FrameView.h"
#include "GraphicsContext.h"
#include "ImageBuffer.h"
@@ -41,18 +42,23 @@
m_sizeAndScalesMap.clear();
ImageDataMap::iterator end = m_imageDataMap.end();
- 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());
delete it->second.buffer;
+ }
m_imageDataMap.clear();
}
-void SVGImageCache::removeRendererFromCache(const RenderObject* renderer)
+void SVGImageCache::removeClientFromCache(const CachedImageClient* client)
{
- ASSERT(renderer);
- m_sizeAndScalesMap.remove(renderer);
+ ASSERT(client);
+ m_sizeAndScalesMap.remove(client);
- ImageDataMap::iterator it = m_imageDataMap.find(renderer);
+ ImageDataMap::iterator it = m_imageDataMap.find(client);
if (it == m_imageDataMap.end())
return;
@@ -60,17 +66,17 @@
m_imageDataMap.remove(it);
}
-void SVGImageCache::setRequestedSizeAndScales(const RenderObject* renderer, const SizeAndScales& sizeAndScales)
+void SVGImageCache::setRequestedSizeAndScales(const CachedImageClient* client, const SizeAndScales& sizeAndScales)
{
- ASSERT(renderer);
+ ASSERT(client);
ASSERT(!sizeAndScales.size.isEmpty());
- m_sizeAndScalesMap.set(renderer, sizeAndScales);
+ m_sizeAndScalesMap.set(client, sizeAndScales);
}
-SVGImageCache::SizeAndScales SVGImageCache::requestedSizeAndScales(const RenderObject* renderer) const
+SVGImageCache::SizeAndScales SVGImageCache::requestedSizeAndScales(const CachedImageClient* client) const
{
- ASSERT(renderer);
- SizeAndScalesMap::const_iterator it = m_sizeAndScalesMap.find(renderer);
+ ASSERT(client);
+ SizeAndScalesMap::const_iterator it = m_sizeAndScalesMap.find(client);
if (it == m_sizeAndScalesMap.end())
return SizeAndScales();
return it->second;
@@ -122,12 +128,12 @@
redraw();
}
-Image* SVGImageCache::lookupOrCreateBitmapImageForRenderer(const RenderObject* renderer)
+Image* SVGImageCache::lookupOrCreateBitmapImageForClient(const CachedImageClient* client)
{
- ASSERT(renderer);
+ ASSERT(client);
- // The cache needs to know the size of the renderer before querying an image for it.
- SizeAndScalesMap::iterator sizeIt = m_sizeAndScalesMap.find(renderer);
+ // The cache needs to know the size of the client before querying an image for it.
+ SizeAndScalesMap::iterator sizeIt = m_sizeAndScalesMap.find(client);
if (sizeIt == m_sizeAndScalesMap.end())
return Image::nullImage();
@@ -136,8 +142,8 @@
float scale = sizeIt->second.scale;
ASSERT(!size.isEmpty());
- // Lookup image for renderer in cache and eventually update it.
- ImageDataMap::iterator it = m_imageDataMap.find(renderer);
+ // Lookup image for client in cache and eventually update it.
+ ImageDataMap::iterator it = m_imageDataMap.find(client);
if (it != m_imageDataMap.end()) {
ImageData& data = ""
@@ -145,7 +151,7 @@
if (data.sizeAndScales.size == size && data.sizeAndScales.zoom == zoom && data.sizeAndScales.scale == scale)
return data.image.get();
- // If the image size for the renderer changed, we have to delete the buffer, remove the item from the cache and recreate it.
+ // 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);
}
@@ -164,7 +170,7 @@
Image* newImagePtr = newImage.get();
ASSERT(newImagePtr);
- m_imageDataMap.add(renderer, ImageData(newBuffer.leakPtr(), newImage.release(), sizeIt->second));
+ m_imageDataMap.add(client, ImageData(newBuffer.leakPtr(), newImage.release(), sizeIt->second));
return newImagePtr;
}
Modified: trunk/Source/WebCore/svg/graphics/SVGImageCache.h (119999 => 120000)
--- trunk/Source/WebCore/svg/graphics/SVGImageCache.h 2012-06-11 19:50:00 UTC (rev 119999)
+++ trunk/Source/WebCore/svg/graphics/SVGImageCache.h 2012-06-11 20:15:47 UTC (rev 120000)
@@ -31,8 +31,8 @@
namespace WebCore {
class CachedImage;
+class CachedImageClient;
class ImageBuffer;
-class RenderObject;
class SVGImage;
class SVGImageCache {
@@ -63,12 +63,12 @@
float scale;
};
- void removeRendererFromCache(const RenderObject*);
+ void removeClientFromCache(const CachedImageClient*);
- void setRequestedSizeAndScales(const RenderObject*, const SizeAndScales&);
- SizeAndScales requestedSizeAndScales(const RenderObject*) const;
+ void setRequestedSizeAndScales(const CachedImageClient*, const SizeAndScales&);
+ SizeAndScales requestedSizeAndScales(const CachedImageClient*) const;
- Image* lookupOrCreateBitmapImageForRenderer(const RenderObject*);
+ Image* lookupOrCreateBitmapImageForClient(const CachedImageClient*);
void imageContentChanged();
private:
@@ -98,8 +98,8 @@
RefPtr<Image> image;
};
- typedef HashMap<const RenderObject*, SizeAndScales> SizeAndScalesMap;
- typedef HashMap<const RenderObject*, ImageData> ImageDataMap;
+ typedef HashMap<const CachedImageClient*, SizeAndScales> SizeAndScalesMap;
+ typedef HashMap<const CachedImageClient*, ImageData> ImageDataMap;
SVGImage* m_svgImage;
SizeAndScalesMap m_sizeAndScalesMap;
