93764 2012-08-11 03:10:52 -0700 Skipping frames in animated GIFs (or other multiframe images) can cause inifinite loop in WebCore::BitmapImage::startAnimation() 2013-06-25 13:39:37 -0700 1 1 1 Unclassified WebKit Images 528+ (Nightly build) All All RESOLVED DUPLICATE 116041 P2 Normal --- 0 tobias.netzel webkit-unassigned ap oldest_to_newest 693088 0 157864 tobias.netzel 2012-08-11 03:10:52 -0700 Created attachment 157864 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. 693759 1 ap 2012-08-13 09:55:01 -0700 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. 693954 2 158074 tobias.netzel 2012-08-13 12:32:47 -0700 Created attachment 158074 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/ 774481 3 tobias.netzel 2012-11-23 05:42:09 -0800 May I kindly ask for review? Thank you! 809197 4 tobias.netzel 2013-01-17 01:55:36 -0800 Is there anything I can do to get someone providing some feedback? 809203 5 183150 tobias.netzel 2013-01-17 02:02:56 -0800 Created attachment 183150 patch format as produced by the scripts 903406 6 tobias.netzel 2013-06-25 12:48:02 -0700 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 *** 157864 2012-08-11 03:10:52 -0700 2012-08-13 12:32:47 -0700 don't force decoding of skipped frames BitmapImage.patch text/plain 1329 tobias.netzel SW5kZXg6IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL0JpdG1hcEltYWdlLmNwcAo9 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09CmRpZmYgLS1naXQgYS90YWdzL1NhZmFyaS01MzcuMy4xL1NvdXJjZS9XZWJDb3Jl L3BsYXRmb3JtL2dyYXBoaWNzL0JpdG1hcEltYWdlLmNwcCBiL3RhZ3MvU2FmYXJpLTUzNy4zLjEv U291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvQml0bWFwSW1hZ2UuY3BwCi0tLSBhL3Rh Z3MvU2FmYXJpLTUzNy4zLjEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvQml0bWFw SW1hZ2UuY3BwCShSZXZpc2lvbiAxMjQ4ODMpCisrKyBiL3RhZ3MvU2FmYXJpLTUzNy4zLjEvU291 cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvQml0bWFwSW1hZ2UuY3BwCShBcmJlaXRza29w aWUpCkBAIC00MzEsOSArNDMxLDkgQEAKICAgICAgICAgLy8gU2VlIGlmIHdlJ3ZlIGFsc28gcGFz c2VkIHRoZSB0aW1lIGZvciBmcmFtZXMgYWZ0ZXIgdGhhdCB0byBzdGFydCwgaW4KICAgICAgICAg Ly8gY2FzZSB3ZSBuZWVkIHRvIHNraXAgc29tZSBmcmFtZXMgZW50aXJlbHkuICBSZW1lbWJlciBu b3QgdG8gYWR2YW5jZQogICAgICAgICAvLyB0byBhbiBpbmNvbXBsZXRlIGZyYW1lLgotICAgICAg ICBmb3IgKHNpemVfdCBmcmFtZUFmdGVyTmV4dCA9IChuZXh0RnJhbWUgKyAxKSAlIGZyYW1lQ291 bnQoKTsgZnJhbWVJc0NvbXBsZXRlQXRJbmRleChmcmFtZUFmdGVyTmV4dCk7IGZyYW1lQWZ0ZXJO ZXh0ID0gKG5leHRGcmFtZSArIDEpICUgZnJhbWVDb3VudCgpKSB7CisgICAgICAgIGZvciAoc2l6 ZV90IGZyYW1lQWZ0ZXJOZXh0ID0gKG5leHRGcmFtZSArIDEpICUgZnJhbWVDb3VudCgpOyBtX2Zy YW1lc1tmcmFtZUFmdGVyTmV4dF0ubV9pc0NvbXBsZXRlOyBmcmFtZUFmdGVyTmV4dCA9IChuZXh0 RnJhbWUgKyAxKSAlIGZyYW1lQ291bnQoKSkgewogICAgICAgICAgICAgLy8gU2hvdWxkIHdlIHNr aXAgdGhlIG5leHQgZnJhbWU/Ci0gICAgICAgICAgICBkb3VibGUgZnJhbWVBZnRlck5leHRTdGFy dFRpbWUgPSBtX2Rlc2lyZWRGcmFtZVN0YXJ0VGltZSArIGZyYW1lRHVyYXRpb25BdEluZGV4KG5l eHRGcmFtZSk7CisgICAgICAgICAgICBkb3VibGUgZnJhbWVBZnRlck5leHRTdGFydFRpbWUgPSBt X2Rlc2lyZWRGcmFtZVN0YXJ0VGltZSArIG1fZnJhbWVzW25leHRGcmFtZV0ubV9kdXJhdGlvbjsK ICAgICAgICAgICAgIGlmICh0aW1lIDwgZnJhbWVBZnRlck5leHRTdGFydFRpbWUpCiAgICAgICAg ICAgICAgICAgYnJlYWs7CiAK 158074 2012-08-13 12:32:47 -0700 2013-01-17 02:02:56 -0800 prepared for review BitmapImage_v2.patch text/plain 2209 tobias.netzel SW5kZXg6IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL0JpdG1hcEltYWdlLmNwcAo9 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09CmRpZmYgLS1naXQgYS90cnVuay9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFw aGljcy9CaXRtYXBJbWFnZS5jcHAgYi90cnVuay9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFw aGljcy9CaXRtYXBJbWFnZS5jcHAKLS0tIGEvdHJ1bmsvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0v Z3JhcGhpY3MvQml0bWFwSW1hZ2UuY3BwCShSZXZpc2lvbiAxMjU0MzApCisrKyBiL3RydW5rL1Nv dXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL0JpdG1hcEltYWdlLmNwcAkoQXJiZWl0c2tv cGllKQpAQCAtNDM3LDkgKzQzNyw5IEBACiAgICAgICAgIC8vIFNlZSBpZiB3ZSd2ZSBhbHNvIHBh c3NlZCB0aGUgdGltZSBmb3IgZnJhbWVzIGFmdGVyIHRoYXQgdG8gc3RhcnQsIGluCiAgICAgICAg IC8vIGNhc2Ugd2UgbmVlZCB0byBza2lwIHNvbWUgZnJhbWVzIGVudGlyZWx5LiAgUmVtZW1iZXIg bm90IHRvIGFkdmFuY2UKICAgICAgICAgLy8gdG8gYW4gaW5jb21wbGV0ZSBmcmFtZS4KLSAgICAg ICAgZm9yIChzaXplX3QgZnJhbWVBZnRlck5leHQgPSAobmV4dEZyYW1lICsgMSkgJSBmcmFtZUNv dW50KCk7IGZyYW1lSXNDb21wbGV0ZUF0SW5kZXgoZnJhbWVBZnRlck5leHQpOyBmcmFtZUFmdGVy TmV4dCA9IChuZXh0RnJhbWUgKyAxKSAlIGZyYW1lQ291bnQoKSkgeworICAgICAgICBmb3IgKHNp emVfdCBmcmFtZUFmdGVyTmV4dCA9IChuZXh0RnJhbWUgKyAxKSAlIGZyYW1lQ291bnQoKTsgbV9m cmFtZXNbZnJhbWVBZnRlck5leHRdLm1faXNDb21wbGV0ZTsgZnJhbWVBZnRlck5leHQgPSAobmV4 dEZyYW1lICsgMSkgJSBmcmFtZUNvdW50KCkpIHsKICAgICAgICAgICAgIC8vIFNob3VsZCB3ZSBz a2lwIHRoZSBuZXh0IGZyYW1lPwotICAgICAgICAgICAgZG91YmxlIGZyYW1lQWZ0ZXJOZXh0U3Rh cnRUaW1lID0gbV9kZXNpcmVkRnJhbWVTdGFydFRpbWUgKyBmcmFtZUR1cmF0aW9uQXRJbmRleChu ZXh0RnJhbWUpOworICAgICAgICAgICAgZG91YmxlIGZyYW1lQWZ0ZXJOZXh0U3RhcnRUaW1lID0g bV9kZXNpcmVkRnJhbWVTdGFydFRpbWUgKyBtX2ZyYW1lc1tuZXh0RnJhbWVdLm1fZHVyYXRpb247 CiAgICAgICAgICAgICBpZiAodGltZSA8IGZyYW1lQWZ0ZXJOZXh0U3RhcnRUaW1lKQogICAgICAg ICAgICAgICAgIGJyZWFrOwogCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQpkaWZmIC0tZ2l0IGEvdHJ1bmsvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nIGIvdHJ1bmsv U291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCi0tLSBhL3RydW5rL1NvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkoUmV2aXNpb24gMTI1NDMwKQorKysgYi90cnVuay9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VM b2cJKEFyYmVpdHNrb3BpZSkKQEAgLTEsMyArMSwxNiBAQAorMjAxMi0wOC0xMyAgVG9iaWFzIE5l dHplbCAgPHRvYmlhcy5uZXR6ZWxAZ29vZ2xlbWFpbC5jb20+CisKKyAgICAgICAgU2tpcHBpbmcg ZnJhbWVzIGluIGFuaW1hdGVkIEdJRnMgKG9yIG90aGVyIG11bHRpZnJhbWUgaW1hZ2VzKSBjYW4g Y2F1c2UgaW5pZmluaXRlIGxvb3AKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTkzNzY0CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkK KworICAgICAgICBEb24ndCBmb3JjZSBkZWNvZGluZyBvZiBza2lwcGVkIGZyYW1lcy4KKworICAg ICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL0JpdG1hcEltYWdlLmNwcDoKKyAgICAgICAgKFdlYkNv cmU6OkJpdG1hcEltYWdlOjpzdGFydEFuaW1hdGlvbik6IERvIGl0IGxpa2UgaW4gZGF0YUNoYW5n ZWQoKSBhbmQgYXZvaWQgdGhlIGNhbGwgdG8KKyAgICAgICAgICBlbnN1cmVGcmFtZUlzQ2FjaGVk KCkgd2hpY2ggZm9yY2VzIGRlY29kaW5nCisKIDIwMTItMDYtMjQgIFJvYmVydCBIb2dhbiAgPHJv YmVydEB3ZWJraXQub3JnPgogCiAgICAgICAgIENTUyAyLjEgZmFpbHVyZTogV29yZC1zcGFjaW5n IGFmZmVjdHMgZWFjaCBzcGFjZSBhbmQgbm9uLWJyZWFraW5nIHNwYWNlCg== 183150 2013-01-17 02:02:56 -0800 2013-06-25 13:39:37 -0700 patch format as produced by the scripts BitmapImage_v3.patch text/plain 1975 tobias.netzel SW5kZXg6IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL0JpdG1hcEltYWdlLmNwcAo9 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9CaXRtYXBJbWFn ZS5jcHAJKHJldmlzaW9uIDEyNTQzMCkKKysrIFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBo aWNzL0JpdG1hcEltYWdlLmNwcAkod29ya2luZyBjb3B5KQpAQCAtNDM3LDkgKzQzNyw5IEBACiAg ICAgICAgIC8vIFNlZSBpZiB3ZSd2ZSBhbHNvIHBhc3NlZCB0aGUgdGltZSBmb3IgZnJhbWVzIGFm dGVyIHRoYXQgdG8gc3RhcnQsIGluCiAgICAgICAgIC8vIGNhc2Ugd2UgbmVlZCB0byBza2lwIHNv bWUgZnJhbWVzIGVudGlyZWx5LiAgUmVtZW1iZXIgbm90IHRvIGFkdmFuY2UKICAgICAgICAgLy8g dG8gYW4gaW5jb21wbGV0ZSBmcmFtZS4KLSAgICAgICAgZm9yIChzaXplX3QgZnJhbWVBZnRlck5l eHQgPSAobmV4dEZyYW1lICsgMSkgJSBmcmFtZUNvdW50KCk7IGZyYW1lSXNDb21wbGV0ZUF0SW5k ZXgoZnJhbWVBZnRlck5leHQpOyBmcmFtZUFmdGVyTmV4dCA9IChuZXh0RnJhbWUgKyAxKSAlIGZy YW1lQ291bnQoKSkgeworICAgICAgICBmb3IgKHNpemVfdCBmcmFtZUFmdGVyTmV4dCA9IChuZXh0 RnJhbWUgKyAxKSAlIGZyYW1lQ291bnQoKTsgbV9mcmFtZXNbZnJhbWVBZnRlck5leHRdLm1faXND b21wbGV0ZTsgZnJhbWVBZnRlck5leHQgPSAobmV4dEZyYW1lICsgMSkgJSBmcmFtZUNvdW50KCkp IHsKICAgICAgICAgICAgIC8vIFNob3VsZCB3ZSBza2lwIHRoZSBuZXh0IGZyYW1lPwotICAgICAg ICAgICAgZG91YmxlIGZyYW1lQWZ0ZXJOZXh0U3RhcnRUaW1lID0gbV9kZXNpcmVkRnJhbWVTdGFy dFRpbWUgKyBmcmFtZUR1cmF0aW9uQXRJbmRleChuZXh0RnJhbWUpOworICAgICAgICAgICAgZG91 YmxlIGZyYW1lQWZ0ZXJOZXh0U3RhcnRUaW1lID0gbV9kZXNpcmVkRnJhbWVTdGFydFRpbWUgKyBt X2ZyYW1lc1tuZXh0RnJhbWVdLm1fZHVyYXRpb247CiAgICAgICAgICAgICBpZiAodGltZSA8IGZy YW1lQWZ0ZXJOZXh0U3RhcnRUaW1lKQogICAgICAgICAgICAgICAgIGJyZWFrOwogCkluZGV4OiBT b3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvQ2hh bmdlTG9nCShyZXZpc2lvbiAxMjU0MzApCisrKyBTb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cJKHdv cmtpbmcgY29weSkKQEAgLTEsMyArMSwxNiBAQAorMjAxMi0wOC0xMyAgVG9iaWFzIE5ldHplbCAg PHRvYmlhcy5uZXR6ZWxAZ29vZ2xlbWFpbC5jb20+CisKKyAgICAgICAgU2tpcHBpbmcgZnJhbWVz IGluIGFuaW1hdGVkIEdJRnMgKG9yIG90aGVyIG11bHRpZnJhbWUgaW1hZ2VzKSBjYW4gY2F1c2Ug aW5pZmluaXRlIGxvb3AKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTkzNzY0CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkKKworICAg ICAgICBEb24ndCBmb3JjZSBkZWNvZGluZyBvZiBza2lwcGVkIGZyYW1lcy4KKworICAgICAgICAq IHBsYXRmb3JtL2dyYXBoaWNzL0JpdG1hcEltYWdlLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkJp dG1hcEltYWdlOjpzdGFydEFuaW1hdGlvbik6IERvIGl0IGxpa2UgaW4gZGF0YUNoYW5nZWQoKSBh bmQgYXZvaWQgdGhlIGNhbGwgdG8KKyAgICAgICAgICBlbnN1cmVGcmFtZUlzQ2FjaGVkKCkgd2hp Y2ggZm9yY2VzIGRlY29kaW5nCisKIDIwMTItMDYtMjQgIFJvYmVydCBIb2dhbiAgPHJvYmVydEB3 ZWJraXQub3JnPgogCiAgICAgICAgIENTUyAyLjEgZmFpbHVyZTogV29yZC1zcGFjaW5nIGFmZmVj dHMgZWFjaCBzcGFjZSBhbmQgbm9uLWJyZWFraW5nIHNwYWNlCg==