Bug 22108

Additionally, if I reload the image while it is already standalone in its own tab (and is actually animating), the animation fails to happen. I have to repeat the context-menu-click-and-move-to-new-tab action to get it to animate again.

Can you find a regression window for this?

See also: bug 22029, bug 22154.

*** Bug 22154 has been marked as a duplicate of this bug. ***

*** Bug 22029 has been marked as a duplicate of this bug. ***

From the duped bugs, here are other pages that exhibit problems: http://radar.weather.gov/ridge/radar_lite.php?rid=ICT&product=NCR&loop=yes http://hurricane.accuweather.com/hurricane/satellite.asp?region=hpac&anim=1&type=ei&basin=epacific Verified all three pages' looping images are GIFs. It is not clear from reading these comments if the problem is that the images fail to loop (i.e. they animate exactly once) or if they fail to animate at all (only the initial frame shows).

After clicking Animate there is no visible animation, but you do get an additional frame in the "Show the previous page" sequence. It looks like it might be the first frame in the animation sequence.

(In reply to comment #6) > It is not clear from > reading these comments if the problem is that the images fail to loop (i.e. > they animate exactly once) or if they fail to animate at all (only the initial > frame shows). in my experience, only the first frame shows. very rarely the image will animate (no obvious cause of success), and when it does it loops correctly.

This bug seems sporadic, and doesn't happen in simple test cases. When a complex page with an animated GIF loads, the GIF doesn't always start animating. However, if you cause a redraw, or load the image in its own tab, then it animates fine. Can someone try to whip up a testcase that is simpler than the URLs listed above?

(In reply to comment #2) > Can you find a regression window for this? testing the nightlies, it looks like this problem surfaced between (r37605,r37469]. pretty big window, but narrows it down a bit.

Did you type those numbers correctly? The first is larger than the second. I'm especially curious as to whether r37612 affected this.

(In reply to comment #11) > Did you type those numbers correctly? The first is larger than the second. unfortunately yes. from http://nightly.webkit.org/builds/trunk/mac/1 , the builds for r37605 and r37469 are adjacent.

Adjacent, in descending order. So the range is (r37469,r37605], assuming you're saying the problem reproduces in the later build and not the earlier. I was trying to figure out if there was a typo. That sounds like r37612 is not at fault, which surprises me slightly.

(In reply to comment #13) > Adjacent, in descending order. So the range is (r37469,r37605], assuming > you're saying the problem reproduces in the later build and not the earlier. I > was trying to figure out if there was a typo. sorry, you are correct. > That sounds like r37612 is not at fault, which surprises me slightly. well, I can't claim 100% accuracy, either. r37605 works more often than not. I only flagged it because of one animation failure (out of about 48 different tests). that may have been a false positive as it works with all of the other cases that r37698 fails on.

I use wunderground.com as well and I can confirm that of the nightlies r37605 and previous work normally, but r37698 and newer do not animate and only show the first frame. I am currently checking the code out of svn to do intermediate builds to narrow down which revision breaks the animation.

To update my previous post, I've done a few intermediate builds and it appears that r37612 is the responsible party for breaking these animations. The animated weather radar on wunderground.com works normally in r37611 but does not animate and only shows the first frame in r37612. I'm not familiar enough with the WebKit code base to dig into the code itself, but please let me know if there is any more detail I can provide.

The patch from bug #19663 may have caused this regression. We still need a reduced sample case; I'll take a stab at it.

*** Bug 21894 has been marked as a duplicate of this bug. ***

<rdar://problem/6388383>

Could it be that the same routines are used and causing similar problems? http://radar.weather.gov/Conus/full_loop.php This site does not animate when load is complete. Using nightly build r38592

Still no animation with build r28645 http://www.wunderground.com/radar/mixedcomposite.asp?region=a5&size=2x&type=loop http://radar.weather.gov/Conus/full_loop.php

I'm looking at something like this on the Chromium side at the moment, though it manifests a bit differently (looping images fail to loop). I think the underlying problem may be the same, but it's tricky to tell because the two have different ImageSource implementations/interactions with the underlying decoders. The basic problem is that for large images, we destroy the decoder as we go, and keep re-setting the data on it. My concern is that important state like the repetition count, or perhaps the frame count (?) is then basically uninitialized, and BitmapImage can ask the source for it and get garbage, leading to problems. In Chromium's case, the underlying GIF decoder (available in the WebKit tree as GIFImage[Decoder,Reader].cpp) has a repetitionCount() function that simply returns the value of a member variable, which itself is not set unless we happen to run across the repetition count extension block while decoding. So if we destroy the decoder, the value returned from this function can be wrong for an arbitrary time. I'm seeing cases where it is, in fact, wrong, and BitmapImage erroneously believes the image loops once instead of looping forever. This doesn't explain the CG bug described here; I'm wondering if perhaps in that case the frame count is wrong for the same reason, or something. hyatt isn't around to chime in and I can't see into the CG decoder's source. There are possible fixes on the Chromium side, but all involve modifying the decoder. I can't add new functions to the CG decoder (and don't have a reference to the existing set to see if I'd need to), so I'm not sure how best to address this.

Created attachment 25693 [details] patch v1 hyatt and I have already chatted some on IRC about this patch. This fixes animations by not deleting the decoder, meaning the loop count can stay valid across frames. I didn't actually determine for sure that this was the problem in CG WebKit, but it was the problem in Chromium, and patching the fix in here made the testcases seem to work in my Safari Win build. It turns out the CG decoder was already doing a pretty good job of dropping frames when their external references were released, and of deleting frames internally during re-decoding, so it didn't suffer from the peak memory issues Cairo/Chromium did, and the old "delete ImageDecoder" code wasn't really necessary for it at all. Unfortunately it also decodes from the beginning of the stream more than it needs to, so it still uses far more CPU than it should (i.e. this patch does not fix the underlying problem in bug 22280). Perhaps hyatt can use the framework in this patch to help address things.

Comment on attachment 25693 [details] patch v1 r=me

Fixed in r39309.

Why has this been marked as resolved? I'm using r39469 and weather radar loops from weather.gov still don't animate. They didn't work with for me with r39309 or any subsequent builds either.

This is marked as resolved because a patch that fixed some or all problems was committed. Please file a new bug if you are still seeing problems, and describe them in detail.

(In reply to comment #27) > This is marked as resolved because a patch that fixed some or all problems was > committed. Please file a new bug if you are still seeing problems, and describe > them in detail. > This site still exhibits the no animation problem http://radar.weather.gov/Conus/full_loop.php Using WebKit-SVN-r39469

As I said, please file a new bug. Also, this weather.gov page is different from the one discussed here (http://radar.weather.gov/ridge/radar_lite.php?rid=ICT&product=NCR&loop=yes), and it doesn't work well even in shipping Safari/WebKit for me.

See also bug 23082.

And bug 22647.

*** Bug 22647 has been marked as a duplicate of this bug. ***

Bug 22995 tracks remaining issues.