[chromium] Leaking SkBitmaps for background images
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 25 Aug 2011 16:42:15 +0000 (16:42 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 25 Aug 2011 16:42:15 +0000 (16:42 +0000)
https://bugs.webkit.org/show_bug.cgi?id=66488

Patch by John Bates <jbates@google.com> on 2011-08-25
Reviewed by Stephen White.

This patch simply changes NativeImageSkia to have a SkBitmap instead of
deriving from SkBitmap. All dependent code updated to access the member
instead of calling SkBitmap methods on NativeImageSkia objects. This
may or may not fix the memory leak, but it's definitely a bug that could
cause memory leaks.

Source/WebCore:

* platform/chromium/DragImageChromiumSkia.cpp:
(WebCore::createDragImageFromImage):
* platform/graphics/chromium/PlatformImage.cpp:
(WebCore::PlatformImage::updateFromImage):
* platform/graphics/skia/BitmapImageSingleFrameSkia.h:
(WebCore::BitmapImageSingleFrameSkia::currentFrameHasAlpha):
(WebCore::BitmapImageSingleFrameSkia::size):
(WebCore::BitmapImageSingleFrameSkia::notSolidColor):
* platform/graphics/skia/GraphicsContext3DSkia.cpp:
(WebCore::GraphicsContext3D::getImageData):
* platform/graphics/skia/ImageSkia.cpp:
(WebCore::paintSkBitmap):
(WebCore::Image::drawPattern):
(WebCore::BitmapImage::checkForSolidColor):
* platform/graphics/skia/NativeImageSkia.cpp:
(WebCore::NativeImageSkia::NativeImageSkia):
(WebCore::NativeImageSkia::decodedSize):
(WebCore::NativeImageSkia::resizedBitmap):
* platform/graphics/skia/NativeImageSkia.h:
(WebCore::NativeImageSkia::bitmap):
* platform/graphics/skia/PatternSkia.cpp:
(WebCore::Pattern::platformPattern):
* platform/image-decoders/ImageDecoder.h:
(WebCore::ImageFrame::getAddr):
* platform/image-decoders/skia/ImageDecoderSkia.cpp:
(WebCore::ImageFrame::operator=):
(WebCore::ImageFrame::clearPixelData):
(WebCore::ImageFrame::zeroFillPixelData):
(WebCore::ImageFrame::copyBitmapData):
(WebCore::ImageFrame::setSize):
(WebCore::ImageFrame::hasAlpha):
(WebCore::ImageFrame::setHasAlpha):
(WebCore::ImageFrame::width):
(WebCore::ImageFrame::height):

Source/WebKit/chromium:

* src/PlatformBridge.cpp:
(WebCore::PlatformBridge::clipboardWriteImage):
* src/WebImageDecoder.cpp:
(WebKit::WebImageDecoder::getFrameAtIndex):
* src/WebImageSkia.cpp:
(WebKit::WebImage::fromData):
(WebKit::WebImage::operator=):

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

16 files changed:
Source/WebCore/ChangeLog
Source/WebCore/platform/chromium/DragImageChromiumSkia.cpp
Source/WebCore/platform/graphics/chromium/PlatformImage.cpp
Source/WebCore/platform/graphics/skia/BitmapImageSingleFrameSkia.h
Source/WebCore/platform/graphics/skia/GraphicsContext3DSkia.cpp
Source/WebCore/platform/graphics/skia/ImageSkia.cpp
Source/WebCore/platform/graphics/skia/NativeImageSkia.cpp
Source/WebCore/platform/graphics/skia/NativeImageSkia.h
Source/WebCore/platform/graphics/skia/PatternSkia.cpp
Source/WebCore/platform/image-decoders/ImageDecoder.h
Source/WebCore/platform/image-decoders/skia/ImageDecoderSkia.cpp
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/src/PlatformBridge.cpp
Source/WebKit/chromium/src/WebImageDecoder.cpp
Source/WebKit/chromium/src/WebImageSkia.cpp
Source/WebKit/chromium/tests/DragImageTest.cpp

index 74b1d91367500911d248cccc14405156518c7159..15312c63289a8fe4e590b36d167873ec94093ef7 100644 (file)
@@ -1,3 +1,51 @@
+2011-08-25  John Bates  <jbates@google.com>
+
+        [chromium] Leaking SkBitmaps for background images
+        https://bugs.webkit.org/show_bug.cgi?id=66488
+
+        Reviewed by Stephen White.
+
+        This patch simply changes NativeImageSkia to have a SkBitmap instead of
+        deriving from SkBitmap. All dependent code updated to access the member
+        instead of calling SkBitmap methods on NativeImageSkia objects. This
+        may or may not fix the memory leak, but it's definitely a bug that could
+        cause memory leaks.
+
+        * platform/chromium/DragImageChromiumSkia.cpp:
+        (WebCore::createDragImageFromImage):
+        * platform/graphics/chromium/PlatformImage.cpp:
+        (WebCore::PlatformImage::updateFromImage):
+        * platform/graphics/skia/BitmapImageSingleFrameSkia.h:
+        (WebCore::BitmapImageSingleFrameSkia::currentFrameHasAlpha):
+        (WebCore::BitmapImageSingleFrameSkia::size):
+        (WebCore::BitmapImageSingleFrameSkia::notSolidColor):
+        * platform/graphics/skia/GraphicsContext3DSkia.cpp:
+        (WebCore::GraphicsContext3D::getImageData):
+        * platform/graphics/skia/ImageSkia.cpp:
+        (WebCore::paintSkBitmap):
+        (WebCore::Image::drawPattern):
+        (WebCore::BitmapImage::checkForSolidColor):
+        * platform/graphics/skia/NativeImageSkia.cpp:
+        (WebCore::NativeImageSkia::NativeImageSkia):
+        (WebCore::NativeImageSkia::decodedSize):
+        (WebCore::NativeImageSkia::resizedBitmap):
+        * platform/graphics/skia/NativeImageSkia.h:
+        (WebCore::NativeImageSkia::bitmap):
+        * platform/graphics/skia/PatternSkia.cpp:
+        (WebCore::Pattern::platformPattern):
+        * platform/image-decoders/ImageDecoder.h:
+        (WebCore::ImageFrame::getAddr):
+        * platform/image-decoders/skia/ImageDecoderSkia.cpp:
+        (WebCore::ImageFrame::operator=):
+        (WebCore::ImageFrame::clearPixelData):
+        (WebCore::ImageFrame::zeroFillPixelData):
+        (WebCore::ImageFrame::copyBitmapData):
+        (WebCore::ImageFrame::setSize):
+        (WebCore::ImageFrame::hasAlpha):
+        (WebCore::ImageFrame::setHasAlpha):
+        (WebCore::ImageFrame::width):
+        (WebCore::ImageFrame::height):
+
 2011-08-25  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r93774.
index e5c1cee732ac85855318c1babbe43e5d7a796f65..e5ca1757a5aab596f87dd6b2da05bebf653d14be 100644 (file)
@@ -101,7 +101,7 @@ DragImageRef createDragImageFromImage(Image* image)
         return 0;
 
     SkBitmap* dragImage = new SkBitmap();
-    bitmap->copyTo(dragImage, SkBitmap::kARGB_8888_Config);
+    bitmap->bitmap().copyTo(dragImage, SkBitmap::kARGB_8888_Config);
     return dragImage;
 }
 
index c31b29c4b40310f58a802d74e95b2b7dab0045a7..9672c9c7b6659fd57481359bdcfb78a2056bcd03 100644 (file)
@@ -50,10 +50,10 @@ void PlatformImage::updateFromImage(NativeImagePtr nativeImage)
 #if USE(SKIA)
     // The layer contains an Image.
     NativeImageSkia* skiaImage = static_cast<NativeImageSkia*>(nativeImage);
-    const SkBitmap* skiaBitmap = skiaImage;
+    ASSERT(skiaImage);
+    const SkBitmap& skiaBitmap = skiaImage->bitmap();
 
-    IntSize bitmapSize(skiaBitmap->width(), skiaBitmap->height());
-    ASSERT(skiaBitmap);
+    IntSize bitmapSize(skiaBitmap.width(), skiaBitmap.height());
 #elif USE(CG)
     // NativeImagePtr is a CGImageRef on Mac OS X.
     int width = CGImageGetWidth(nativeImage);
@@ -69,10 +69,10 @@ void PlatformImage::updateFromImage(NativeImagePtr nativeImage)
     }
 
 #if USE(SKIA)
-    SkAutoLockPixels lock(*skiaBitmap);
+    SkAutoLockPixels lock(skiaBitmap);
     // FIXME: do we need to support more image configurations?
-    ASSERT(skiaBitmap->config()== SkBitmap::kARGB_8888_Config);
-    skiaBitmap->copyPixelsTo(m_pixelData.get(), bufferSize);
+    ASSERT(skiaBitmap.config()== SkBitmap::kARGB_8888_Config);
+    skiaBitmap.copyPixelsTo(m_pixelData.get(), bufferSize);
 #elif USE(CG)
     // FIXME: we should get rid of this temporary copy where possible.
     int tempRowBytes = width * 4;
index a33b5f75d9af3591af2ecdbd3555dc799c950268..9c89c69273d39416dc5c8f47fddaa62da7d00583 100644 (file)
@@ -53,11 +53,11 @@ public:
 
     virtual bool isBitmapImage() const { return true; }
 
-    virtual bool currentFrameHasAlpha() { return !m_nativeImage.isOpaque(); }
+    virtual bool currentFrameHasAlpha() { return !m_nativeImage.bitmap().isOpaque(); }
 
     virtual IntSize size() const
     {
-        return IntSize(m_nativeImage.width(), m_nativeImage.height());
+        return IntSize(m_nativeImage.bitmap().width(), m_nativeImage.bitmap().height());
     }
 
     // Do nothing, as we only have the one representation of data (decoded).
@@ -77,7 +77,7 @@ public:
 #if !ASSERT_DISABLED
     virtual bool notSolidColor()
     {
-        return m_nativeImage.width() != 1 || m_nativeImage.height() != 1;
+        return m_nativeImage.bitmap().width() != 1 || m_nativeImage.bitmap().height() != 1;
     }
 #endif
 
index ec5175c351851de44bbfbbd4669024e7803013df..f341152d0be543fa87f9507c06a7e92eacd11fd4 100644 (file)
@@ -54,7 +54,7 @@ bool GraphicsContext3D::getImageData(Image* image,
     OwnPtr<NativeImageSkia> pixels;
     NativeImageSkia* skiaImage = image->nativeImageForCurrentFrame();
     AlphaOp neededAlphaOp = AlphaDoNothing;
-    bool hasAlpha = skiaImage ? !skiaImage->isOpaque() : true;
+    bool hasAlpha = skiaImage ? !skiaImage->bitmap().isOpaque() : true;
     if ((!skiaImage || ignoreGammaAndColorProfile || (hasAlpha && !premultiplyAlpha)) && image->data()) {
         ImageSource decoder(ImageSource::AlphaNotPremultiplied,
                             ignoreGammaAndColorProfile ? ImageSource::GammaAndColorProfileIgnored : ImageSource::GammaAndColorProfileApplied);
@@ -64,9 +64,9 @@ bool GraphicsContext3D::getImageData(Image* image,
             return false;
         hasAlpha = decoder.frameHasAlphaAtIndex(0);
         pixels = adoptPtr(decoder.createFrameAtIndex(0));
-        if (!pixels.get() || !pixels->isDataComplete() || !pixels->width() || !pixels->height())
+        if (!pixels.get() || !pixels->isDataComplete() || !pixels->bitmap().width() || !pixels->bitmap().height())
             return false;
-        SkBitmap::Config skiaConfig = pixels->config();
+        SkBitmap::Config skiaConfig = pixels->bitmap().config();
         if (skiaConfig != SkBitmap::kARGB_8888_Config)
             return false;
         skiaImage = pixels.get();
@@ -76,13 +76,13 @@ bool GraphicsContext3D::getImageData(Image* image,
         neededAlphaOp = AlphaDoUnmultiply;
     if (!skiaImage)
         return false;
-    SkBitmap& skiaImageRef = *skiaImage;
+    const SkBitmap& skiaImageRef = skiaImage->bitmap();
     SkAutoLockPixels lock(skiaImageRef);
-    ASSERT(skiaImage->rowBytes() == skiaImage->width() * 4);
-    outputVector.resize(skiaImage->rowBytes() * skiaImage->height());
-    return packPixels(reinterpret_cast<const uint8_t*>(skiaImage->getPixels()),
+    ASSERT(skiaImageRef.rowBytes() == skiaImageRef.width() * 4);
+    outputVector.resize(skiaImageRef.rowBytes() * skiaImageRef.height());
+    return packPixels(reinterpret_cast<const uint8_t*>(skiaImageRef.getPixels()),
                       SK_B32_SHIFT ? SourceFormatRGBA8 : SourceFormatBGRA8,
-                      skiaImage->width(), skiaImage->height(), 0,
+                      skiaImageRef.width(), skiaImageRef.height(), 0,
                       format, type, neededAlphaOp, outputVector.data());
 }
 
index 939bac6fc53c8998c503d552d5593c52e9a99991..06fc7d76582c01545033cf788e2a9237f5b9a57d 100644 (file)
@@ -229,7 +229,7 @@ static void paintSkBitmap(PlatformContextSkia* platformContext, const NativeImag
         // is something interesting going on with the matrix (like a rotation).
         // Note: for serialization, we will want to subset the bitmap first so
         // we don't send extra pixels.
-        canvas->drawBitmapRect(bitmap, &srcRect, destRect, &paint);
+        canvas->drawBitmapRect(bitmap.bitmap(), &srcRect, destRect, &paint);
     }
 }
 
@@ -331,7 +331,7 @@ void Image::drawPattern(GraphicsContext* context,
     } else {
         // No need to do nice resampling.
         SkBitmap srcSubset;
-        bitmap->extractSubset(&srcSubset, srcRect);
+        bitmap->bitmap().extractSubset(&srcSubset, srcRect);
         shader = SkShader::CreateBitmapShader(srcSubset, SkShader::kRepeat_TileMode, SkShader::kRepeat_TileMode);
     }
 
@@ -385,12 +385,12 @@ void BitmapImage::checkForSolidColor()
     WebCore::NativeImageSkia* frame = frameAtIndex(0);
 
     if (frame && size().width() == 1 && size().height() == 1) {
-        SkAutoLockPixels lock(*frame);
-        if (!frame->getPixels())
+        SkAutoLockPixels lock(frame->bitmap());
+        if (!frame->bitmap().getPixels())
             return;
 
         m_isSolidColor = true;
-        m_solidColor = Color(frame->getColor(0, 0));
+        m_solidColor = Color(frame->bitmap().getColor(0, 0));
     }
 }
 
index 8fe11a3722cbb262c1082f5697817a2134437727..f308579b81dfbee291fe9008aacd5176db4788ee 100644 (file)
 namespace WebCore {
 
 NativeImageSkia::NativeImageSkia()
-    : m_isDataComplete(false),
-      m_resizeRequests(0)
+    : m_resizeRequests(0),
+      m_isDataComplete(false)
 {
 }
 
 NativeImageSkia::NativeImageSkia(const SkBitmap& other)
-    : SkBitmap(other),
-      m_isDataComplete(false),
-      m_resizeRequests(0)
+    : m_image(other),
+      m_resizeRequests(0),
+      m_isDataComplete(false)
 {
 }
 
@@ -57,7 +57,7 @@ NativeImageSkia::~NativeImageSkia()
 
 int NativeImageSkia::decodedSize() const
 {
-    return getSize() + m_resizedImage.getSize();
+    return m_image.getSize() + m_resizedImage.getSize();
 }
 
 bool NativeImageSkia::hasResizedBitmap(const SkIRect& srcSubset, int destWidth, int destHeight) const
@@ -75,7 +75,7 @@ SkBitmap NativeImageSkia::resizedBitmap(const SkIRect& srcSubset,
             && shouldCacheResampling(srcSubset, destWidth, destHeight, destVisibleSubset);
 
         SkBitmap subset;
-        extractSubset(&subset, srcSubset);
+        m_image.extractSubset(&subset, srcSubset);
         if (!shouldCache) {
             // Just resize the visible subset and return it.
             SkBitmap resizedImage = skia::ImageOperations::Resize(subset, skia::ImageOperations::RESIZE_LANCZOS3, destWidth, destHeight, destVisibleSubset);
index e1135ea8ab54b560b5d928b8eed034e42b0d0baf..a70a47c420432b04e888c53661619bd5135c4b44 100644 (file)
@@ -38,9 +38,9 @@
 namespace WebCore {
 
 // This object is used as the "native image" in our port. When WebKit uses
-// "NativeImagePtr", it is a pointer to this type. It is an SkBitmap, but also
+// "NativeImagePtr", it is a pointer to this type. It has an SkBitmap, and also
 // stores a cached resized image.
-class NativeImageSkia : public SkBitmap {
+class NativeImageSkia {
 public:
     NativeImageSkia();
     ~NativeImageSkia();
@@ -62,6 +62,10 @@ public:
     // Returns true if the entire image has been decoded.
     bool isDataComplete() const { return m_isDataComplete; }
 
+    // Get reference to the internal SkBitmap representing this image.
+    const SkBitmap& bitmap() const { return m_image; }
+    SkBitmap& bitmap() { return m_image; }
+
     // We can keep a resized version of the bitmap cached on this object.
     // This function will return true if there is a cached version of the
     // given image subset with the given dimensions and subsets.
@@ -116,9 +120,8 @@ private:
                                int destHeight,
                                const SkIRect& destSubset) const;
 
-    // Set to true when the data is complete. Before the entire image has
-    // loaded, we do not want to cache a resize.
-    bool m_isDataComplete;
+    // The original image.
+    SkBitmap m_image;
 
     // The cached bitmap. This will be empty() if there is no cached image.
     mutable SkBitmap m_resizedImage;
@@ -138,6 +141,10 @@ private:
     // image resizes.
     mutable CachedImageInfo m_cachedImageInfo;
     mutable int m_resizeRequests;
+
+    // Set to true when the data is complete. Before the entire image has
+    // loaded, we do not want to cache a resize.
+    bool m_isDataComplete;
 };
 
 }
index 72fac773492ba4da4fef14020f1730b6005fc2d4..cd6a6676e3a9ad1da0315955394cc8e023242971 100644 (file)
@@ -59,13 +59,13 @@ PlatformPatternPtr Pattern::platformPattern(const AffineTransform& patternTransf
     // and expanded scale and skew in:
     // LayoutTests/svg/W3C-SVG-1.1/pservers-grad-06-b.svg
 
-    SkBitmap* bm = m_tileImage->nativeImageForCurrentFrame();
+    const NativeImageSkia* image = m_tileImage->nativeImageForCurrentFrame();
     // If we don't have a bitmap, return a transparent shader.
-    if (!bm)
+    if (!image)
         m_pattern = new SkColorShader(SkColorSetARGB(0, 0, 0, 0));
 
     else if (m_repeatX && m_repeatY)
-        m_pattern = SkShader::CreateBitmapShader(*bm, SkShader::kRepeat_TileMode, SkShader::kRepeat_TileMode);
+        m_pattern = SkShader::CreateBitmapShader(image->bitmap(), SkShader::kRepeat_TileMode, SkShader::kRepeat_TileMode);
 
     else {
 
@@ -83,11 +83,11 @@ PlatformPatternPtr Pattern::platformPattern(const AffineTransform& patternTransf
         // original, then copy the orignal into it.
         // FIXME: Is there a better way to pad (not scale) an image in skia?
         SkBitmap bm2;
-        bm2.setConfig(bm->config(), bm->width() + expandW, bm->height() + expandH);
+        bm2.setConfig(image->bitmap().config(), image->bitmap().width() + expandW, image->bitmap().height() + expandH);
         bm2.allocPixels();
         bm2.eraseARGB(0x00, 0x00, 0x00, 0x00);
         SkCanvas canvas(bm2);
-        canvas.drawBitmap(*bm, 0, 0);
+        canvas.drawBitmap(image->bitmap(), 0, 0);
         m_pattern = SkShader::CreateBitmapShader(bm2, tileModeX, tileModeY);
     }
     m_pattern->setLocalMatrix(m_patternSpaceTransformation);
index ce8b27ced5a34c05ea72a007ef6b056e0c3cca79..7b4847f473a8355422688e00dfa2ecfbb6ad597a 100644 (file)
@@ -154,7 +154,7 @@ namespace WebCore {
         inline PixelData* getAddr(int x, int y)
         {
 #if USE(SKIA)
-            return m_bitmap.getAddr32(x, y);
+            return m_bitmap.bitmap().getAddr32(x, y);
 #elif PLATFORM(QT)
             m_image = m_pixmap.toImage();
             m_pixmap = QPixmap();
index 724faeb1fd0e46f86fe50530297174f8afa9f2c5..9397ad5c3007827924e4ff903fd40bac06495c43 100644 (file)
@@ -47,7 +47,7 @@ ImageFrame& ImageFrame::operator=(const ImageFrame& other)
     m_bitmap = other.m_bitmap;
     // Keep the pixels locked since we will be writing directly into the
     // bitmap throughout this object's lifetime.
-    m_bitmap.lockPixels();
+    m_bitmap.bitmap().lockPixels();
     setOriginalFrameRect(other.originalFrameRect());
     setStatus(other.status());
     setDuration(other.duration());
@@ -58,7 +58,7 @@ ImageFrame& ImageFrame::operator=(const ImageFrame& other)
 
 void ImageFrame::clearPixelData()
 {
-    m_bitmap.reset();
+    m_bitmap.bitmap().reset();
     m_status = FrameEmpty;
     // NOTE: Do not reset other members here; clearFrameBufferCache()
     // calls this to free the bitmap data, but other functions like
@@ -68,7 +68,7 @@ void ImageFrame::clearPixelData()
 
 void ImageFrame::zeroFillPixelData()
 {
-    m_bitmap.eraseARGB(0, 0, 0, 0);
+    m_bitmap.bitmap().eraseARGB(0, 0, 0, 0);
 }
 
 bool ImageFrame::copyBitmapData(const ImageFrame& other)
@@ -76,9 +76,9 @@ bool ImageFrame::copyBitmapData(const ImageFrame& other)
     if (this == &other)
         return true;
 
-    m_bitmap.reset();
+    m_bitmap.bitmap().reset();
     const NativeImageSkia& otherBitmap = other.m_bitmap;
-    return otherBitmap.copyTo(&m_bitmap, otherBitmap.config());
+    return otherBitmap.bitmap().copyTo(&m_bitmap.bitmap(), otherBitmap.bitmap().config());
 }
 
 bool ImageFrame::setSize(int newWidth, int newHeight)
@@ -86,8 +86,8 @@ bool ImageFrame::setSize(int newWidth, int newHeight)
     // This function should only be called once, it will leak memory
     // otherwise.
     ASSERT(width() == 0 && height() == 0);
-    m_bitmap.setConfig(SkBitmap::kARGB_8888_Config, newWidth, newHeight);
-    if (!m_bitmap.allocPixels())
+    m_bitmap.bitmap().setConfig(SkBitmap::kARGB_8888_Config, newWidth, newHeight);
+    if (!m_bitmap.bitmap().allocPixels())
         return false;
 
     zeroFillPixelData();
@@ -102,12 +102,12 @@ NativeImagePtr ImageFrame::asNewNativeImage() const
 
 bool ImageFrame::hasAlpha() const
 {
-    return !m_bitmap.isOpaque();
+    return !m_bitmap.bitmap().isOpaque();
 }
 
 void ImageFrame::setHasAlpha(bool alpha)
 {
-    m_bitmap.setIsOpaque(!alpha);
+    m_bitmap.bitmap().setIsOpaque(!alpha);
 }
 
 void ImageFrame::setColorProfile(const ColorProfile& colorProfile)
@@ -124,12 +124,12 @@ void ImageFrame::setStatus(FrameStatus status)
 
 int ImageFrame::width() const
 {
-    return m_bitmap.width();
+    return m_bitmap.bitmap().width();
 }
 
 int ImageFrame::height() const
 {
-    return m_bitmap.height();
+    return m_bitmap.bitmap().height();
 }
 
 } // namespace WebCore
index 52e29009b4fc94b0214007dfbf07ef37d429c7c4..ef6342bfb76d926a5effc614a84a11923c66b2e9 100644 (file)
@@ -1,3 +1,24 @@
+2011-08-25  John Bates  <jbates@google.com>
+
+        [chromium] Leaking SkBitmaps for background images
+        https://bugs.webkit.org/show_bug.cgi?id=66488
+
+        Reviewed by Stephen White.
+
+        This patch simply changes NativeImageSkia to have a SkBitmap instead of
+        deriving from SkBitmap. All dependent code updated to access the member
+        instead of calling SkBitmap methods on NativeImageSkia objects. This
+        may or may not fix the memory leak, but it's definitely a bug that could
+        cause memory leaks.
+
+        * src/PlatformBridge.cpp:
+        (WebCore::PlatformBridge::clipboardWriteImage):
+        * src/WebImageDecoder.cpp:
+        (WebKit::WebImageDecoder::getFrameAtIndex):
+        * src/WebImageSkia.cpp:
+        (WebKit::WebImage::fromData):
+        (WebKit::WebImage::operator=):
+
 2011-08-25  Brian Salomon  <bsalomon@google.com>
 
         [SKIA] Move forward decl of skia type outside namespace Webkit
index 7d21842046415a0a3927a019f5b39c3d7edb7097..37c85e97169426d49f70feb60a430e16c44c8c8d 100644 (file)
@@ -209,7 +209,7 @@ void PlatformBridge::clipboardWriteImage(NativeImagePtr image,
                                          const String& title)
 {
 #if WEBKIT_USING_SKIA
-    WebImage webImage(*image);
+    WebImage webImage(image->bitmap());
 #else
     WebImage webImage(image);
 #endif
index 08153ba0884759f424860dff149dd2115709c0f2..7d9baa0f0655079900e32a579cb486361e0c254f 100644 (file)
@@ -113,7 +113,7 @@ WebImage WebImageDecoder::getFrameAtIndex(int index = 0) const
         return WebImage();
 #if WEBKIT_USING_SKIA
     OwnPtr<NativeImageSkia> image = adoptPtr(frameBuffer->asNewNativeImage());
-    return WebImage(*image);
+    return WebImage(image->bitmap());
 #elif WEBKIT_USING_CG
     // FIXME: Implement CG side of this.
     return WebImage(frameBuffer->asNewNativeImage());
index 026bcdeed5cbbc582a6abd5575e4a805f72b7cb5..1e6f4d12e8ab7b7c0fec1e7219e8423dd41467d6 100644 (file)
@@ -81,7 +81,7 @@ WebImage WebImage::fromData(const WebData& data, const WebSize& desiredSize)
     if (!frame)
         return WebImage();
 
-    return WebImage(*frame);
+    return WebImage(frame->bitmap());
 }
 
 void WebImage::reset()
@@ -113,7 +113,7 @@ WebImage& WebImage::operator=(const PassRefPtr<Image>& image)
 {
     NativeImagePtr p;
     if (image.get() && (p = image->nativeImageForCurrentFrame()))
-        assign(*p);
+        assign(p->bitmap());
     else
         reset();
     return *this;
index 8ce6fe29556f2c23c4741b0f94c15606ede8b59e..a49a7234c337ec241566e7f15488203ac7fc4492 100644 (file)
@@ -53,9 +53,9 @@ public:
         , m_size(size)
     {
         m_nativeImage = new NativeImageSkia();
-        m_nativeImage->setConfig(SkBitmap::kARGB_8888_Config,
-                                 size.width(), size.height(), 0);
-        m_nativeImage->allocPixels();
+        m_nativeImage->bitmap().setConfig(SkBitmap::kARGB_8888_Config,
+                                          size.width(), size.height(), 0);
+        m_nativeImage->bitmap().allocPixels();
     }
 
     virtual ~TestImage()
@@ -143,8 +143,8 @@ TEST(DragImageTest, CreateDragImage)
         RefPtr<TestImage> testImage(TestImage::create(IntSize(1, 1)));
         DragImageRef dragImage = createDragImageFromImage(testImage.get());
         ASSERT_TRUE(dragImage);
-        SkAutoLockPixels lock1(*dragImage), lock2(*(testImage->nativeImageForCurrentFrame()));
-        EXPECT_NE(dragImage->getPixels(), testImage->nativeImageForCurrentFrame()->getPixels());
+        SkAutoLockPixels lock1(*dragImage), lock2(testImage->nativeImageForCurrentFrame()->bitmap());
+        EXPECT_NE(dragImage->getPixels(), testImage->nativeImageForCurrentFrame()->bitmap().getPixels());
     }
 }