GIF: Avoid copying/storing data when possible

If the input SkStream has a length and position, do not copy and store
LZW blocks or ColorMaps. Instead, mark the position and size, and read
from the stream when necessary.

This will save memory in Chromium's use case, which has already
buffered all of its data.

In the case where we *do* need to copy, store it on the SkStreamBuffer.
This allows SkGifImageReader to have simpler code.

Add tests.

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

9 files changed, 419 insertions, 122 deletions

diff --git a/gn/tests.gni b/gn/tests.gni
index 79fd4be34c..95e1a7980c 100644
--- a/gn/tests.gni
+++ b/gn/tests.gni
@@ -216,6 +216,7 @@ tests_sources = [
   "$_tests/SRGBMipMapTest.cpp",
   "$_tests/SRGBReadWritePixelsTest.cpp",
   "$_tests/SRGBTest.cpp",
+  "$_tests/StreamBufferTest.cpp",
   "$_tests/StreamTest.cpp",
   "$_tests/StringTest.cpp",
   "$_tests/StrokerTest.cpp",
diff --git a/src/codec/SkStreamBuffer.cpp b/src/codec/SkStreamBuffer.cpp
index 1e6063706e..754065d97a 100644
--- a/src/codec/SkStreamBuffer.cpp
+++ b/src/codec/SkStreamBuffer.cpp
@@ -9,15 +9,79 @@
 
 SkStreamBuffer::SkStreamBuffer(SkStream* stream)
     : fStream(stream)
+    , fPosition(0)
     , fBytesBuffered(0)
+    , fHasLengthAndPosition(stream->hasLength() && stream->hasPosition())
+    , fTrulyBuffered(0)
 {}
 
-size_t SkStreamBuffer::buffer(size_t bytesToBuffer) {
+SkStreamBuffer::~SkStreamBuffer() {
+    fMarkedData.foreach([](size_t, SkData** data) { (*data)->unref(); });
+}
+
+const char* SkStreamBuffer::get() const {
+    SkASSERT(fBytesBuffered >= 1);
+    if (fHasLengthAndPosition && fTrulyBuffered < fBytesBuffered) {
+        const size_t bytesToBuffer = fBytesBuffered - fTrulyBuffered;
+        char* dst = SkTAddOffset<char>(const_cast<char*>(fBuffer), fTrulyBuffered);
+        SkDEBUGCODE(const size_t bytesRead =)
+        // This stream is rewindable, so it should be safe to call the non-const
+        // read()
+        const_cast<SkStream*>(fStream.get())->read(dst, bytesToBuffer);
+        SkASSERT(bytesRead == bytesToBuffer);
+        fTrulyBuffered = fBytesBuffered;
+    }
+    return fBuffer;
+}
+
+bool SkStreamBuffer::buffer(size_t totalBytesToBuffer) {
     // FIXME (scroggo): What should we do if the client tries to read too much?
     // Should not be a problem in GIF.
-    SkASSERT(fBytesBuffered + bytesToBuffer <= kMaxSize);
+    SkASSERT(totalBytesToBuffer <= kMaxSize);
+
+    if (totalBytesToBuffer <= fBytesBuffered) {
+        return true;
+    }
+
+    if (fHasLengthAndPosition) {
+        const size_t remaining = fStream->getLength() - fStream->getPosition() + fTrulyBuffered;
+        fBytesBuffered = SkTMin(remaining, totalBytesToBuffer);
+    } else {
+        const size_t extraBytes = totalBytesToBuffer - fBytesBuffered;
+        const size_t bytesBuffered = fStream->read(fBuffer + fBytesBuffered, extraBytes);
+        fBytesBuffered += bytesBuffered;
+    }
+    return fBytesBuffered == totalBytesToBuffer;
+}
+
+size_t SkStreamBuffer::markPosition() {
+    SkASSERT(fBytesBuffered >= 1);
+    if (!fHasLengthAndPosition) {
+        sk_sp<SkData> data(SkData::MakeWithCopy(fBuffer, fBytesBuffered));
+        SkASSERT(nullptr == fMarkedData.find(fPosition));
+        fMarkedData.set(fPosition, data.release());
+    }
+    return fPosition;
+}
+
+sk_sp<SkData> SkStreamBuffer::getDataAtPosition(size_t position, size_t length) {
+    if (!fHasLengthAndPosition) {
+        SkData** data = fMarkedData.find(position);
+        SkASSERT(data);
+        SkASSERT((*data)->size() == length);
+        return sk_ref_sp<SkData>(*data);
+    }
+
+    SkASSERT(position + length <= fStream->getLength());
+
+    const size_t oldPosition = fStream->getPosition();
+    if (!fStream->seek(position)) {
+        return nullptr;
+    }
 
-    const size_t bytesBuffered = fStream->read(fBuffer + fBytesBuffered, bytesToBuffer);
-    fBytesBuffered += bytesBuffered;
-    return bytesBuffered;
+    sk_sp<SkData> data(SkData::MakeUninitialized(length));
+    void* dst = data->writable_data();
+    const bool success = fStream->read(dst, length) == length;
+    fStream->seek(oldPosition);
+    return success ? data : nullptr;
 }
diff --git a/src/codec/SkStreamBuffer.h b/src/codec/SkStreamBuffer.h
index f04ddcbfd8..2b60775dd2 100644
--- a/src/codec/SkStreamBuffer.h
+++ b/src/codec/SkStreamBuffer.h
@@ -8,8 +8,10 @@
 #ifndef SkStreamBuffer_DEFINED
 #define SkStreamBuffer_DEFINED
 
+#include "SkData.h"
 #include "SkStream.h"
 #include "SkTypes.h"
+#include "../private/SkTHash.h"
 
 /**
  *  Helper class for reading from a stream that may not have all its data
@@ -25,28 +27,29 @@ public:
     // Takes ownership of the SkStream.
     SkStreamBuffer(SkStream*);
 
-    /**
-     *  Return a pointer the buffered data.
-     *
-     *  The number of bytes buffered is the sum of values returned by calls to
-     *  buffer() since the last call to flush().
-     */
-    const char* get() const { SkASSERT(fBytesBuffered >= 1); return fBuffer; }
+    ~SkStreamBuffer();
 
     /**
-     *  Bytes in the buffer.
+     *  Return a pointer the buffered data.
      *
-     *  Sum of the values returned by calls to buffer() since the last call to
-     *  flush().
+     *  The number of bytes buffered is the number passed to buffer()
+     *  after the last call to flush().
      */
-    size_t bytesBuffered() const { return fBytesBuffered; }
+    const char* get() const;
 
     /**
      *  Buffer from the stream into our buffer.
      *
-     *  Returns the number of bytes successfully buffered.
+     *  If this call returns true, get() can be used to access |bytes| bytes
+     *  from the stream. In addition, markPosition() can be called to mark this
+     *  position and enable calling getAtPosition() later to retrieve |bytes|
+     *  bytes.
+     *
+     *  @param bytes Total number of bytes desired.
+     *
+     *  @return Whether all bytes were successfully buffered.
      */
-    size_t buffer(size_t bytes);
+    bool buffer(size_t bytes);
 
     /**
      *  Flush the buffer.
@@ -54,15 +57,62 @@ public:
      *  After this call, no bytes are buffered.
      */
     void flush() {
+        if (fHasLengthAndPosition) {
+            if (fTrulyBuffered < fBytesBuffered) {
+                fStream->move(fBytesBuffered - fTrulyBuffered);
+            }
+            fTrulyBuffered = 0;
+        }
+        fPosition += fBytesBuffered;
         fBytesBuffered = 0;
     }
 
+    /**
+     *  Mark the current position in the stream to return to it later.
+     *
+     *  This is the position of the start of the buffer. After this call, a
+     *  a client can call getDataAtPosition to retrieve all the bytes currently
+     *  buffered.
+     *
+     *  @return size_t Position which can be passed to getDataAtPosition later
+     *      to retrieve the data currently buffered.
+     */
+    size_t markPosition();
+
+    /**
+     *  Retrieve data at position, as previously marked by markPosition().
+     *
+     *  @param position Position to retrieve data, as marked by markPosition().
+     *  @param length   Amount of data required at position.
+     *  @return SkData The data at position.
+     */
+    sk_sp<SkData> getDataAtPosition(size_t position, size_t length);
+
 private:
     static constexpr size_t kMaxSize = 256 * 3;
 
     std::unique_ptr<SkStream>   fStream;
+    size_t                      fPosition;
     char                        fBuffer[kMaxSize];
     size_t                      fBytesBuffered;
+    // If the stream has a length and position, we can make two optimizations:
+    // - We can skip buffering
+    // - During parsing, we can store the position and size of data that is
+    //   needed later during decoding.
+    const bool                  fHasLengthAndPosition;
+    // When fHasLengthAndPosition is true, we do not need to actually buffer
+    // inside buffer(). We'll buffer inside get(). This keeps track of how many
+    // bytes we've buffered inside get(), for the (non-existent) case of:
+    //  buffer(n)
+    //  get()
+    //  buffer(n + u)
+    //  get()
+    // The second call to get() needs to only truly buffer the part that was
+    // not already buffered.
+    mutable size_t              fTrulyBuffered;
+    // Only used if !fHasLengthAndPosition. In that case, markPosition will
+    // copy into an SkData, stored here.
+    SkTHashMap<size_t, SkData*> fMarkedData;
 };
 #endif // SkStreamBuffer_DEFINED
 
diff --git a/tests/CodecPartialTest.cpp b/tests/CodecPartialTest.cpp
index 93e5d63f22..62f9d8570e 100644
--- a/tests/CodecPartialTest.cpp
+++ b/tests/CodecPartialTest.cpp
@@ -12,6 +12,7 @@
 #include "SkRWBuffer.h"
 #include "SkString.h"
 
+#include "FakeStreams.h"
 #include "Resources.h"
 #include "Test.h"
 
@@ -50,46 +51,6 @@ static void compare_bitmaps(skiatest::Reporter* r, const SkBitmap& bm1, const Sk
     }
 }
 
-/*
- *  Represents a stream without all of its data.
- */
-class HaltingStream : public SkStream {
-public:
-    HaltingStream(sk_sp<SkData> data, size_t initialLimit)
-        : fTotalSize(data->size())
-        , fLimit(initialLimit)
-        , fStream(std::move(data))
-    {}
-
-    void addNewData(size_t extra) {
-        fLimit = SkTMin(fTotalSize, fLimit + extra);
-    }
-
-    size_t read(void* buffer, size_t size) override {
-        if (fStream.getPosition() + size > fLimit) {
-            size = fLimit - fStream.getPosition();
-        }
-
-        return fStream.read(buffer, size);
-    }
-
-    bool isAtEnd() const override {
-        return fStream.isAtEnd();
-    }
-
-    bool hasPosition() const override { return true; }
-    size_t getPosition() const override { return fStream.getPosition(); }
-    bool rewind() override { return fStream.rewind(); }
-    bool move(long offset) override { return fStream.move(offset); }
-
-    bool isAllDataReceived() const { return fLimit == fTotalSize; }
-
-private:
-    const size_t    fTotalSize;
-    size_t          fLimit;
-    SkMemoryStream  fStream;
-};
-
 static void test_partial(skiatest::Reporter* r, const char* name, size_t minBytes = 0) {
     sk_sp<SkData> file = make_from_resource(name);
     if (!file) {
diff --git a/tests/CodecTest.cpp b/tests/CodecTest.cpp
index 05859a95b3..19494b280d 100644
--- a/tests/CodecTest.cpp
+++ b/tests/CodecTest.cpp
@@ -5,6 +5,7 @@
  * found in the LICENSE file.
  */
 
+#include "FakeStreams.h"
 #include "Resources.h"
 #include "SkAndroidCodec.h"
 #include "SkBitmap.h"
@@ -891,35 +892,6 @@ private:
     const size_t   fLimit;
 };
 
-// Stream that is not an asset stream (!hasPosition() or !hasLength())
-class NotAssetMemStream : public SkStream {
-public:
-    NotAssetMemStream(sk_sp<SkData> data) : fStream(std::move(data)) {}
-
-    bool hasPosition() const override {
-        return false;
-    }
-
-    bool hasLength() const override {
-        return false;
-    }
-
-    size_t peek(void* buf, size_t bytes) const override {
-        return fStream.peek(buf, bytes);
-    }
-    size_t read(void* buf, size_t bytes) override {
-        return fStream.read(buf, bytes);
-    }
-    bool rewind() override {
-        return fStream.rewind();
-    }
-    bool isAtEnd() const override {
-        return fStream.isAtEnd();
-    }
-private:
-    SkMemoryStream fStream;
-};
-
 // Disable RAW tests for Win32.
 #if defined(SK_CODEC_DECODES_RAW) && (!defined(_WIN32))
 // Test that the RawCodec works also for not asset stream. This will test the code path using
diff --git a/tests/FakeStreams.h b/tests/FakeStreams.h
new file mode 100644
index 0000000000..6b510b8970
--- /dev/null
+++ b/tests/FakeStreams.h
@@ -0,0 +1,86 @@
+/*
+ * Copyright 2016 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "SkData.h"
+#include "SkStream.h"
+
+#ifndef FakeStreams_DEFINED
+#define FakeStreams_DEFINED
+
+// Stream that is not an asset stream (!hasPosition() or !hasLength())
+class NotAssetMemStream : public SkStream {
+public:
+    NotAssetMemStream(sk_sp<SkData> data) : fStream(std::move(data)) {}
+
+    bool hasPosition() const override {
+        return false;
+    }
+
+    bool hasLength() const override {
+        return false;
+    }
+
+    size_t peek(void* buf, size_t bytes) const override {
+        return fStream.peek(buf, bytes);
+    }
+    size_t read(void* buf, size_t bytes) override {
+        return fStream.read(buf, bytes);
+    }
+    bool rewind() override {
+        return fStream.rewind();
+    }
+    bool isAtEnd() const override {
+        return fStream.isAtEnd();
+    }
+private:
+    SkMemoryStream fStream;
+};
+
+/*
+ *  Represents a stream without all of its data.
+ */
+class HaltingStream : public SkStream {
+public:
+    HaltingStream(sk_sp<SkData> data, size_t initialLimit)
+        : fTotalSize(data->size())
+        , fLimit(initialLimit)
+        , fStream(std::move(data))
+    {}
+
+    void addNewData(size_t extra) {
+        fLimit = SkTMin(fTotalSize, fLimit + extra);
+    }
+
+    size_t read(void* buffer, size_t size) override {
+        if (fStream.getPosition() + size > fLimit) {
+            size = fLimit - fStream.getPosition();
+        }
+
+        return fStream.read(buffer, size);
+    }
+
+    bool isAtEnd() const override {
+        return fStream.isAtEnd();
+    }
+
+    bool hasLength() const override { return true; }
+    size_t getLength() const override { return fLimit; }
+
+    bool hasPosition() const override { return true; }
+    size_t getPosition() const override { return fStream.getPosition(); }
+    bool rewind() override { return fStream.rewind(); }
+    bool move(long offset) override { return fStream.move(offset); }
+    bool seek(size_t position) override { return fStream.seek(position); }
+
+    bool isAllDataReceived() const { return fLimit == fTotalSize; }
+
+private:
+    const size_t    fTotalSize;
+    size_t          fLimit;
+    SkMemoryStream  fStream;
+};
+#endif // FakeStreams_DEFINED
diff --git a/tests/StreamBufferTest.cpp b/tests/StreamBufferTest.cpp
new file mode 100644
index 0000000000..96bca35c92
--- /dev/null
+++ b/tests/StreamBufferTest.cpp
@@ -0,0 +1,135 @@
+/*
+ * Copyright 2016 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "SkData.h"
+#include "SkOSPath.h"
+#include "SkStream.h"
+#include "SkStreamBuffer.h"
+
+#include "FakeStreams.h"
+#include "Test.h"
+
+static const char* gText = "Four score and seven years ago";
+
+static void test_get_data_at_position(skiatest::Reporter* r, SkStreamBuffer* buffer, size_t position,
+                                    size_t length) {
+    sk_sp<SkData> data = buffer->getDataAtPosition(position, length);
+    REPORTER_ASSERT(r, data);
+    if (data) {
+        REPORTER_ASSERT(r, !memcmp(data->data(), gText + position, length));
+    }
+}
+
+// Test buffering from the beginning, by different amounts.
+static void test_buffer_from_beginning(skiatest::Reporter* r, SkStream* stream, size_t length) {
+    SkStreamBuffer buffer(stream);
+
+    // Buffer an arbitrary amount:
+    size_t buffered = length / 2;
+    REPORTER_ASSERT(r, buffer.buffer(buffered));
+    REPORTER_ASSERT(r, !memcmp(buffer.get(), gText, buffered));
+
+    // Buffering less is free:
+    REPORTER_ASSERT(r, buffer.buffer(buffered / 2));
+
+    // Buffer more should succeed:
+    REPORTER_ASSERT(r, buffer.buffer(length));
+    REPORTER_ASSERT(r, !memcmp(buffer.get(), gText, length));
+}
+
+// Test flushing the stream as we read.
+static void test_flushing(skiatest::Reporter* r, SkStream* stream, size_t length,
+                          bool getDataAtPosition) {
+    SkStreamBuffer buffer(stream);
+    const size_t step = 5;
+    for (size_t position = 0; position + step <= length; position += step) {
+        REPORTER_ASSERT(r, buffer.buffer(step));
+        REPORTER_ASSERT(r, buffer.markPosition() == position);
+
+        if (!getDataAtPosition) {
+            REPORTER_ASSERT(r, !memcmp(buffer.get(), gText + position, step));
+        }
+        buffer.flush();
+    }
+
+    REPORTER_ASSERT(r, !buffer.buffer(step));
+
+    if (getDataAtPosition) {
+        for (size_t position = 0; position + step <= length; position += step) {
+            test_get_data_at_position(r, &buffer, position, step);
+        }
+    }
+}
+
+DEF_TEST(StreamBuffer, r) {
+    const size_t size = strlen(gText);
+    sk_sp<SkData> data(SkData::MakeWithoutCopy(gText, size));
+
+    SkString tmpDir = skiatest::GetTmpDir();
+    const char* subdir = "streamBuffer.txt";
+    SkString path;
+
+    if (!tmpDir.isEmpty()) {
+        path = SkOSPath::Join(tmpDir.c_str(), subdir);
+        SkFILEWStream writer(path.c_str());
+        writer.write(gText, size);
+    }
+
+    struct {
+        std::function<SkStream*()> createStream;
+        bool                       skipIfNoTmpDir;
+    } factories[] = {
+        { [&data]() { return new SkMemoryStream(data); },       false },
+        { [&data]() { return new NotAssetMemStream(data); },    false },
+        { [&path]() { return new SkFILEStream(path.c_str()); }, true  },
+    };
+
+    for (auto f : factories) {
+        if (tmpDir.isEmpty() && f.skipIfNoTmpDir) {
+            continue;
+        }
+        test_buffer_from_beginning(r, f.createStream(), size);
+        test_flushing(r, f.createStream(), size, false);
+        test_flushing(r, f.createStream(), size, true);
+    }
+
+    // Stream that will receive more data. Will be owned by the SkStreamBuffer.
+    HaltingStream* stream = new HaltingStream(data, 6);
+    SkStreamBuffer buffer(stream);
+
+    // Can only buffer less than what's available (6).
+    REPORTER_ASSERT(r, !buffer.buffer(7));
+    REPORTER_ASSERT(r, buffer.buffer(5));
+    REPORTER_ASSERT(r, !memcmp(buffer.get(), gText, 5));
+
+    // Add some more data. We can buffer and read all of it.
+    stream->addNewData(8);
+    REPORTER_ASSERT(r, buffer.buffer(14));
+    REPORTER_ASSERT(r, !memcmp(buffer.get(), gText, 14));
+
+    // Flush the buffer, which moves the position.
+    buffer.flush();
+
+    // Add some data, and try to read more. Can only read what is
+    // available.
+    stream->addNewData(9);
+    REPORTER_ASSERT(r, !buffer.buffer(13));
+    stream->addNewData(4);
+    REPORTER_ASSERT(r, buffer.buffer(13));
+
+    // Do not call get on this data. We'll come back to this data after adding
+    // more.
+    buffer.flush();
+    const size_t remaining = size - 27;
+    REPORTER_ASSERT(r, remaining > 0);
+    stream->addNewData(remaining);
+    REPORTER_ASSERT(r, buffer.buffer(remaining));
+    REPORTER_ASSERT(r, !memcmp(buffer.get(), gText + 27, remaining));
+
+    // Now go back to the data we skipped.
+    test_get_data_at_position(r, &buffer, 14, 13);
+}
diff --git a/third_party/gif/SkGifImageReader.cpp b/third_party/gif/SkGifImageReader.cpp
index 9a14018734..ae5663f651 100644
--- a/third_party/gif/SkGifImageReader.cpp
+++ b/third_party/gif/SkGifImageReader.cpp
@@ -305,7 +305,8 @@ bool SkGIFLZWContext::doLZW(const unsigned char* block, size_t bytesInBlock)
     return true;
 }
 
-sk_sp<SkColorTable> SkGIFColorMap::buildTable(SkColorType colorType, size_t transparentPixel) const
+sk_sp<SkColorTable> SkGIFColorMap::buildTable(SkStreamBuffer* streamBuffer, SkColorType colorType,
+                                              size_t transparentPixel) const
 {
     if (!m_isDefined)
         return nullptr;
@@ -323,8 +324,14 @@ sk_sp<SkColorTable> SkGIFColorMap::buildTable(SkColorType colorType, size_t tran
     }
     m_packColorProc = proc;
 
+    const size_t bytes = m_colors * SK_BYTES_PER_COLORMAP_ENTRY;
+    sk_sp<SkData> rawData(streamBuffer->getDataAtPosition(m_position, bytes));
+    if (!rawData) {
+        return nullptr;
+    }
+
     SkASSERT(m_colors <= SK_MAX_COLORS);
-    const uint8_t* srcColormap = m_rawData->bytes();
+    const uint8_t* srcColormap = rawData->bytes();
     SkPMColor colorStorage[SK_MAX_COLORS];
     for (size_t i = 0; i < m_colors; i++) {
         if (i == transparentPixel) {
@@ -341,18 +348,19 @@ sk_sp<SkColorTable> SkGIFColorMap::buildTable(SkColorType colorType, size_t tran
     return m_table;
 }
 
-sk_sp<SkColorTable> SkGifImageReader::getColorTable(SkColorType colorType, size_t index) const {
+sk_sp<SkColorTable> SkGifImageReader::getColorTable(SkColorType colorType, size_t index) {
     if (index >= m_frames.size()) {
         return nullptr;
     }
 
     const SkGIFFrameContext* frameContext = m_frames[index].get();
     const SkGIFColorMap& localColorMap = frameContext->localColorMap();
+    const size_t transPix = frameContext->transparentPixel();
     if (localColorMap.isDefined()) {
-        return localColorMap.buildTable(colorType, frameContext->transparentPixel());
+        return localColorMap.buildTable(&m_streamBuffer, colorType, transPix);
     }
     if (m_globalColorMap.isDefined()) {
-        return m_globalColorMap.buildTable(colorType, frameContext->transparentPixel());
+        return m_globalColorMap.buildTable(&m_streamBuffer, colorType, transPix);
     }
     return nullptr;
 }
@@ -360,7 +368,8 @@ sk_sp<SkColorTable> SkGifImageReader::getColorTable(SkColorType colorType, size_
 // Perform decoding for this frame. frameComplete will be true if the entire frame is decoded.
 // Returns false if a decoding error occurred. This is a fatal error and causes the SkGifImageReader to set the "decode failed" flag.
 // Otherwise, either not enough data is available to decode further than before, or the new data has been decoded successfully; returns true in this case.
-bool SkGIFFrameContext::decode(SkGifCodec* client, bool* frameComplete)
+bool SkGIFFrameContext::decode(SkStreamBuffer* streamBuffer, SkGifCodec* client,
+                               bool* frameComplete)
 {
     *frameComplete = false;
     if (!m_lzwContext) {
@@ -379,8 +388,14 @@ bool SkGIFFrameContext::decode(SkGifCodec* client, bool* frameComplete)
 
     // Some bad GIFs have extra blocks beyond the last row, which we don't want to decode.
     while (m_currentLzwBlock < m_lzwBlocks.size() && m_lzwContext->hasRemainingRows()) {
-        if (!m_lzwContext->doLZW(reinterpret_cast<const unsigned char*>(m_lzwBlocks[m_currentLzwBlock]->data()),
-                                                                        m_lzwBlocks[m_currentLzwBlock]->size())) {
+        const auto& block = m_lzwBlocks[m_currentLzwBlock];
+        const size_t len = block.blockSize;
+
+        sk_sp<SkData> data(streamBuffer->getDataAtPosition(block.blockPosition, len));
+        if (!data) {
+            return false;
+        }
+        if (!m_lzwContext->doLZW(reinterpret_cast<const unsigned char*>(data->data()), len)) {
             return false;
         }
         ++m_currentLzwBlock;
@@ -402,7 +417,7 @@ bool SkGifImageReader::decode(size_t frameIndex, bool* frameComplete)
 {
     SkGIFFrameContext* currentFrame = m_frames[frameIndex].get();
 
-    return currentFrame->decode(m_client, frameComplete);
+    return currentFrame->decode(&m_streamBuffer, m_client, frameComplete);
 }
 
 // Parse incoming GIF data stream into internal data structures.
@@ -428,22 +443,19 @@ bool SkGifImageReader::parse(SkGifImageReader::SkGIFParseQuery query)
     }
 
     while (true) {
-        const size_t bytesBuffered = m_streamBuffer.buffer(m_bytesToConsume);
-        if (bytesBuffered < m_bytesToConsume) {
-            // The stream does not yet have enough data. Mark that we need less next time around,
-            // and return.
-            m_bytesToConsume -= bytesBuffered;
+        if (!m_streamBuffer.buffer(m_bytesToConsume)) {
+            // The stream does not yet have enough data.
             return true;
         }
 
         switch (m_state) {
-        case SkGIFLZW:
+        case SkGIFLZW: {
             SkASSERT(!m_frames.empty());
-            // FIXME: All this copying might be wasteful for e.g. SkMemoryStream
-            m_frames.back()->addLzwBlock(m_streamBuffer.get(), m_streamBuffer.bytesBuffered());
+            auto* frame = m_frames.back().get();
+            frame->addLzwBlock(m_streamBuffer.markPosition(), m_bytesToConsume);
             GETN(1, SkGIFSubBlock);
             break;
-
+        }
         case SkGIFLZWStart: {
             SkASSERT(!m_frames.empty());
             m_frames.back()->setDataSize(this->getOneByte());
@@ -494,7 +506,7 @@ bool SkGifImageReader::parse(SkGifImageReader::SkGIFParseQuery query)
         }
 
         case SkGIFGlobalColormap: {
-            m_globalColorMap.setRawData(m_streamBuffer.get(), m_streamBuffer.bytesBuffered());
+            m_globalColorMap.setTablePosition(m_streamBuffer.markPosition());
             GETN(1, SkGIFImageStart);
             break;
         }
@@ -631,7 +643,7 @@ bool SkGifImageReader::parse(SkGifImageReader::SkGIFParseQuery query)
 
         case SkGIFApplicationExtension: {
             // Check for netscape application extension.
-            if (m_streamBuffer.bytesBuffered() == 11) {
+            if (m_bytesToConsume == 11) {
                 const unsigned char* currentComponent =
                     reinterpret_cast<const unsigned char*>(m_streamBuffer.get());
 
@@ -789,7 +801,6 @@ bool SkGifImageReader::parse(SkGifImageReader::SkGIFParseQuery query)
                 // The decoder needs to stop, so we return here, before
                 // flushing the buffer. Next time through, we'll be in the same
                 // state, requiring the same amount in the buffer.
-                m_bytesToConsume = 0;
                 return true;
             }
 
@@ -820,7 +831,8 @@ bool SkGifImageReader::parse(SkGifImageReader::SkGIFParseQuery query)
 
         case SkGIFImageColormap: {
             SkASSERT(!m_frames.empty());
-            m_frames.back()->localColorMap().setRawData(m_streamBuffer.get(), m_streamBuffer.bytesBuffered());
+            auto& cmap = m_frames.back()->localColorMap();
+            cmap.setTablePosition(m_streamBuffer.markPosition());
             GETN(1, SkGIFLZWStart);
             break;
         }
diff --git a/third_party/gif/SkGifImageReader.h b/third_party/gif/SkGifImageReader.h
index 2843a3538f..0481ba2908 100644
--- a/third_party/gif/SkGifImageReader.h
+++ b/third_party/gif/SkGifImageReader.h
@@ -137,37 +137,51 @@ private:
     const SkGIFFrameContext* m_frameContext;
 };
 
+struct SkGIFLZWBlock {
+ public:
+  SkGIFLZWBlock(size_t position, size_t size)
+      : blockPosition(position), blockSize(size) {}
+
+  const size_t blockPosition;
+  const size_t blockSize;
+};
+
 class SkGIFColorMap final {
 public:
     SkGIFColorMap()
         : m_isDefined(false)
+        , m_position(0)
         , m_colors(0)
         , m_packColorProc(nullptr)
     {
     }
 
     void setNumColors(size_t colors) {
+        SkASSERT(!m_colors);
+        SkASSERT(!m_position);
+
         m_colors = colors;
     }
 
-    size_t numColors() const { return m_colors; }
+    void setTablePosition(size_t position) {
+        SkASSERT(!m_isDefined);
 
-    void setRawData(const char* data, size_t size)
-    {
-        // FIXME: Can we avoid this copy?
-        m_rawData = SkData::MakeWithCopy(data, size);
-        SkASSERT(m_colors * SK_BYTES_PER_COLORMAP_ENTRY == size);
+        m_position = position;
         m_isDefined = true;
     }
+
+    size_t numColors() const { return m_colors; }
+
     bool isDefined() const { return m_isDefined; }
 
     // Build RGBA table using the data stream.
-    sk_sp<SkColorTable> buildTable(SkColorType dstColorType, size_t transparentPixel) const;
+    sk_sp<SkColorTable> buildTable(SkStreamBuffer*, SkColorType dstColorType,
+                                   size_t transparentPixel) const;
 
 private:
     bool m_isDefined;
+    size_t m_position;
     size_t m_colors;
-    sk_sp<SkData> m_rawData;
     mutable PackColorProc m_packColorProc;
     mutable sk_sp<SkColorTable> m_table;
 };
@@ -201,12 +215,12 @@ public:
     {
     }
 
-    void addLzwBlock(const void* data, size_t size)
+    void addLzwBlock(size_t position, size_t size)
     {
-        m_lzwBlocks.push_back(SkData::MakeWithCopy(data, size));
+        m_lzwBlocks.push_back(SkGIFLZWBlock(position, size));
     }
 
-    bool decode(SkGifCodec* client, bool* frameDecoded);
+    bool decode(SkStreamBuffer*, SkGifCodec* client, bool* frameDecoded);
 
     int frameId() const { return m_frameId; }
     void setRect(unsigned x, unsigned y, unsigned width, unsigned height)
@@ -266,7 +280,9 @@ private:
     unsigned m_delayTime; // Display time, in milliseconds, for this image in a multi-image GIF.
 
     std::unique_ptr<SkGIFLZWContext> m_lzwContext;
-    std::vector<sk_sp<SkData>> m_lzwBlocks; // LZW blocks for this frame.
+    // LZW blocks for this frame.
+    std::vector<SkGIFLZWBlock> m_lzwBlocks;
+
     SkGIFColorMap m_localColorMap;
 
     size_t m_currentLzwBlock;
@@ -359,7 +375,7 @@ public:
     }
 
     // Return the color table for frame index (which may be the global color table).
-    sk_sp<SkColorTable> getColorTable(SkColorType dstColorType, size_t index) const;
+    sk_sp<SkColorTable> getColorTable(SkColorType dstColorType, size_t index);
 
     bool firstFrameHasAlpha() const { return m_firstFrameHasAlpha; }