Move gif image decoder to SkCodec

third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoderTest.cpp

Index: third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoderTest.cpp
diff --git a/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoderTest.cpp b/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoderTest.cpp
index 8c4e988b4ca6653476eb4df645c89a6ac8288843..9b68d8cba6f89810d60559f178078a69b330d044 100644
--- a/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoderTest.cpp
+++ b/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoderTest.cpp
@@ -59,16 +59,7 @@ void testRepetitionCount(const char* dir,
   RefPtr<SharedBuffer> data = readFile(dir, file);
   ASSERT_TRUE(data.get());
   decoder->setData(data.get(), true);
-  EXPECT_EQ(cAnimationLoopOnce,
-            decoder->repetitionCount());  // Default value before decode.
-
-  for (size_t i = 0; i < decoder->frameCount(); ++i) {
-    ImageFrame* frame = decoder->frameBufferAtIndex(i);
-    EXPECT_EQ(ImageFrame::FrameComplete, frame->getStatus());
-  }
-
-  EXPECT_EQ(expectedRepetitionCount,
-            decoder->repetitionCount());  // Expected value after decode.
+  EXPECT_EQ(expectedRepetitionCount, decoder->repetitionCount());
 }
 
 }  // anonymous namespace
@@ -79,7 +70,6 @@ TEST(GIFImageDecoderTest, decodeTwoFrames) {
   RefPtr<SharedBuffer> data = readFile(layoutTestResourcesDir, "animated.gif");
   ASSERT_TRUE(data.get());
   decoder->setData(data.get(), true);
-  EXPECT_EQ(cAnimationLoopOnce, decoder->repetitionCount());
 
   ImageFrame* frame = decoder->frameBufferAtIndex(0);
   uint32_t generationID0 = frame->bitmap().getGenerationID();
@@ -104,10 +94,6 @@ TEST(GIFImageDecoderTest, parseAndDecode) {
   RefPtr<SharedBuffer> data = readFile(layoutTestResourcesDir, "animated.gif");
   ASSERT_TRUE(data.get());
   decoder->setData(data.get(), true);
-  EXPECT_EQ(cAnimationLoopOnce, decoder->repetitionCount());
-
-  // This call will parse the entire file.
-  EXPECT_EQ(2u, decoder->frameCount());
 
   ImageFrame* frame = decoder->frameBufferAtIndex(0);
   EXPECT_EQ(ImageFrame::FrameComplete, frame->getStatus());
@@ -307,24 +293,6 @@ TEST(GIFImageDecoderTest, badCode) {
   EXPECT_TRUE(testDecoder->failed());
 }
 
-TEST(GIFImageDecoderTest, invalidDisposalMethod) {
-  std::unique_ptr<ImageDecoder> decoder = createDecoder();
-
-  // The image has 2 frames, with disposal method 4 and 5, respectively.
-  RefPtr<SharedBuffer> data =
-      readFile(decodersTestingDir, "invalid-disposal-method.gif");
-  ASSERT_TRUE(data.get());
-  decoder->setData(data.get(), true);
-
-  EXPECT_EQ(2u, decoder->frameCount());
-  // Disposal method 4 is converted to ImageFrame::DisposeOverwritePrevious.
-  EXPECT_EQ(ImageFrame::DisposeOverwritePrevious,
-            decoder->frameBufferAtIndex(0)->getDisposalMethod());
-  // Disposal method 5 is ignored.
-  EXPECT_EQ(ImageFrame::DisposeNotSpecified,
-            decoder->frameBufferAtIndex(1)->getDisposalMethod());
-}
-
 TEST(GIFImageDecoderTest, firstFrameHasGreaterSizeThanScreenSize) {
   RefPtr<SharedBuffer> fullData = readFile(
       decodersTestingDir, "first-frame-has-greater-size-than-screen-size.gif");
@@ -359,14 +327,6 @@ TEST(GIFImageDecoderTest, bitmapAlphaType) {
   RefPtr<SharedBuffer> fullData = readFile(decodersTestingDir, "radient.gif");
   ASSERT_TRUE(fullData.get());
 
-  // Empirically chosen truncation size:
-  //   a) large enough to produce a partial frame &&
-  //   b) small enough to not fully decode the frame
-  const size_t kTruncateSize = 800;
-  ASSERT_TRUE(kTruncateSize < fullData->size());
-  RefPtr<SharedBuffer> partialData =
-      SharedBuffer::create(fullData->data(), kTruncateSize);
-
   std::unique_ptr<ImageDecoder> premulDecoder = WTF::wrapUnique(
       new GIFImageDecoder(ImageDecoder::AlphaPremultiplied,
                           ColorBehavior::transformToTargetForTesting(),
@@ -376,34 +336,18 @@ TEST(GIFImageDecoderTest, bitmapAlphaType) {
                           ColorBehavior::transformToTargetForTesting(),
                           ImageDecoder::noDecodedImageByteLimit));
 
-  // Partially decoded frame => the frame alpha type is unknown and should
-  // reflect the requested format.
-  premulDecoder->setData(partialData.get(), false);
-  ASSERT_TRUE(premulDecoder->frameCount());
-  unpremulDecoder->setData(partialData.get(), false);
-  ASSERT_TRUE(unpremulDecoder->frameCount());
-  ImageFrame* premulFrame = premulDecoder->frameBufferAtIndex(0);
-  EXPECT_TRUE(premulFrame &&
-              premulFrame->getStatus() != ImageFrame::FrameComplete);
-  EXPECT_EQ(premulFrame->bitmap().alphaType(), kPremul_SkAlphaType);
-  ImageFrame* unpremulFrame = unpremulDecoder->frameBufferAtIndex(0);
-  EXPECT_TRUE(unpremulFrame &&
-              unpremulFrame->getStatus() != ImageFrame::FrameComplete);
-  EXPECT_EQ(unpremulFrame->bitmap().alphaType(), kUnpremul_SkAlphaType);
-
-  // Fully decoded frame => the frame alpha type is known (opaque).
   premulDecoder->setData(fullData.get(), true);
   ASSERT_TRUE(premulDecoder->frameCount());
   unpremulDecoder->setData(fullData.get(), true);
   ASSERT_TRUE(unpremulDecoder->frameCount());
-  premulFrame = premulDecoder->frameBufferAtIndex(0);
+  ImageFrame* premulFrame = premulDecoder->frameBufferAtIndex(0);
   EXPECT_TRUE(premulFrame &&
               premulFrame->getStatus() == ImageFrame::FrameComplete);
-  EXPECT_EQ(premulFrame->bitmap().alphaType(), kOpaque_SkAlphaType);
-  unpremulFrame = unpremulDecoder->frameBufferAtIndex(0);
+  EXPECT_EQ(premulFrame->bitmap().alphaType(), kPremul_SkAlphaType);

[msarett1, 2017/03/07 13:27:18]

I don't think radient.gif is one of those edge case images where we would have
to iterate over all of the pixels to find out that it is opaque (I agree, we
definitely shouldn't do that).

I think this is an image without a transparent index that we should know to mark
as opaque when the decode is complete.

[cblume, 2017/03/07 19:36:29]

You are right.
radient.gif is only marked as transparent while there actually are transparent
pixels because the first frame is partially decoded.

Do we want to correctly mark that it is opaque? If so we'll need to update
SkCodec to provide that information. I don't think there is any way for me to
know right now as a client of SkCodec.

I feel like maybe this could be included as a bool in the frame info? What do
you think?

[scroggo_chromium, 2017/03/07 21:35:08]

That sounds reasonable to me.

[scroggo_chromium, 2017/03/07 22:06:14]

Oh yeah - we already do report whether the first frame is opaque, in the
SkImageInfo. But we should report for later frames as well.

+  ImageFrame* unpremulFrame = unpremulDecoder->frameBufferAtIndex(0);
   EXPECT_TRUE(unpremulFrame &&
               unpremulFrame->getStatus() == ImageFrame::FrameComplete);
-  EXPECT_EQ(unpremulFrame->bitmap().alphaType(), kOpaque_SkAlphaType);
+  EXPECT_EQ(unpremulFrame->bitmap().alphaType(), kUnpremul_SkAlphaType);
 }
 
 }  // namespace blink