Change SkImageDecoders to take an SkStreamRewindable.

Change SkImageDecoders to take an SkStreamRewindable.

Only affects factories, static functions that will use the factories,
and subset decoding, which all require rewinding. The decoders
themselves continue to take an SkStream. This is merely documentation
stating which functions will possibly rewind the passed in SkStream.

This is part of the general change to coordinate SkStreams with
Android's streams, which don't necessarily support rewinding in all
cases.

Update callers to use SkStreamRewindable.

BUG=skia:1572
R=bungeman@google.com, reed@google.com

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

[reed1, 2013-08-28 19:19:39]

Knowing nothing about streams... lgtm

[bungeman-skia, 2013-09-19 16:15:49]

It seems somewhat interesting that now we have the SkFrontBufferedStream which
could fit between SkStream and SkStreamRewindable
(SkStreamPartiallyRewindable?). It seems that then the encoders could advertise
how much buffer they will want. Anyhow, something to think about.

https://codereview.chromium.org/23477009/diff/8001/src/images/SkImageDecoder_...
File src/images/SkImageDecoder_libwebp.cpp (right):

https://codereview.chromium.org/23477009/diff/8001/src/images/SkImageDecoder_...
src/images/SkImageDecoder_libwebp.cpp:139: int                 fHasAlpha;
Vertical aligning declarations (aka table style declarations) are an eyesore.
There, I said it. This drives me crazy. What possible reason does it have? Sure,
I can read off all of the names a bit easier, but why would I care about that?
What I really want to know when looking at a name is what type it is, the name
can be anything (and often is) but the type should mean something.

I also firmly believe that no one should ever use the right binding form of
'const' in a declaration, and that * and & should always be closer to the name
than the type (since that's the actual syntactic binding; Google Style Guide
allows placing next to type or name). So maybe I'm just crazy.

Since I'm looking for justification, the Google Style Guide doen't directly back
me up on this, but the example at
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Variab...
implies that one should not use table style for declarations. The Skia style
guide shows some table style assignments, but no table style declarations. I'm
sometimes ok with the table style assignments.

https://codereview.chromium.org/23477009/diff/8001/src/images/SkMovie.cpp
File src/images/SkMovie.cpp (right):

https://codereview.chromium.org/23477009/diff/8001/src/images/SkMovie.cpp#new...
src/images/SkMovie.cpp:95: SkAutoTUnref<SkStreamRewindable>
stream(SkStreamRewindable::NewFromFile(path));
Should probably just call this as SkStream::NewFromFile since it's a static
method. It also returns and SkStreamAsset, though that makes little difference
here.

[scroggo, 2013-09-25 20:25:09]

After uploading patch set 3, it occurred to me that this change is larger than
necessary. While the image decoder factories and subset decoding require
SkStreamRewindable, the decoders themselves do not. Patch set 4 only requires an
SkStreamRewindable if rewind might actually be used. Now, if someone knows they
have a JPEG stream, they don't have to wrap it in something rewindable just to
call SkJPEGDecoder::decode on it </pedantic>

https://codereview.chromium.org/23477009/diff/8001/src/images/SkImageDecoder_...
File src/images/SkImageDecoder_libwebp.cpp (right):

https://codereview.chromium.org/23477009/diff/8001/src/images/SkImageDecoder_...
src/images/SkImageDecoder_libwebp.cpp:139: int                 fHasAlpha;
On 2013/09/19 16:15:49, bungeman1 wrote:
> Vertical aligning declarations (aka table style declarations) are an eyesore.
> There, I said it. This drives me crazy. What possible reason does it have?
Sure,
> I can read off all of the names a bit easier, but why would I care about that?
> What I really want to know when looking at a name is what type it is, the name
> can be anything (and often is) but the type should mean something.

I could have sworn this was in our style guide. It's certainly something we do a
lot in the Skia code base. I thought reed@ liked it. Mike?

> 
> I also firmly believe that no one should ever use the right binding form of
> 'const' in a declaration, and that * and & should always be closer to the name
> than the type (since that's the actual syntactic binding; Google Style Guide
> allows placing next to type or name). So maybe I'm just crazy.

Personally I like the * next to the type, since an SkStreamRewindable* is
different from an SkStreamRewindable. 
> 
> Since I'm looking for justification, the Google Style Guide doen't directly
back
> me up on this, but the example at
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Variab...
> implies that one should not use table style for declarations. The Skia style
> guide shows some table style assignments, but no table style declarations. I'm
> sometimes ok with the table style assignments.

https://codereview.chromium.org/23477009/diff/8001/src/images/SkMovie.cpp
File src/images/SkMovie.cpp (right):

https://codereview.chromium.org/23477009/diff/8001/src/images/SkMovie.cpp#new...
src/images/SkMovie.cpp:95: SkAutoTUnref<SkStreamRewindable>
stream(SkStreamRewindable::NewFromFile(path));
On 2013/09/19 16:15:49, bungeman1 wrote:
> Should probably just call this as SkStream::NewFromFile since it's a static
> method. It also returns and SkStreamAsset, though that makes little difference
> here.

D'oh! Overzealous search/replace. Fixed

[bungeman-skia, 2013-09-25 20:37:50]

Patch Set 4 lgtm

[scroggo, 2013-09-25 21:34:42]

Committed patchset #4 manually as r11460 (presubmit successful).