More tolerant about malformed GIF files

Source/WebKit/chromium/tests/GIFImageDecoderTest.cpp

Index: Source/WebKit/chromium/tests/GIFImageDecoderTest.cpp
diff --git a/Source/WebKit/chromium/tests/GIFImageDecoderTest.cpp b/Source/WebKit/chromium/tests/GIFImageDecoderTest.cpp
index e7e7172fde2ff5c1674af90bc0e2a7fc429499de..bfd2a1ca072300f238144764bce816b752295c91 100644
--- a/Source/WebKit/chromium/tests/GIFImageDecoderTest.cpp
+++ b/Source/WebKit/chromium/tests/GIFImageDecoderTest.cpp
@@ -186,10 +186,10 @@ TEST(GIFImageDecoderTest, brokenSecondFrame)
     ASSERT_TRUE(data.get());
     decoder->setData(data.get(), true);
 
-    EXPECT_EQ(0u, decoder->frameCount());
-    ImageFrame* frame = decoder->frameBufferAtIndex(0);
+    // One frame is detected but cannot be decoded.
+    EXPECT_EQ(1u, decoder->frameCount());
+    ImageFrame* frame = decoder->frameBufferAtIndex(1);
     EXPECT_FALSE(frame);
-    EXPECT_EQ(cAnimationLoopOnce, decoder->repetitionCount());
 }
 
 TEST(GIFImageDecoderTest, progressiveDecode)
@@ -297,6 +297,44 @@ TEST(GIFImageDecoderTest, frameIsCompleteLoading)
     EXPECT_TRUE(decoder->frameIsCompleteAtIndex(1));
 }
 
+TEST(GIFImageDecoderTest, badImageSeparator)
+{
+    RefPtr<SharedBuffer> fullData = readFile("/LayoutTests/fast/images/resources/animated.gif");
+    ASSERT_TRUE(fullData.get());
+    const size_t fullLength = fullData->size();
+
+    OwnPtr<GIFImageDecoder> decoder(createDecoder());
+    decoder->setData(fullData.get(), true);
+    EXPECT_EQ(2u, decoder->frameCount());
+    ImageFrame* frame = decoder->frameBufferAtIndex(0);
+    ASSERT_TRUE(frame);
+    EXPECT_EQ(ImageFrame::FrameComplete, frame->status());
+    const unsigned referenceHash = hashSkBitmap(frame->getSkBitmap());
+
+    // Replace the second image separator with an invalid character.
+    RefPtr<SharedBuffer> badData = SharedBuffer::create();
+    int count = 0;
+    for (size_t i = 0; i < fullLength; ++i) {
+        if (fullData->data()[i] == ',') {
+            if (count++ == 1) {

[Peter Kasting, 2013/05/29 04:30:17]

Nit: I think "if (++count == 2)" is slightly clearer, and "++count;  if (count
== 2)" is even better; but,

Better than all this would be if we could do the equivalent of find() within the
SharedBuffer to locate the commas and then append blocks of data at once instead
of going character-by-character; but,

Even better would be to just check a known-bad image into the tree and load that
here, and scrap the algorithm entirely.

[Alpha Left Google, 2013/05/29 19:38:58]

Done. SharedBuffer return const char* so a copy is required. I also try to avoid
adding more data files and reuse existing ones.

[Peter Kasting, 2013/05/29 20:02:53]

Why?  A file to test this could be a few hundred bytes.  Seems much clearer and
more maintainable than code.

After all, we aadd test files to test other bugs all the time.

+                badData->append("=", 1);
+                continue;
+            }
+        }
+        badData->append(fullData->data() + i, 1);
+    }
+
+    decoder = createDecoder();
+    decoder->setData(badData.get(), true);
+    EXPECT_EQ(1u, decoder->frameCount());
+    frame = decoder->frameBufferAtIndex(0);
+    ASSERT_TRUE(frame);
+    EXPECT_EQ(ImageFrame::FrameComplete, frame->status());
+    const unsigned hash = hashSkBitmap(frame->getSkBitmap());
+
+    EXPECT_EQ(referenceHash, hash);
+}
+
 #endif
 
 } // namespace