Never consolidate data in ICO decoder

Source/platform/image-decoders/ico/ICOImageDecoder.h

Index: Source/platform/image-decoders/ico/ICOImageDecoder.h
diff --git a/Source/platform/image-decoders/ico/ICOImageDecoder.h b/Source/platform/image-decoders/ico/ICOImageDecoder.h
index 5cc71c0c5e7c64a9f8cf3fc4025809c4e42bbae1..3e2923813b925de70c8025c7c8918cff36377b17 100644
--- a/Source/platform/image-decoders/ico/ICOImageDecoder.h
+++ b/Source/platform/image-decoders/ico/ICOImageDecoder.h
@@ -31,6 +31,7 @@
 #ifndef ICOImageDecoder_h
 #define ICOImageDecoder_h
 
+#include "platform/image-decoders/FastSharedBufferReader.h"
 #include "platform/image-decoders/bmp/BMPImageReader.h"
 
 namespace blink {
@@ -83,16 +84,24 @@ private:
     size_t decodeFrameCount() override;
     void decode(size_t index) override { decode(index, false); }
 
+    // TODO (scroggo): These functions are identical to functions in BMPImageReader. Share code?

[Peter Kasting, 2015/09/08 20:03:13]

Yeah, we should share the code.

This seems like another argument for making the buffer reader object part of
ImageDecoder.  Then we could make these functions protected members of
ImageDecoder as well.  We could also avoid having to customize setData() to
update the buffer reader in each decoder.

[scroggo_chromium, 2015/09/08 20:52:51]

On the other hand, not all of the classes need a FastSharedBufferReader (PNG and
JPEG do not need one; I have not looked at WEBP yet.)

[Peter Kasting, 2015/09/08 21:01:05]

Well, with PNG and JPEG, we rely on an external decoding library.  Doesn't that
library require consolidating the data passed to it?  So it's not so much "don't
need" as "as currently implemented these just unconditionally coalesce".

[scroggo_chromium, 2015/09/09 14:32:23]

Actually, no. JPEG does use an external library, but with my recent changes to
JPEGImageDecoder, libjpeg directly uses the data we give to it. So it will not
do any coalescing.

I haven't dug into PNG very much, but presumably it copies the data when we
supply it. It still doesn't need a FastSharedBufferReader. Maybe it would be
fine for it to have an unused one (it is only four fields, plus the RefPtr; it
also would mean some unnecessary calls to FastSharedBufferReader::setData, but
maybe this is all negligible).

+    inline uint8_t readUint8(size_t offset) const
+    {
+        return m_fastReader.getOneByte(m_decodedOffset + offset);
+    }
+
     inline uint16_t readUint16(int offset) const
     {
-        // TODO (scroggo): This consolidates the data, meaning unnecessary copies.
-        return BMPImageReader::readUint16(&m_data->data()[m_decodedOffset + offset]);
+        char buffer[2];
+        const char* data = m_fastReader.getConsecutiveData(m_decodedOffset + offset, 2, buffer);
+        return BMPImageReader::readUint16(data);
     }
 
     inline uint32_t readUint32(int offset) const
     {
-        // TODO (scroggo): This consolidates the data, meaning unnecessary copies.
-        return BMPImageReader::readUint32(&m_data->data()[m_decodedOffset + offset]);
+        char buffer[4];
+        const char* data = m_fastReader.getConsecutiveData(m_decodedOffset + offset, 4, buffer);
+        return BMPImageReader::readUint32(data);
     }
 
     // If the desired PNGImageDecoder exists, gives it the appropriate data.
@@ -132,6 +141,8 @@ private:
     // if the type could be determined.
     ImageType imageTypeAtIndex(size_t);
 
+    FastSharedBufferReader m_fastReader;
+
     // An index into |m_data| representing how much we've already decoded.
     // Note that this only tracks data _this_ class decodes; once the
     // BMPImageReader takes over this will not be updated further.