Bug 25659 – Calling frameCount() from BitmapImage::destroyDecodedDataIfNecessary() can cause GIF decoding

• Status: RESOLVED
• Resolution: FIXED
• Component: Images

[Simon Fraser (smfr)]

I noticed a stack where, during destruction of an HTMLImageElement, we can actually do some GIF decoding because we're trying to get frameCount():


  15 WebCore  112.00 Kb     WebCore::Document::removedLastRef() /Volumes/WebKit/WebKit.git/WebCore/dom/Document.cpp:413
  14 WebCore  112.00 Kb     WebCore::ContainerNode::removeAllChildren() /Volumes/WebKit/WebKit.git/WebCore/dom/ContainerNode.cpp:56
  13 WebCore  112.00 Kb     void WebCore::removeAllChildrenInContainer<WebCore::Node, WebCore::ContainerNode>(WebCore::ContainerNode*) /Volumes/WebKit/WebKit.git/WebCore/dom/ContainerNodeAlgorithms.h:51
  12 WebCore  112.00 Kb     WebCore::HTMLImageElement::~HTMLImageElement()
  11 WebCore  112.00 Kb     WebCore::HTMLImageLoader::~HTMLImageLoader()
  10 WebCore  112.00 Kb     WebCore::ImageLoader::~ImageLoader()
   9 WebCore  112.00 Kb     WebCore::CachedResource::removeClient(WebCore::CachedResourceClient*) /Volumes/WebKit/WebKit.git/WebCore/loader/CachedResource.cpp:166
   8 WebCore  112.00 Kb     WebCore::CachedImage::allClientsRemoved() /Volumes/WebKit/WebKit.git/WebCore/loader/CachedImage.cpp:112
   7 WebCore  112.00 Kb     WebCore::BitmapImage::resetAnimation() /Volumes/WebKit/WebKit.git/WebCore/platform/graphics/BitmapImage.cpp:378
   6 WebCore  112.00 Kb     WebCore::BitmapImage::destroyDecodedDataIfNecessary(bool) /Volumes/WebKit/WebKit.git/WebCore/platform/graphics/BitmapImage.cpp:98
   5 WebCore  112.00 Kb     WebCore::BitmapImage::frameCount() /Volumes/WebKit/WebKit.git/WebCore/platform/graphics/BitmapImage.cpp:187
   4 WebCore  112.00 Kb     WebCore::ImageSource::frameCount() const /Volumes/WebKit/WebKit.git/WebCore/platform/graphics/cg/ImageSourceCG.cpp:174
   3 ImageIO  112.00 Kb     CGImageSourceGetCount
   2 ImageIO  112.00 Kb     gifPluginImageCount
   1 libGIF.dylib  112.00 Kb     _cg_DGifOpen
   0 libSystem.B.dylib  112.00 Kb     malloc

[Peter Kasting]

I think destroyDecodedData() should be checking m_frames.size() instead of frameCount().  m_frames.size() represents how many frames we have potentially decoded (though some or all may currently be empty), while frameCount() is the size of the whole image.  We never really care how big the image theoretically is, just how much data we could actually have lying around at the moment.

[Peter Kasting]

Created attachment 30726 [details]
patch v1

This should fix it.

[Peter Kasting]

Fixed in r44237.