Bug 93764

Summary: Skipping frames in animated GIFs (or other multiframe images) can cause inifinite loop in WebCore::BitmapImage::startAnimation()
Product: WebKit Reporter: Tobias Netzel <tobias.netzel>
Component: ImagesAssignee: Nobody <webkit-unassigned>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: ap
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
don't force decoding of skipped frames
none
prepared for review
none
patch format as produced by the scripts none

Tobias Netzel
Reported 2012-08-11 03:10:52 PDT
Created attachment 157864 [details] don't force decoding of skipped frames The current code for frame skipping in WebCore::BitmapImage::startAnimation() forces decoding of skipped frames by calling frameIsCompleteAtIndex() and frameDurationAtIndex(). On slow or VERY busy machines and animated images larger than 5MB ( ->cLargeAnimationCutoff) that will cause the for loop to be started over and over again without displaying any frames in case decoding the frame takes more time than the frame's duration. Effectively the application will seem to hang. In the patch I made that frame skipping loop do it the same way as the loop in BitmapImage::dataChanged(); the comment in dataChanged() is also valid for the frame skipping loop.
Attachments
don't force decoding of skipped frames (1.30 KB, patch)
2012-08-11 03:10 PDT, Tobias Netzel 		no flags
DetailsFormatted DiffDiff
prepared for review (2.16 KB, patch)
2012-08-13 12:32 PDT, Tobias Netzel 		no flags
DetailsFormatted DiffDiff
patch format as produced by the scripts (1.93 KB, patch)
2013-01-17 02:02 PST, Tobias Netzel 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1 2012-08-13 09:55:01 PDT
This patch is for a branch. Would you be willing to post a patch against truck for review, following <http://www.webkit.org/coding/contributing.html>? Notably, the patch misses a ChangeLog.
Tobias Netzel
Comment 2 2012-08-13 12:32:47 PDT
Created attachment 158074 [details] prepared for review Added a ChangeLog entry. Should be covered by existing tests. I didn't run the tests and I don't have the resources to do so. I found this website which should be enough stress testing for animated GIFs: http://twistedsifter.com/2012/06/high-quality-animated-gifs-by-micael-reynaud/
Tobias Netzel
Comment 3 2012-11-23 05:42:09 PST
May I kindly ask for review? Thank you!
Tobias Netzel
Comment 4 2013-01-17 01:55:36 PST
Is there anything I can do to get someone providing some feedback?
Tobias Netzel
Comment 5 2013-01-17 02:02:56 PST
Created attachment 183150 [details] patch format as produced by the scripts
Tobias Netzel
Comment 6 2013-06-25 12:48:02 PDT
Finally someone else fixed this - but why was it impossible to get this patch committed? *** This bug has been marked as a duplicate of bug 116041 ***
Note You need to log in before you can comment on or make changes to this bug.