GIF: Only report a frame after knowing dependency

Previously, getFrameInfo might report a frame that was truncated prior
to setting its requiredFrame. As a result, fRequiredFrame may be
different depending on how much data has already been received.

If there is a local color table, do not report the frame until the
color table has been received, since that is used to determine
fRequiredFrame. If there is no local color table, set fRequiredFrame
and report the frame after reading the header.

Add a test.

Replace make_from_resource with GetResourceAsData

Change-Id: I1b697f766c1d0e1e12ab2ae1d27167af5193395d
Reviewed-on: https://skia-review.googlesource.com/7756
Reviewed-by: Matt Sarett <msarett@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>

2 files changed, 27 insertions, 10 deletions

diff --git a/third_party/gif/SkGifImageReader.cpp b/third_party/gif/SkGifImageReader.cpp
index 2603807cb3..f617ccdc2a 100644
--- a/third_party/gif/SkGifImageReader.cpp
+++ b/third_party/gif/SkGifImageReader.cpp
@@ -457,7 +457,6 @@ bool SkGifImageReader::parse(SkGifImageReader::SkGIFParseQuery query)
         case SkGIFLZWStart: {
             SkASSERT(!m_frames.empty());
             auto* currentFrame = m_frames.back().get();
-            setRequiredFrame(currentFrame);
 
             currentFrame->setDataSize(this->getOneByte());
             GETN(1, SkGIFSubBlock);
@@ -800,14 +799,17 @@ bool SkGifImageReader::parse(SkGifImageReader::SkGIFParseQuery query)
                 break;
             }
 
+            setRequiredFrame(currentFrame);
             GETN(1, SkGIFLZWStart);
             break;
         }
 
         case SkGIFImageColormap: {
             SkASSERT(!m_frames.empty());
-            auto& cmap = m_frames.back()->localColorMap();
+            auto* currentFrame = m_frames.back().get();
+            auto& cmap = currentFrame->localColorMap();
             cmap.setTablePosition(m_streamBuffer.markPosition());
+            setRequiredFrame(currentFrame);
             GETN(1, SkGIFLZWStart);
             break;
         }
diff --git a/third_party/gif/SkGifImageReader.h b/third_party/gif/SkGifImageReader.h
index c51f4a8983..24c917af2a 100644
--- a/third_party/gif/SkGifImageReader.h
+++ b/third_party/gif/SkGifImageReader.h
@@ -202,7 +202,7 @@ public:
         , m_height(0)
         , m_transparentPixel(SkGIFColorMap::kNotFound)
         , m_disposalMethod(SkCodecAnimation::Keep_DisposalMethod)
-        , m_requiredFrame(SkCodec::kNone)
+        , m_requiredFrame(kUninitialized)
         , m_dataSize(0)
         , m_progressiveDisplay(false)
         , m_interlaced(false)
@@ -242,8 +242,13 @@ public:
     void setTransparentPixel(size_t pixel) { m_transparentPixel = pixel; }
     SkCodecAnimation::DisposalMethod getDisposalMethod() const { return m_disposalMethod; }
     void setDisposalMethod(SkCodecAnimation::DisposalMethod disposalMethod) { m_disposalMethod = disposalMethod; }
-    size_t getRequiredFrame() const { return m_requiredFrame; }
+
+    size_t getRequiredFrame() const {
+        SkASSERT(this->reachedStartOfData());
+        return m_requiredFrame;
+    }
     void setRequiredFrame(size_t req) { m_requiredFrame = req; }
+
     unsigned delayTime() const { return m_delayTime; }
     void setDelayTime(unsigned delay) { m_delayTime = delay; }
     bool isComplete() const { return m_isComplete; }
@@ -266,7 +271,11 @@ public:
     const SkGIFColorMap& localColorMap() const { return m_localColorMap; }
     SkGIFColorMap& localColorMap() { return m_localColorMap; }
 
+    bool reachedStartOfData() const { return m_requiredFrame != kUninitialized; }
+
 private:
+    static constexpr size_t kUninitialized = static_cast<size_t>(-2);
+
     int m_frameId;
     unsigned m_xOffset;
     unsigned m_yOffset; // With respect to "screen" origin.
@@ -346,13 +355,19 @@ public:
 
     size_t imagesCount() const
     {
-        if (m_frames.empty())
-            return 0;
+        // Report the first frame immediately, so the parser can stop when it
+        // sees the size on a SizeQuery.
+        const size_t frames = m_frames.size();
+        if (frames <= 1) {
+            return frames;
+        }
 
-        // This avoids counting an empty frame when the file is truncated right after
-        // SkGIFControlExtension but before SkGIFImageHeader.
-        // FIXME: This extra complexity is not necessary and we should just report m_frames.size().
-        return m_frames.back()->isHeaderDefined() ? m_frames.size() : m_frames.size() - 1;
+        // This avoids counting an empty frame when the file is truncated (or
+        // simply not yet complete) after receiving SkGIFControlExtension (and
+        // possibly SkGIFImageHeader) but before reading the color table. This
+        // ensures that we do not count a frame before we know its required
+        // frame.
+        return m_frames.back()->reachedStartOfData() ? frames : frames - 1;
     }
     int loopCount() const {
         if (cLoopCountNotSeen == m_loopCount) {