2011-02-15 Ian Henderson <ianh@apple.com>
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 16 Feb 2011 01:15:20 +0000 (01:15 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 16 Feb 2011 01:15:20 +0000 (01:15 +0000)
        Reviewed by Darin Adler.

        To determine image properties, CG allocates memory which isn't included in CachedImage's decoded size
        https://bugs.webkit.org/show_bug.cgi?id=53281

        When determining properties of an image (such as its size), CG ends up
        decoding part of the image.  This patch adds accounting for this extra
        decoded size so a cache prune can clean up the allocations.

        * platform/graphics/BitmapImage.cpp:
        (WebCore::BitmapImage::BitmapImage):
        (WebCore::BitmapImage::destroyMetadataAndNotify):
        Clearing the source destroys the extra decoded data.  Report this
        change in decoded size to the image observer.
        (WebCore::BitmapImage::cacheFrame):
        The first decoded frame subsumes the data decoded when determining
        image properties, so we subtract it out here.
        (WebCore::BitmapImage::didDecodeProperties):
        Reports the extra decoded size to the image's observer.
        (WebCore::BitmapImage::size):
        (WebCore::BitmapImage::currentFrameSize):
        (WebCore::BitmapImage::getHotSpot):
        (WebCore::BitmapImage::frameCount):
        (WebCore::BitmapImage::isSizeAvailable):
        (WebCore::BitmapImage::repetitionCount):
        * platform/graphics/BitmapImage.h:
        * platform/graphics/ImageSource.cpp:
        (WebCore::ImageSource::bytesDecodedToDetermineProperties):
        The default value is 0 to match the current behavior on other
        platforms.
        * platform/graphics/ImageSource.h:
        * platform/graphics/cg/ImageSourceCG.cpp:
        (WebCore::ImageSource::bytesDecodedToDetermineProperties):
        Add a constant value for bytesDecodedToDetermineProperties(), measured
        by tracing malloc/calloc calls while asking an image source for its
        properties.

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

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/BitmapImage.cpp
Source/WebCore/platform/graphics/BitmapImage.h
Source/WebCore/platform/graphics/ImageSource.cpp
Source/WebCore/platform/graphics/ImageSource.h
Source/WebCore/platform/graphics/cg/ImageSourceCG.cpp

index 96a7b14..f69866b 100644 (file)
@@ -1,3 +1,42 @@
+2011-02-15  Ian Henderson  <ianh@apple.com>
+
+        Reviewed by Darin Adler.
+
+        To determine image properties, CG allocates memory which isn't included in CachedImage's decoded size
+        https://bugs.webkit.org/show_bug.cgi?id=53281
+
+        When determining properties of an image (such as its size), CG ends up
+        decoding part of the image.  This patch adds accounting for this extra
+        decoded size so a cache prune can clean up the allocations.
+
+        * platform/graphics/BitmapImage.cpp:
+        (WebCore::BitmapImage::BitmapImage):
+        (WebCore::BitmapImage::destroyMetadataAndNotify):
+        Clearing the source destroys the extra decoded data.  Report this
+        change in decoded size to the image observer.
+        (WebCore::BitmapImage::cacheFrame):
+        The first decoded frame subsumes the data decoded when determining
+        image properties, so we subtract it out here.
+        (WebCore::BitmapImage::didDecodeProperties):
+        Reports the extra decoded size to the image's observer.
+        (WebCore::BitmapImage::size):
+        (WebCore::BitmapImage::currentFrameSize):
+        (WebCore::BitmapImage::getHotSpot):
+        (WebCore::BitmapImage::frameCount):
+        (WebCore::BitmapImage::isSizeAvailable):
+        (WebCore::BitmapImage::repetitionCount):
+        * platform/graphics/BitmapImage.h:
+        * platform/graphics/ImageSource.cpp:
+        (WebCore::ImageSource::bytesDecodedToDetermineProperties):
+        The default value is 0 to match the current behavior on other
+        platforms.
+        * platform/graphics/ImageSource.h:
+        * platform/graphics/cg/ImageSourceCG.cpp:
+        (WebCore::ImageSource::bytesDecodedToDetermineProperties):
+        Add a constant value for bytesDecodedToDetermineProperties(), measured
+        by tracing malloc/calloc calls while asking an image source for its
+        properties.
+
 2011-02-15  James Robinson  <jamesr@chromium.org>
 
         Reviewed by Alexey Proskuryakov.
index 1148aa6..6027f34 100644 (file)
@@ -60,6 +60,7 @@ BitmapImage::BitmapImage(ImageObserver* observer)
     , m_sizeAvailable(false)
     , m_hasUniformFrameSize(true)
     , m_decodedSize(0)
+    , m_decodedPropertiesSize(0)
     , m_haveFrameCount(false)
     , m_frameCount(0)
 {
@@ -104,8 +105,12 @@ void BitmapImage::destroyMetadataAndNotify(int framesCleared)
     m_isSolidColor = false;
     invalidatePlatformData();
 
-    const int deltaBytes = framesCleared * -frameBytes(m_size);
+    int deltaBytes = framesCleared * -frameBytes(m_size);
     m_decodedSize += deltaBytes;
+    if (framesCleared > 0) {
+        deltaBytes -= m_decodedPropertiesSize;
+        m_decodedPropertiesSize = 0;
+    }
     if (deltaBytes && imageObserver())
         imageObserver()->decodedSizeChanged(this, deltaBytes);
 }
@@ -132,18 +137,41 @@ void BitmapImage::cacheFrame(size_t index)
     if (frameSize != m_size)
         m_hasUniformFrameSize = false;
     if (m_frames[index].m_frame) {
-        const int deltaBytes = frameBytes(frameSize);
+        int deltaBytes = frameBytes(frameSize);
         m_decodedSize += deltaBytes;
+        // The fully-decoded frame will subsume the partially decoded data used
+        // to determine image properties.
+        deltaBytes -= m_decodedPropertiesSize;
+        m_decodedPropertiesSize = 0;
         if (imageObserver())
             imageObserver()->decodedSizeChanged(this, deltaBytes);
     }
 }
 
+void BitmapImage::didDecodeProperties() const
+{
+    if (m_decodedSize)
+        return;
+    size_t updatedSize = m_source.bytesDecodedToDetermineProperties();
+    if (m_decodedPropertiesSize == updatedSize)
+        return;
+    int deltaBytes = updatedSize - m_decodedPropertiesSize;
+#ifndef NDEBUG
+    bool overflow = updatedSize > m_decodedPropertiesSize && deltaBytes < 0;
+    bool underflow = updatedSize < m_decodedPropertiesSize && deltaBytes > 0;
+    ASSERT(!overflow && !underflow);
+#endif
+    m_decodedPropertiesSize = updatedSize;
+    if (imageObserver())
+        imageObserver()->decodedSizeChanged(this, deltaBytes);
+}
+
 IntSize BitmapImage::size() const
 {
     if (m_sizeAvailable && !m_haveSize) {
         m_size = m_source.size();
         m_haveSize = true;
+        didDecodeProperties();
     }
     return m_size;
 }
@@ -152,12 +180,16 @@ IntSize BitmapImage::currentFrameSize() const
 {
     if (!m_currentFrame || m_hasUniformFrameSize)
         return size();
-    return m_source.frameSizeAtIndex(m_currentFrame);
+    IntSize frameSize = m_source.frameSizeAtIndex(m_currentFrame);
+    didDecodeProperties();
+    return frameSize;
 }
 
 bool BitmapImage::getHotSpot(IntPoint& hotSpot) const
 {
-    return m_source.getHotSpot(hotSpot);
+    bool result = m_source.getHotSpot(hotSpot);
+    didDecodeProperties();
+    return result;
 }
 
 bool BitmapImage::dataChanged(bool allDataReceived)
@@ -190,6 +222,7 @@ size_t BitmapImage::frameCount()
     if (!m_haveFrameCount) {
         m_haveFrameCount = true;
         m_frameCount = m_source.frameCount();
+        didDecodeProperties();
     }
     return m_frameCount;
 }
@@ -200,6 +233,7 @@ bool BitmapImage::isSizeAvailable()
         return true;
 
     m_sizeAvailable = m_source.isSizeAvailable();
+    didDecodeProperties();
 
     return m_sizeAvailable;
 }
@@ -256,6 +290,7 @@ int BitmapImage::repetitionCount(bool imageKnownToBeComplete)
         // decoder will default to cAnimationLoopOnce, and we'll try and read
         // the count again once the whole image is decoded.
         m_repetitionCount = m_source.repetitionCount();
+        didDecodeProperties();
         m_repetitionCountStatus = (imageKnownToBeComplete || m_repetitionCount == cAnimationNone) ? Certain : Uncertain;
     }
     return m_repetitionCount;
index e9678c1..07cb12a 100644 (file)
@@ -211,6 +211,12 @@ protected:
     // Whether or not size is available yet.    
     bool isSizeAvailable();
 
+    // Called after asking the source for any information that may require
+    // decoding part of the image (e.g., the image size).  We need to report
+    // the partially decoded data to our observer so it has an accurate
+    // account of the BitmapImage's memory usage.
+    void didDecodeProperties() const;
+
     // Animation.
     int repetitionCount(bool imageKnownToBeComplete);  // |imageKnownToBeComplete| should be set if the caller knows the entire image has been decoded.
     bool shouldAnimate();
@@ -277,6 +283,7 @@ protected:
     mutable bool m_hasUniformFrameSize;
 
     unsigned m_decodedSize; // The current size of all decoded frames.
+    mutable unsigned m_decodedPropertiesSize; // The size of data decoded by the source to determine image properties (e.g. size, frame count, etc).
 
     mutable bool m_haveFrameCount;
     size_t m_frameCount;
index 984b7d2..f4495ed 100644 (file)
@@ -115,6 +115,11 @@ bool ImageSource::getHotSpot(IntPoint&) const
     return false;
 }
 
+size_t ImageSource::bytesDecodedToDetermineProperties() const
+{
+    return 0;
+}
+
 int ImageSource::repetitionCount()
 {
     return m_decoder ? m_decoder->repetitionCount() : cAnimationNone;
index 3f16d07..dff803f 100644 (file)
@@ -169,6 +169,8 @@ public:
     IntSize frameSizeAtIndex(size_t) const;
     bool getHotSpot(IntPoint&) const;
 
+    size_t bytesDecodedToDetermineProperties() const;
+
     int repetitionCount();
 
     size_t frameCount() const;
index 4ed8684..fc7cd3e 100644 (file)
@@ -222,6 +222,17 @@ bool ImageSource::getHotSpot(IntPoint& hotSpot) const
     return true;
 }
 
+size_t ImageSource::bytesDecodedToDetermineProperties() const
+{
+    // Measured by tracing malloc/calloc calls on Mac OS 10.6.6, x86_64.
+    // A non-zero value ensures cached images with no decoded frames still enter
+    // the live decoded resources list when the CGImageSource decodes image
+    // properties, allowing the cache to prune the partially decoded image.
+    // This value is likely to be inaccurate on other platforms, but the overall
+    // behavior is unchanged.
+    return 13088;
+}
+    
 int ImageSource::repetitionCount()
 {
     int result = cAnimationLoopOnce; // No property means loop once.