Remove dependency on getLength from webp decoder.

Remove dependency on getLength from webp decoder.

In webp_parse_header, continue reading until end of stream is reached,
or we have read WEBP_VP8_HEADER_SIZE bytes. Do not check to see if the
stream was too short, since it may not have a way to report its length,
and WEBP_VP8_HEADER_SIZE is padded slightly. Instead, depend on
WebPGetFeatures to report that the stream did not have enough data.

In webp_idecode, only check length if it is available.

BUG=https://b.corp.google.com/issue?id=8432093
R=djsollen@google.com, vikasa@google.com

Committed: https://code.google.com/p/skia/source/detail?r=10848

[djsollen, 2013-08-12 17:47:21]

Craig, just wanted to give you a heads up as I think this CL is a more
comprehensive fix.

[djsollen, 2013-08-12 17:47:32]

lgtm

[djsollen, 2013-08-12 17:54:57]

retracting my l*tm as I missed this.

https://codereview.chromium.org/22841005/diff/1/src/images/SkImageDecoder_lib...
File src/images/SkImageDecoder_libwebp.cpp (right):

https://codereview.chromium.org/22841005/diff/1/src/images/SkImageDecoder_lib...
src/images/SkImageDecoder_libwebp.cpp:68: if ((size_t)-1 == bytesRead) {
I don't think our stream should ever return -1.  If we return 0 then we should
check if we are at end of the stream so I think replacing the break with a
continue will suffice.

[vikasa, 2013-08-12 18:25:40]

https://codereview.chromium.org/22841005/diff/1/src/images/SkImageDecoder_lib...
File src/images/SkImageDecoder_libwebp.cpp (right):

https://codereview.chromium.org/22841005/diff/1/src/images/SkImageDecoder_lib...
src/images/SkImageDecoder_libwebp.cpp:75: } while (!stream->isAtEnd() &&
bytesToRead > 0);
Assumption here is that 'stream->isAtEnd()' works reliably across all use-cases
(different types of file descriptors (ParcelFileDescriptor) etc).

https://codereview.chromium.org/22841005/diff/1/src/images/SkImageDecoder_lib...
src/images/SkImageDecoder_libwebp.cpp:202: SkTMin(stream->getLength(),
WEBP_IDECODE_BUFFER_SZ) : WEBP_IDECODE_BUFFER_SZ;
As mentioned by Craig (w.r.t https://codereview.chromium.org/22629010/), the
return value of stream->getLength() can be relied-upon.

[vikasa, 2013-08-12 18:26:17]

On 2013/08/12 18:25:40, vikasa wrote:
>
https://codereview.chromium.org/22841005/diff/1/src/images/SkImageDecoder_lib...
> File src/images/SkImageDecoder_libwebp.cpp (right):
> 
>
https://codereview.chromium.org/22841005/diff/1/src/images/SkImageDecoder_lib...
> src/images/SkImageDecoder_libwebp.cpp:75: } while (!stream->isAtEnd() &&
> bytesToRead > 0);
> Assumption here is that 'stream->isAtEnd()' works reliably across all
use-cases
> (different types of file descriptors (ParcelFileDescriptor) etc).
> 
>
https://codereview.chromium.org/22841005/diff/1/src/images/SkImageDecoder_lib...
> src/images/SkImageDecoder_libwebp.cpp:202: SkTMin(stream->getLength(),
> WEBP_IDECODE_BUFFER_SZ) : WEBP_IDECODE_BUFFER_SZ;
> As mentioned by Craig (w.r.t https://codereview.chromium.org/22629010/), the
> return value of stream->getLength() can be relied-upon.

s/can be/can't be

[scroggo, 2013-08-12 18:44:57]

https://codereview.chromium.org/22841005/diff/1/src/images/SkImageDecoder_lib...
File src/images/SkImageDecoder_libwebp.cpp (right):

https://codereview.chromium.org/22841005/diff/1/src/images/SkImageDecoder_lib...
src/images/SkImageDecoder_libwebp.cpp:202: SkTMin(stream->getLength(),
WEBP_IDECODE_BUFFER_SZ) : WEBP_IDECODE_BUFFER_SZ;
On 2013/08/12 18:25:41, vikasa wrote:
> As mentioned by Craig (w.r.t https://codereview.chromium.org/22629010/), the
> return value of stream->getLength() can[not] be relied-upon.

Right. That is why I only check stream->getLength() if stream->hasLength()
returns true, in which case we assume that the length is reliable. I also need
to update CreateJavaOutputStreamAdaptor to return false for hasLength as
appropriate.

[djsollen, 2013-08-12 18:46:52]

https://codereview.chromium.org/22841005/diff/1/src/images/SkImageDecoder_lib...
File src/images/SkImageDecoder_libwebp.cpp (right):

https://codereview.chromium.org/22841005/diff/1/src/images/SkImageDecoder_lib...
src/images/SkImageDecoder_libwebp.cpp:75: } while (!stream->isAtEnd() &&
bytesToRead > 0);
On 2013/08/12 18:25:41, vikasa wrote:
> Assumption here is that 'stream->isAtEnd()' works reliably across all
use-cases
> (different types of file descriptors (ParcelFileDescriptor) etc).

Yes, As part of this we are also updating the code that converts Java Streams to
SkStreams to support the new features in SkStream.

[vikasa, 2013-08-12 19:01:20]

lgtm

[scroggo, 2013-08-12 19:08:10]

New patch up

https://codereview.chromium.org/22841005/diff/1/src/images/SkImageDecoder_lib...
File src/images/SkImageDecoder_libwebp.cpp (right):

https://codereview.chromium.org/22841005/diff/1/src/images/SkImageDecoder_lib...
src/images/SkImageDecoder_libwebp.cpp:68: if ((size_t)-1 == bytesRead) {
On 2013/08/12 17:54:57, djsollen wrote:
> I don't think our stream should ever return -1.  If we return 0 then we should
> check if we are at end of the stream so I think replacing the break with a
> continue will suffice.

Done.

[djsollen, 2013-08-12 19:32:50]

lgtm

[scroggo, 2013-08-21 14:56:15]

Committed patchset #2 manually as r10848 (presubmit successful).