Fix ICO bug exposed by the fuzzer

Fix ICO bug exposed by the fuzzer

BUG=skia:5248
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1949773002

Committed: https://skia.googlesource.com/skia/+/3e375b0a51a46814cf08ecd1a513862006a704e5

[msarett, 2016-05-03 22:08:08]

Description was changed from

==========
Fix ICO bug exposed by the fuzzer

BUG=skia:5248
==========

to

==========
Fix ICO bug exposed by the fuzzer

BUG=skia:5248
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[msarett, 2016-05-03 22:09:05]

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

[scroggo, 2016-05-04 13:02:19]

lgtm

https://codereview.chromium.org/1949773002/diff/1/src/codec/SkBmpStandardCode...
File src/codec/SkBmpStandardCodec.cpp (right):

https://codereview.chromium.org/1949773002/diff/1/src/codec/SkBmpStandardCode...
src/codec/SkBmpStandardCodec.cpp:153: // In the case of ico-in-bmps, we will
report BGRA to the client,
nit: Shouldn't this be bmp-in-icos?

https://codereview.chromium.org/1949773002/diff/1/src/codec/SkBmpStandardCode...
src/codec/SkBmpStandardCodec.cpp:159: case 8:
We used to handle <= 8. I take it < was unnecessary?

https://codereview.chromium.org/1949773002/diff/1/src/codec/SkBmpStandardCode...
src/codec/SkBmpStandardCodec.cpp:161: swizzlerInfo.alpha(),
this->bitsPerPixel());
8?

https://codereview.chromium.org/1949773002/diff/1/src/codec/SkBmpStandardCode...
src/codec/SkBmpStandardCodec.cpp:165: SkEncodedInfo::kOpaque_Alpha,
swizzlerInfo.bitsPerComponent());
Why do you need to check bitsPerComponent here? Will it ever be anything other
than 8? If so, it seems like something doesn't match up.

[msarett, 2016-05-04 19:27:49]

https://codereview.chromium.org/1949773002/diff/1/src/codec/SkBmpStandardCode...
File src/codec/SkBmpStandardCodec.cpp (right):

https://codereview.chromium.org/1949773002/diff/1/src/codec/SkBmpStandardCode...
src/codec/SkBmpStandardCodec.cpp:153: // In the case of ico-in-bmps, we will
report BGRA to the client,
On 2016/05/04 13:02:19, scroggo wrote:
> nit: Shouldn't this be bmp-in-icos?

Done.

https://codereview.chromium.org/1949773002/diff/1/src/codec/SkBmpStandardCode...
src/codec/SkBmpStandardCodec.cpp:159: case 8:
On 2016/05/04 13:02:19, scroggo wrote:
> We used to handle <= 8. I take it < was unnecessary?

Nice catch!  This is me trying to introduce a new bug.  <= 8 is correct, I'll
fix this.

https://codereview.chromium.org/1949773002/diff/1/src/codec/SkBmpStandardCode...
src/codec/SkBmpStandardCodec.cpp:161: swizzlerInfo.alpha(),
this->bitsPerPixel());
On 2016/05/04 13:02:19, scroggo wrote:
> 8?

It could be 1, 2, 4, or 8.

https://codereview.chromium.org/1949773002/diff/1/src/codec/SkBmpStandardCode...
src/codec/SkBmpStandardCodec.cpp:165: SkEncodedInfo::kOpaque_Alpha,
swizzlerInfo.bitsPerComponent());
On 2016/05/04 13:02:19, scroggo wrote:
> Why do you need to check bitsPerComponent here? Will it ever be anything other
> than 8? If so, it seems like something doesn't match up.

It will always be 8.  I'll put 8.

[msarett, 2016-05-04 19:28:44]

Patchset #2 (id:20001) has been deleted

[msarett, 2016-05-04 19:29:32]

The CQ bit was checked by msarett@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-04 19:29:43]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1949773002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1949773002/40001

[commit-bot: I haz the power, 2016-05-04 19:42:53]

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

[commit-bot: I haz the power, 2016-05-04 19:42:54]

Dry run: This issue passed the CQ dry run.

[msarett, 2016-05-04 20:02:46]

The CQ bit was checked by msarett@google.com

[msarett, 2016-05-04 20:02:47]

The patchset sent to the CQ was uploaded after l-g-t-m from scroggo@google.com
Link to the patchset: https://codereview.chromium.org/1949773002/#ps40001
(title: " ")

[commit-bot: I haz the power, 2016-05-04 20:02:54]

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

[commit-bot: I haz the power, 2016-05-04 20:03:55]

Description was changed from

==========
Fix ICO bug exposed by the fuzzer

BUG=skia:5248
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Fix ICO bug exposed by the fuzzer

BUG=skia:5248
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/3e375b0a51a46814cf08ecd1a513862006a704e5
==========

[commit-bot: I haz the power, 2016-05-04 20:03:56]

Committed patchset #2 (id:40001) as
https://skia.googlesource.com/skia/+/3e375b0a51a46814cf08ecd1a513862006a704e5