Bug 31009 - Crash for specific animated gif
Summary: Crash for specific animated gif
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Images (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-02 05:43 PST by Kai Koehne
Modified: 2010-06-12 15:44 PDT (History)
3 users (show)

See Also:


Attachments
Animated gif image causing the crash (294.90 KB, image/gif)
2009-11-02 05:43 PST, Kai Koehne
no flags Details
Proposed patch (as suggested by zecke on IRC) (1.58 KB, patch)
2009-11-02 07:08 PST, Kai Koehne
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kai Koehne 2009-11-02 05:43:54 PST
Created attachment 42311 [details]
Animated gif image causing the crash

This is reproducable both on Linux & Windows. Loading the attached image will crash the qt standalone demo browser.

Backtrace:

QtWebKitd4.dll!WTF::Vector<WebCore::RGBA32Buffer,0>::at(unsigned int i=87)  Line 508 + 0x29 bytes	C++
 	QtWebKitd4.dll!WTF::Vector<WebCore::RGBA32Buffer,0>::operator[](unsigned int i=87)  Line 517 + 0x13 bytes	C++
 	QtWebKitd4.dll!WebCore::ImageDecoderQt::clearFrameBufferCache(unsigned int index=87)  Line 157 + 0xf bytes	C++
 	QtWebKitd4.dll!WebCore::ImageSource::clear(bool destroyAll=false, unsigned int clearBeforeFrame=1, WebCore::SharedBuffer * data=0x0f1c8178, bool allDataReceived=true)  Line 61	C++
 	QtWebKitd4.dll!WebCore::BitmapImage::destroyDecodedData(bool destroyAll=false)  Line 91	C++
 	QtWebKitd4.dll!WebCore::BitmapImage::destroyDecodedDataIfNecessary(bool destroyAll=false)  Line 100	C++
 	QtWebKitd4.dll!WebCore::BitmapImage::internalAdvanceAnimation(bool skippingFrames=false)  Line 421	C++
 	QtWebKitd4.dll!WebCore::BitmapImage::advanceAnimation(WebCore::Timer<WebCore::BitmapImage> * __formal=0x0c9c5740)  Line 386	C++
 	QtWebKitd4.dll!WebCore::Timer<WebCore::BitmapImage>::fired()  Line 98 + 0x1f bytes	C++
 	QtWebKitd4.dll!WebCore::ThreadTimers::sharedTimerFiredInternal()  Line 115	C++
 	QtWebKitd4.dll!WebCore::ThreadTimers::sharedTimerFired()  Line 91	C++
 	QtWebKitd4.dll!WebCore::SharedTimerQt::timerEvent(QTimerEvent * ev=0x0012dab4)  Line 106	C++
 	QtCored4.dll!QObject::event(QEvent * e=0x0012dab4)  Line 1196	C++
 	QtGuid4.dll!QApplicationPrivate::notify_helper(QObject * receiver=0x0a877b18, QEvent * e=0x0012dab4)  Line 4245 + 0x11 bytes	C++
 	QtGuid4.dll!QApplication::notify(QObject * receiver=0x0a877b18, QEvent * e=0x0012dab4)  Line 3669 + 0x10 bytes	C++
 	QtCored4.dll!QCoreApplication::notifyInternal(QObject * receiver=0x0a877b18, QEvent * event=0x0012dab4)  Line 704 + 0x15 bytes	C++
 	QtCored4.dll!QCoreApplication::sendEvent(QObject * receiver=0x0a877b18, QEvent * event=0x0012dab4)  Line 215 + 0x39 bytes	C++
 	QtCored4.dll!QEventDispatcherWin32Private::sendTimerEvent(int timerId=16777254)  Line 589 + 0x10 bytes	C++
 	QtCored4.dll!qt_internal_proc(HWND__ * hwnd=0x001e0610, unsigned int message=275, unsigned int wp=16777254, long lp=0)  Line 489	C++
....
Comment 1 Holger Freyther 2009-11-02 05:51:44 PST
A classic off by one... The method is called to clean frames before the one given in the index... but we are counting up to the index.

The other part is that the QGif handler is broken and will not allow to jump to a given so once we have evicted the frame we will not be able to load it.

For now I think it is best to remove the implementation in ImageDecoderQt::clearFrameBufferCache and deal with the extra memory allocated...
Comment 2 Kai Koehne 2009-11-02 07:08:57 PST
Created attachment 42316 [details]
Proposed patch (as suggested by zecke on IRC)
Comment 3 Holger Freyther 2009-11-02 07:16:04 PST
Comment on attachment 42316 [details]
Proposed patch (as suggested by zecke on IRC)

Normally we don't put unused parameters in comments, on the other hand there is existing code that is doing it... After the bot has landed the change I will reopen the bug as we should attempt to safe memory, specially on big animations...
Comment 4 WebKit Commit Bot 2009-11-02 09:53:42 PST
Comment on attachment 42316 [details]
Proposed patch (as suggested by zecke on IRC)

Clearing flags on attachment: 42316

Committed r50413: <http://trac.webkit.org/changeset/50413>
Comment 5 WebKit Commit Bot 2009-11-02 09:53:51 PST
All reviewed patches have been landed.  Closing bug.
Comment 6 Holger Freyther 2009-11-02 17:38:03 PST
Reopened as Qt will need to implement this function to save memory with big GIF animations.
Comment 7 Alexey Proskuryakov 2010-06-12 15:44:45 PDT
Implementing this function is now tracked as bug 32121.