Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(24)

Issue 2548863002: Add tests for PNGImageDecoder. (Closed)

Created:
4 years ago by joostouwerling
Modified:
4 years ago
Reviewers:
scroggo_chromium, Nico
CC:
chromium-reviews, blink-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add a basic test suite for PNGImageDecoder. Add the following tests for PNGImageDecoder: - Verify that the repetition count is cAnimationNone. - Verify that the image size is decoded correctly. - Verify that the decoded frame count is 1, and that the frame duration is 0. - Verify that progressively providing the image data yields the same frame buffer hashes as when the data is provided in a truncated form. - Verify that progressive decoding can completely decode the image when at first half of the image data is provided and the image is partially decoded, and then the rest of the image data is provided. It verifies that the content of the intermediate and final frame are different. - Verify that the decoder is invalidated when invalid image data is provided. - Verify that frameIsCompleteAtIndex() returns true if and only if the frame is completely decoded. Fully receiving the image data is not enough to be considered complete for static PNGs. The image that is being tested is a simple 111 by 29 pixels image created by myself. As an additional note here - some of these tests are a little awkward when this CL is viewed in isolation, with e.g. separate functions for tests which allow more configuration than necessary. This is done so they can be reused for APNG tests, which is currently in development in crrev.com/2386453003. BUG=437662 Committed: https://crrev.com/378e8f3d4729d64eacd3ea24bd05f4d1e11502f6 Cr-Commit-Position: refs/heads/master@{#436435}

Patch Set 1 #

Patch Set 2 : Modify comments for tests #

Total comments: 8

Patch Set 3 : Improve progressive PNG decoding tests. #

Total comments: 7

Patch Set 4 : Remove testProgressiveDecodingChangesFrameBuffer and make const-correct. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+180 lines, -0 lines) Patch
A third_party/WebKit/LayoutTests/images/resources/png-simple.png View Binary file 0 comments Download
M third_party/WebKit/Source/platform/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
A third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoderTest.cpp View 1 2 3 1 chunk +179 lines, -0 lines 0 comments Download

Messages

Total messages: 24 (12 generated)
joostouwerling
4 years ago (2016-12-02 21:18:12 UTC) #5
joostouwerling
As an additional note here - some of these tests are a little awkward when ...
4 years ago (2016-12-02 21:48:52 UTC) #6
scroggo_chromium
On 2016/12/02 21:48:52, joostouwerling wrote: > As an additional note here - some of these ...
4 years ago (2016-12-05 14:40:17 UTC) #7
joostouwerling
https://codereview.chromium.org/2548863002/diff/20001/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoderTest.cpp File third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoderTest.cpp (right): https://codereview.chromium.org/2548863002/diff/20001/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoderTest.cpp#newcode76 third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoderTest.cpp:76: EXPECT_NE(hashFull, hashPartial); On 2016/12/05 14:40:17, scroggo_chromium wrote: > It ...
4 years ago (2016-12-05 18:29:32 UTC) #10
joostouwerling
Adding thakis@ for BUILD.gn
4 years ago (2016-12-05 18:30:28 UTC) #12
scroggo_chromium
https://codereview.chromium.org/2548863002/diff/40001/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoderTest.cpp File third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoderTest.cpp (right): https://codereview.chromium.org/2548863002/diff/40001/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoderTest.cpp#newcode104 third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoderTest.cpp:104: size_t frameRectRows = frame->originalFrameRect().height(); nit: could be const. https://codereview.chromium.org/2548863002/diff/40001/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoderTest.cpp#newcode122 ...
4 years ago (2016-12-05 18:41:30 UTC) #13
joostouwerling
https://codereview.chromium.org/2548863002/diff/40001/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoderTest.cpp File third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoderTest.cpp (right): https://codereview.chromium.org/2548863002/diff/40001/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoderTest.cpp#newcode104 third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoderTest.cpp:104: size_t frameRectRows = frame->originalFrameRect().height(); On 2016/12/05 18:41:29, scroggo_chromium wrote: ...
4 years ago (2016-12-05 19:05:01 UTC) #14
scroggo_chromium
lgtm https://codereview.chromium.org/2548863002/diff/40001/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoderTest.cpp File third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoderTest.cpp (right): https://codereview.chromium.org/2548863002/diff/40001/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoderTest.cpp#newcode122 third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoderTest.cpp:122: // This makes sure we don't count going ...
4 years ago (2016-12-05 19:17:20 UTC) #16
Nico
build.gn lgtm
4 years ago (2016-12-05 20:25:42 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2548863002/60001
4 years ago (2016-12-05 20:28:35 UTC) #19
commit-bot: I haz the power
Committed patchset #4 (id:60001)
4 years ago (2016-12-05 22:51:08 UTC) #22
commit-bot: I haz the power
4 years ago (2016-12-05 22:53:01 UTC) #24
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/378e8f3d4729d64eacd3ea24bd05f4d1e11502f6
Cr-Commit-Position: refs/heads/master@{#436435}

Powered by Google App Engine
This is Rietveld 408576698