Bug 25659 - Calling frameCount() from BitmapImage::destroyDecodedDataIfNecessary() can cause GIF decoding
Summary: Calling frameCount() from BitmapImage::destroyDecodedDataIfNecessary() can ca...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Images (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Peter Kasting
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-05-08 21:08 PDT by Simon Fraser (smfr)
Modified: 2009-05-28 10:29 PDT (History)
1 user (show)

See Also:

Attachments
patch v1 (1.26 KB, patch)
2009-05-27 18:19 PDT, Peter Kasting 		simon.fraser: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2009-05-08 21:08:02 PDT 
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
Comment 1 Peter Kasting 2009-05-27 17:25:23 PDT 
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.
Comment 2 Peter Kasting 2009-05-27 18:19:18 PDT 
Created attachment 30726 [details]
patch v1

This should fix it.
Comment 3 Peter Kasting 2009-05-28 10:29:31 PDT 
Fixed in r44237.