[webkit-reviews] review denied: [Bug 35411] Clean up m_failed usage in open-source image decoders : [Attachment 50738] Part 2, v2

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has denied Peter Kasting
<pkasting at google.com>'s request for review:
Bug 35411: Clean up m_failed usage in open-source image decoders
https://bugs.webkit.org/show_bug.cgi?id=35411

Attachment 50738: Part 2, v2
https://bugs.webkit.org/attachment.cgi?id=50738&action=review

------- Additional Comments from David Levin <levin at chromium.org>
The ChangeLog is missing too sparse for me to effectively review this.

Ideally there would be per function comments which explain why that function
was changed.

For example, I can't tell why the changes in do_lzw were done because it
changes the return value in a lot of places. I'm sure this is obvious to you
and probably obvious for someone who knows the code well but neither of those
is true for me.

Similarly, in GIFImageReader::read, it seems that it may return true in a few
cases in which it returned false before like in this code:
    if (!map)
	return clientptr ? clientptr->setFailed() : false;

I don't understand why this is good (from reading the ChangeLog and the code).

Lastly, there is no new layout tests or explanation of why one either isn't
needed or can't be done.