Fix bug in SkGifCodec / Switch SkImageDec tests to use Codec

Fix bug in SkGifCodec / Switch SkImageDec tests to use Codec

SkImageDecoder is still used throughout tests, tools, gms etc.
Deleting it from tests is an easy first step.

Bonus is that we add tests of SkCodec.

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

Committed: https://skia.googlesource.com/skia/+/7f7ec206de39fde8dc490e9feb0f65322af1b989

[msarett, 2016-02-25 00:02:49]

Description was changed from

==========
Delete uses of SkImageDecoder in tests/

SkImageDecoder is still used throughout tests, tools, gms etc.
Deleting it from tests is an easy first step.

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

to

==========
Delete uses of SkImageDecoder in tests/

SkImageDecoder is still used throughout tests, tools, gms etc.
Deleting it from tests is an easy first step.

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

[msarett, 2016-02-25 00:03:36]

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

[scroggo, 2016-02-25 12:43:29]

You have to delete KtxTest, but for the others, I think it makes sense to switch
them to use SkCodec.

https://codereview.chromium.org/1733863003/diff/1/tests/BadIcoTest.cpp
File tests/BadIcoTest.cpp (left):

https://codereview.chromium.org/1733863003/diff/1/tests/BadIcoTest.cpp#oldcode33
tests/BadIcoTest.cpp:33: bool success =
SkImageDecoder::DecodeFile(fullPath.c_str(), &bm);
Why not switch this to use SkCodec?

https://codereview.chromium.org/1733863003/diff/1/tests/GifTest.cpp
File tests/GifTest.cpp (left):

https://codereview.chromium.org/1733863003/diff/1/tests/GifTest.cpp#oldcode58
tests/GifTest.cpp:58: bool imageDecodeSuccess = SkImageDecoder::DecodeMemory(
Switch to use SkCodec?

https://codereview.chromium.org/1733863003/diff/1/tests/IndexedPngOverflowTes...
File tests/IndexedPngOverflowTest.cpp (left):

https://codereview.chromium.org/1733863003/diff/1/tests/IndexedPngOverflowTes...
tests/IndexedPngOverflowTest.cpp:35: bool success =
SkImageDecoder::DecodeMemory(
SkCodec?

https://codereview.chromium.org/1733863003/diff/1/tests/InvalidIndexedPngTest...
File tests/InvalidIndexedPngTest.cpp (left):

https://codereview.chromium.org/1733863003/diff/1/tests/InvalidIndexedPngTest...
tests/InvalidIndexedPngTest.cpp:37: SkImageDecoder::DecodeMemory(gPngData,
sizeof(gPngData), &image);
SkCodec?

[msarett, 2016-03-01 00:19:35]

Patchset #2 (id:20001) has been deleted

[msarett, 2016-03-01 00:21:11]

> You have to delete KtxTest, but for the others, I think it makes sense to
switch
> them to use SkCodec.

SGTM.  I think the GifTest is particularly interested because SkCodec made
different decisions that SkImageDecoder on how to deal with invalid frame
dimensions.

https://codereview.chromium.org/1733863003/diff/1/tests/BadIcoTest.cpp
File tests/BadIcoTest.cpp (left):

https://codereview.chromium.org/1733863003/diff/1/tests/BadIcoTest.cpp#oldcode33
tests/BadIcoTest.cpp:33: bool success =
SkImageDecoder::DecodeFile(fullPath.c_str(), &bm);
On 2016/02/25 12:43:29, scroggo wrote:
> Why not switch this to use SkCodec?

Done.

https://codereview.chromium.org/1733863003/diff/1/tests/GifTest.cpp
File tests/GifTest.cpp (left):

https://codereview.chromium.org/1733863003/diff/1/tests/GifTest.cpp#oldcode58
tests/GifTest.cpp:58: bool imageDecodeSuccess = SkImageDecoder::DecodeMemory(
On 2016/02/25 12:43:29, scroggo wrote:
> Switch to use SkCodec?

Done.

https://codereview.chromium.org/1733863003/diff/1/tests/IndexedPngOverflowTes...
File tests/IndexedPngOverflowTest.cpp (left):

https://codereview.chromium.org/1733863003/diff/1/tests/IndexedPngOverflowTes...
tests/IndexedPngOverflowTest.cpp:35: bool success =
SkImageDecoder::DecodeMemory(
On 2016/02/25 12:43:29, scroggo wrote:
> SkCodec?

Done.

https://codereview.chromium.org/1733863003/diff/1/tests/InvalidIndexedPngTest...
File tests/InvalidIndexedPngTest.cpp (left):

https://codereview.chromium.org/1733863003/diff/1/tests/InvalidIndexedPngTest...
tests/InvalidIndexedPngTest.cpp:37: SkImageDecoder::DecodeMemory(gPngData,
sizeof(gPngData), &image);
On 2016/02/25 12:43:29, scroggo wrote:
> SkCodec?

Done.

[scroggo, 2016-03-01 14:51:54]

lgtm

https://codereview.chromium.org/1733863003/diff/40001/tests/GifTest.cpp
File tests/GifTest.cpp (right):

https://codereview.chromium.org/1733863003/diff/40001/tests/GifTest.cpp#newco...
tests/GifTest.cpp:212: if (kIndex_8_SkColorType == codec->getInfo().colorType())
{
Fine to have this here, although I assume it's always true for SkGifCodec, and
these are all gifs?

https://codereview.chromium.org/1733863003/diff/40001/tests/InvalidIndexedPng...
File tests/InvalidIndexedPngTest.cpp (right):

https://codereview.chromium.org/1733863003/diff/40001/tests/InvalidIndexedPng...
tests/InvalidIndexedPngTest.cpp:11: #include "SkData.h"
Why'd you need to include SkData?

[msarett, 2016-03-01 16:24:28]

https://codereview.chromium.org/1733863003/diff/40001/tests/GifTest.cpp
File tests/GifTest.cpp (right):

https://codereview.chromium.org/1733863003/diff/40001/tests/GifTest.cpp#newco...
tests/GifTest.cpp:212: if (kIndex_8_SkColorType == codec->getInfo().colorType())
{
On 2016/03/01 14:51:54, scroggo wrote:
> Fine to have this here, although I assume it's always true for SkGifCodec, and
> these are all gifs?

Yes that's correct.

https://codereview.chromium.org/1733863003/diff/40001/tests/InvalidIndexedPng...
File tests/InvalidIndexedPngTest.cpp (right):

https://codereview.chromium.org/1733863003/diff/40001/tests/InvalidIndexedPng...
tests/InvalidIndexedPngTest.cpp:11: #include "SkData.h"
On 2016/03/01 14:51:54, scroggo wrote:
> Why'd you need to include SkData?

After refactoring, I don't think I need either of these.

[msarett, 2016-03-01 16:24:48]

The CQ bit was checked by msarett@google.com

[msarett, 2016-03-01 16:24:48]

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/1733863003/#ps60001
(title: "Remove unnecessary includes")

[commit-bot: I haz the power, 2016-03-01 16:25:00]

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

[commit-bot: I haz the power, 2016-03-01 16:42:00]

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

[commit-bot: I haz the power, 2016-03-01 16:42:02]

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...)

[msarett, 2016-03-01 18:48:16]

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

[commit-bot: I haz the power, 2016-03-01 18:48:26]

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

[msarett, 2016-03-01 18:50:06]

PTAL

Glad you suggested that we keep these tests!  The Release trybot caught an
uninitialized memory bug.

When this lands, I'll be sure to mark it for M50.

[scroggo, 2016-03-01 18:55:01]

lgtm

[msarett, 2016-03-01 18:55:42]

Description was changed from

==========
Delete uses of SkImageDecoder in tests/

SkImageDecoder is still used throughout tests, tools, gms etc.
Deleting it from tests is an easy first step.

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

to

==========
Fix bug in SkGifCodec / Switch SkImageDec tests to use Codec

SkImageDecoder is still used throughout tests, tools, gms etc.
Deleting it from tests is an easy first step.

Bonus is that we add tests of SkCodec.

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

[msarett, 2016-03-01 20:10:00]

The CQ bit was unchecked by msarett@google.com

[msarett, 2016-03-01 20:10:02]

The CQ bit was checked by msarett@google.com

[commit-bot: I haz the power, 2016-03-01 20:10:06]

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

[commit-bot: I haz the power, 2016-03-01 20:12:29]

Description was changed from

==========
Fix bug in SkGifCodec / Switch SkImageDec tests to use Codec

SkImageDecoder is still used throughout tests, tools, gms etc.
Deleting it from tests is an easy first step.

Bonus is that we add tests of SkCodec.

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

to

==========
Fix bug in SkGifCodec / Switch SkImageDec tests to use Codec

SkImageDecoder is still used throughout tests, tools, gms etc.
Deleting it from tests is an easy first step.

Bonus is that we add tests of SkCodec.

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

Committed:
https://skia.googlesource.com/skia/+/7f7ec206de39fde8dc490e9feb0f65322af1b989
==========

[commit-bot: I haz the power, 2016-03-01 20:12:30]

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