Focus SkScaledCodec on BitmapRegionDecoder

Focus SkScaledCodec on BitmapRegionDecoder

The primary goal of SkScaledCodec is to replace the current
implementation of BitmapRegionDecoder, which depends on modified
versions of libjpeg and libpng, with an implementation that uses
standard versions of the libaries. Since BitmapRegionDecoder only
supports PNG, WEBP and JPEG, limit SkScaledCodec to those classes.
We will focus on those three until we complete this primary goal.
Then we can continue to make SkScaledCodec work for other formats.

Fix some bugs in SkScaledCodec::NewFromStream:
- Handle a NULL input stream properly
- Ensure that the input stream is deleted as expected on bad data

Add tests for these error cases.

BUG=skia:4428
Committed: https://skia.googlesource.com/skia/+/2c3b218f74aa0ccd544a73feb506a0be56d90abe

[scroggo, 2015-10-06 17:17:55]

scroggo@google.com changed reviewers:
+ msarett@google.com

[msarett, 2015-10-06 17:38:02]

https://codereview.chromium.org/1389053002/diff/20001/src/codec/SkScaledCodec...
File src/codec/SkScaledCodec.cpp (right):

https://codereview.chromium.org/1389053002/diff/20001/src/codec/SkScaledCodec...
src/codec/SkScaledCodec.cpp:20: switch (codec->getEncodedFormat()) {
I'm opposed to this.

I think we've both done a significant amount of refactoring work to make
SkScaledCodec work for all of our image types.  And I think we've made design
decisions based on this goal.

To me, this change feels like we are throwing away work.  I'm fine with
"ignoring" bugs that pop up, but I'm worried this goes too far.

Happy to talk in person.

[scroggo, 2015-10-06 17:52:41]

https://codereview.chromium.org/1389053002/diff/20001/src/codec/SkScaledCodec...
File src/codec/SkScaledCodec.cpp (right):

https://codereview.chromium.org/1389053002/diff/20001/src/codec/SkScaledCodec...
src/codec/SkScaledCodec.cpp:20: switch (codec->getEncodedFormat()) {
On 2015/10/06 17:38:02, msarett wrote:
> I'm opposed to this.
> 
> I think we've both done a significant amount of refactoring work to make
> SkScaledCodec work for all of our image types.  And I think we've made design
> decisions based on this goal.
> 
> To me, this change feels like we are throwing away work.  I'm fine with
> "ignoring" bugs that pop up, but I'm worried this goes too far.
> 
> Happy to talk in person.

Ignoring bugs is hard though. If we want to run nanobench and DM, we need to
either make sure that we are not running any images that might break in our
classes where it might not work, or we need to fix (or special case) those bugs.

As an alternative, we could make DM/nanobench ignore anything that is not one of
the three we care about.

And it's not throwing away work. It's reprioritizing. The non-big-three will
still be there when we return to work on them. We should file any outstanding
bugs we are aware of, but leaving them enabled means we may run into them again,
and spend time trying to work around or fix them.

[msarett, 2015-10-06 18:24:29]

lgtm

[scroggo, 2015-10-09 15:12:57]

The CQ bit was checked by scroggo@google.com

[commit-bot: I haz the power, 2015-10-09 15:13:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1389053002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1389053002/20001

[commit-bot: I haz the power, 2015-10-09 15:15:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-09 15:15:23]

Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)

[scroggo, 2015-10-09 15:32:39]

Patchset #3 (id:40001) has been deleted

[scroggo, 2015-10-09 15:34:14]

The CQ bit was checked by scroggo@google.com

[scroggo, 2015-10-09 15:34:14]

The patchset sent to the CQ was uploaded after l-g-t-m from msarett@google.com
Link to the patchset: https://codereview.chromium.org/1389053002/#ps80001
(title: "Disable SkScaledCodec non BRD types in CodexTest")

[commit-bot: I haz the power, 2015-10-09 15:34:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1389053002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1389053002/80001

[commit-bot: I haz the power, 2015-10-09 15:41:04]

Committed patchset #4 (id:80001) as
https://skia.googlesource.com/skia/+/2c3b218f74aa0ccd544a73feb506a0be56d90abe