[webkit-reviews] review granted: [Bug 170697] CachedImage should cancel loading images for unsupported/unknown types : [Attachment 307254] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 170697: CachedImage should cancel loading images for unsupported/unknown
types
https://bugs.webkit.org/show_bug.cgi?id=170697

Attachment 307254: Patch

https://bugs.webkit.org/attachment.cgi?id=307254&action=review




--- Comment #20 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 307254
  --> https://bugs.webkit.org/attachment.cgi?id=307254
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=307254&action=review

> Source/WebCore/loader/cache/CachedImage.cpp:407
>	   // Image decoding failed. Either we need more image data or the
image data is malformed.

This comment seems wrong to me from reading the code below.
If we need more image data, we should not call error(). We should just exit
early in order to wait for more data.
Can you check we are doing the right thing for this case and update the comment
accordingly?

> Source/WebCore/loader/cache/CachedImage.cpp:409
> +	   if (m_loader)

Can we have the case of a null m_loader in
CachedImage::addIncrementalDataBuffer.
Probably not, so we might be able to call m_loader->cancel() directly. Maybe
with an ASSERT(m_loader) before?