Bug 25659

Summary: Calling frameCount() from BitmapImage::destroyDecodedDataIfNecessary() can cause GIF decoding
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: ImagesAssignee: Peter Kasting <pkasting>
Status: RESOLVED FIXED    
Severity: Normal CC: pkasting
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
patch v1 simon.fraser: review+

Simon Fraser (smfr)
Reported 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
Attachments
patch v1 (1.26 KB, patch)
2009-05-27 18:19 PDT, Peter Kasting 		simon.fraser: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Peter Kasting
Comment 1 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.
Peter Kasting
Comment 2 2009-05-27 18:19:18 PDT
Created attachment 30726 [details] patch v1 This should fix it.
Peter Kasting
Comment 3 2009-05-28 10:29:31 PDT
Fixed in r44237.
Note You need to log in before you can comment on or make changes to this bug.