[webkit-reviews] review denied: [Bug 115623] Crash in Image Decoder due to large gifs : [Attachment 201016] The decoded GIF size > 5M, reserve the initialized frame

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied gordon
<gosun at blackberry.com>'s request for review:
Bug 115623: Crash in Image Decoder  due to large gifs
https://bugs.webkit.org/show_bug.cgi?id=115623

Attachment 201016: The decoded GIF size > 5M, reserve the initialized frame
https://bugs.webkit.org/attachment.cgi?id=201016&action=review

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=201016&action=review


> LayoutTests/ChangeLog:12
> +	   If the decoded GIF is larger than 5M,
> +	   we clear the the frame buffer except the current and after frame.
> +	   However, if the current frame is the last frame of the GIF,
> +	   and the first frame has been re-initialized for next time,
> +	   then should not clear first frame, it should be reserved.

This does not explain the test.

Especially since it will decode either only the first frame or no frame at all.


> LayoutTests/ChangeLog:16
> +	   * fast/images/resources/gif-large.gif: Added.

Do you own the rights on this?
Better create a new test image yourself.

> Source/WebCore/ChangeLog:13
> +	   If the decoded GIF is larger than 5M,
> +	   we clear the the frame buffer except the current and after frame.
> +	   However, if the current frame is the last frame of the GIF,
> +	   and the first frame has been re-initialized for next time,
> +	   then should not clear first frame, it should be reserved.
> +	   Test: fast/images/animated-large-image-crash.html

This does not explain the reason of the crash.

> Source/WebCore/platform/image-decoders/gif/GIFImageReader.h:270
> +
> +    size_t currentFrame() const
> +    {
> +	   return m_currentDecodingFrame;
> +    }
> +

Why do you suddenly expose this publicly?