Bug 23684 - Endless loop for image with zero duration frame
Summary: Endless loop for image with zero duration frame
Status: RESOLVED CONFIGURATION CHANGED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Images (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar, Regression
Depends on:
Blocks:
 
Reported: 2009-02-02 10:00 PST by Lyon Chen
Modified: 2021-11-29 16:59 PST (History)
4 users (show)

See Also:


Attachments
Repeating GIF animation where first and third frames have a one second delay but second frame has a zero second delay. (256 bytes, image/gif)
2015-08-02 15:13 PDT, Jeremy Zerfas
no flags Details
Repeating GIF animation where all frames have a zero second delay. (256 bytes, image/gif)
2015-08-02 15:13 PDT, Jeremy Zerfas
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Lyon Chen 2009-02-02 10:00:17 PST
In function BitmapImage::startAnimation(), there's a potential endless loop when the duration of frames is zero, which can be observed in aircanada.com for its searching page.

Following is the verified fix:

diff --git a/WebCore/platform/graphics/BitmapImage.cpp b/WebCore/platform/graphics/BitmapImage.cpp
index 45b32ab..e3ef8b0 100644
--- a/WebCore/platform/graphics/BitmapImage.cpp
+++ b/WebCore/platform/graphics/BitmapImage.cpp
@@ -323,6 +323,10 @@ void BitmapImage::startAnimation(bool catchUpIfNecessary)
             if (time < frameAfterNextStartTime)
                 break;
 
+            /* Is the frame duration is zero, which means we should not animate! */
+            if (frameAfterNextStartTime == m_desiredFrameStartTime)
+                break;
+
             // Yes; skip over it without notifying our observers.
             if (!internalAdvanceAnimation(true))
                 return;
Comment 1 Mark Rowe (bdash) 2009-02-02 12:47:44 PST
<rdar://problem/6548504>
Comment 2 mitz 2009-02-02 13:01:19 PST
Can you attach an image that causes the endless looping?
Comment 3 Lyon Chen 2009-02-02 13:20:49 PST
I got into this situation due to another bug in my own code. I think just in case such a situation occurs it's better to have a way out. But I don't know any such animated GIF image (with zero duration frames) out in the wild.

Comment 4 mitz 2009-02-02 13:24:29 PST
Peter, can this occur or does a "0-duration frame" always indicate a programmer error or a bug in the image decoder (in which case there should be an ASSERT)?
Comment 5 Adam Treat 2009-02-02 13:25:01 PST
I think it is important to note that a malicious gif could be constructed to have this feature though.
Comment 6 mitz 2009-02-02 13:26:18 PST
(In reply to comment #5)
> I think it is important to note that a malicious gif could be constructed to
> have this feature though.

Please construct one for the regression test.
Comment 7 Peter Kasting 2009-02-02 13:32:19 PST
I'd need to see a sample GIF where this happens to know what the effects are.  You can probably hack an existing one and just force 0 into some frames' duration slots.

The reason I say this is because there's already a bunch of code elsewhere that enforces minimum frame durations, and I don't know whether that's already run by the time we get here.  So it's not clear to me whether this can really be a problem.  Plus, I thought I tested some real ad GIFs with 0 frame durations and didn't have problems?  But my memory could be bad.
Comment 8 Jeremy Zerfas 2015-08-02 15:10:58 PDT
This ticket has been open for several years and I just thought I would point out that although this bug seems to have been valid back then, some of the relevant code has been refactored since then and there is no longer a potential for an endless loop. In the current code, if the containing loop doesn't exit the function, then on each iteration of that loop it instead increments frameAfterNextStartTime by the frameDurationAtIndex() function (which normally never returns a value less than 0.011 seconds) and eventually causes the loop to end.

I'm attaching two GIFs I created that contain zero duration frames in case anyone wants them for testing. Both GIFs contain a repeating animation that shows the numbers 1, 2, and 3 after each other. In "Animated GIF with Single Zero Duration Frame.gif", the first and third frames have a one second delay but the second frame has a zero second delay. In "Animated GIF with All Zero Duration Frames.gif" all the frames have zero second delays.

It looks like zero duration frames are allowed by GIFs and do have some valid uses, see http://www.imagemagick.org/Usage/anim_basics/#zero . Mozilla added some code back in late 2013 to respect those zero duration frames for non-looping GIFs but it looks like they just recently found out that the new code is causing problems with some (poorly made) GIFs on the web and will be removing that code, see https://bugzilla.mozilla.org/show_bug.cgi?id=890743 .
Comment 9 Jeremy Zerfas 2015-08-02 15:13:18 PDT
Created attachment 258042 [details]
Repeating GIF animation where first and third frames have a one second delay but second frame has a zero second delay.
Comment 10 Jeremy Zerfas 2015-08-02 15:13:56 PDT
Created attachment 258043 [details]
Repeating GIF animation where all frames have a zero second delay.
Comment 11 mitz 2021-11-29 16:59:50 PST
Marking as resolved based on the observation that an infinite loop can no longer occur here.