Never consolidate data in ICO decoder

Follows the same approach as BMPImageDecoder: Use a
FastSharedBufferReader to avoid consolidation.

Add an optional offset to PNGImageDecoder for ICOImageDecoder, so that
PNGImageDecoder can read a PNG subimage in an ICO without copying the
data. This eliminates another call to SharedBuffer::data(), as well as
a copy to create the separate SharedBuffer for PNG.

BUG=467772
BUG=528625

Review URL: https://codereview.chromium.org/1316203008

git-svn-id: svn://svn.chromium.org/blink/trunk@201981 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoder.cpp

@@ -46,6 +46,7 @@ static const size_t sizeOfDirEntry = 16;
 
 ICOImageDecoder::ICOImageDecoder(AlphaOption alphaOption, GammaAndColorProfileOption colorOptions, size_t maxDecodedBytes)
     : ImageDecoder(alphaOption, colorOptions, maxDecodedBytes)
+    , m_fastReader(nullptr)
     , m_decodedOffset(0)
 {
 }
@@ -56,6 +57,8 @@ ICOImageDecoder::~ICOImageDecoder()
 
 void ICOImageDecoder::onSetData(SharedBuffer* data)
 {
+    m_fastReader.setData(data);
+
     for (BMPReaders::iterator i(m_bmpReaders.begin()); i != m_bmpReaders.end(); ++i) {
         if (*i)
             (*i)->setData(data);
@@ -126,12 +129,7 @@ void ICOImageDecoder::setDataForPNGDecoderAtIndex(size_t index)
     if (!m_pngDecoders[index])
         return;
 
-    const IconDirectoryEntry& dirEntry = m_dirEntries[index];
-    // Copy out PNG data to a separate vector and send to the PNG decoder.
-    // FIXME: Save this copy by making the PNG decoder able to take an
-    // optional offset.
-    RefPtr<SharedBuffer> pngData(SharedBuffer::create(&m_data->data()[dirEntry.m_imageOffset], m_data->size() - dirEntry.m_imageOffset));
-    m_pngDecoders[index]->setData(pngData.get(), isAllDataReceived());
+    m_pngDecoders[index]->setData(m_data.get(), isAllDataReceived());
 }
 
 void ICOImageDecoder::decode(size_t index, bool onlySize)
@@ -186,11 +184,12 @@ bool ICOImageDecoder::decodeAtIndex(size_t index)
     }
 
     if (!m_pngDecoders[index]) {
-        m_pngDecoders[index] = adoptPtr(
-            new PNGImageDecoder(m_premultiplyAlpha ? AlphaPremultiplied : AlphaNotPremultiplied,
-                m_ignoreGammaAndColorProfile ? GammaAndColorProfileIgnored : GammaAndColorProfileApplied, m_maxDecodedBytes));
+        AlphaOption alphaOption = m_premultiplyAlpha ? AlphaPremultiplied : AlphaNotPremultiplied;
+        GammaAndColorProfileOption colorOptions = m_ignoreGammaAndColorProfile ? GammaAndColorProfileIgnored : GammaAndColorProfileApplied;
+        m_pngDecoders[index] = adoptPtr(new PNGImageDecoder(alphaOption, colorOptions, m_maxDecodedBytes, dirEntry.m_imageOffset));
         setDataForPNGDecoderAtIndex(index);
     }
+
     // Fail if the size the PNGImageDecoder calculated does not match the size
     // in the directory.
     if (m_pngDecoders[index]->isSizeAvailable() && (m_pngDecoders[index]->size() != dirEntry.m_size))
@@ -253,14 +252,14 @@ bool ICOImageDecoder::processDirectoryEntries()
 ICOImageDecoder::IconDirectoryEntry ICOImageDecoder::readDirectoryEntry()
 {
     // Read icon data.
-    // The casts to uint8_t in the next few lines are because that's the on-disk
-    // type of the width and height values.  Storing them in ints (instead of
-    // matching uint8_ts) is so we can record dimensions of size 256 (which is
-    // what a zero byte really means).
-    int width = static_cast<uint8_t>(m_data->data()[m_decodedOffset]);
+    // The following calls to readUint8() return a uint8_t, which is appropriate
+    // because that's the on-disk type of the width and height values.  Storing
+    // them in ints (instead of matching uint8_ts) is so we can record dimensions
+    // of size 256 (which is what a zero byte really means).
+    int width = readUint8(0);
     if (!width)
         width = 256;
-    int height = static_cast<uint8_t>(m_data->data()[m_decodedOffset + 1]);
+    int height = readUint8(1);
     if (!height)
         height = 256;
     IconDirectoryEntry entry;
@@ -279,7 +278,7 @@ ICOImageDecoder::IconDirectoryEntry ICOImageDecoder::readDirectoryEntry()
     // this isn't quite what the bitmap info header says later, as we only use
     // this value to determine which icon entry is best.
     if (!entry.m_bitCount) {
-        int colorCount = static_cast<uint8_t>(m_data->data()[m_decodedOffset + 2]);
+        int colorCount = readUint8(2);
         if (!colorCount)
             colorCount = 256;  // Vague in the spec, needed by real-world icons.
         for (--colorCount; colorCount; colorCount >>= 1)
@@ -298,7 +297,9 @@ ICOImageDecoder::ImageType ICOImageDecoder::imageTypeAtIndex(size_t index)
     const uint32_t imageOffset = m_dirEntries[index].m_imageOffset;
     if ((imageOffset > m_data->size()) || ((m_data->size() - imageOffset) < 4))
         return Unknown;
-    return strncmp(&m_data->data()[imageOffset], "\x89PNG", 4) ? BMP : PNG;
+    char buffer[4];
+    const char* data = m_fastReader.getConsecutiveData(imageOffset, 4, buffer);
+    return strncmp(data, "\x89PNG", 4) ? BMP : PNG;
 }
 
 }

third_party/WebKit/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?
+    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.

third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp

@@ -89,9 +89,9 @@ namespace blink {
 class PNGImageReader {
     WTF_MAKE_FAST_ALLOCATED(PNGImageReader);
 public:
-    PNGImageReader(PNGImageDecoder* decoder)
+    PNGImageReader(PNGImageDecoder* decoder, unsigned readOffset)
         : m_decoder(decoder)
-        , m_readOffset(0)
+        , m_readOffset(readOffset)
         , m_currentBufferSize(0)
         , m_decodingSizeOnly(false)
         , m_hasAlpha(false)
@@ -204,9 +204,10 @@ private:
 #endif
 };
 
-PNGImageDecoder::PNGImageDecoder(AlphaOption alphaOption, GammaAndColorProfileOption colorOptions, size_t maxDecodedBytes)
+PNGImageDecoder::PNGImageDecoder(AlphaOption alphaOption, GammaAndColorProfileOption colorOptions, size_t maxDecodedBytes, unsigned offset)
     : ImageDecoder(alphaOption, colorOptions, maxDecodedBytes)
     , m_hasColorProfile(false)
+    , m_offset(offset)
 {
 }
 
@@ -495,7 +496,7 @@ void PNGImageDecoder::decode(bool onlySize)
         return;
 
     if (!m_reader)
-        m_reader = adoptPtr(new PNGImageReader(this));
+        m_reader = adoptPtr(new PNGImageReader(this, m_offset));
 
     // If we couldn't decode the image but have received all the data, decoding
     // has failed.

third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.h

@@ -37,7 +37,7 @@ class PNGImageReader;
 class PLATFORM_EXPORT PNGImageDecoder : public ImageDecoder {
     WTF_MAKE_NONCOPYABLE(PNGImageDecoder);
 public:
-    PNGImageDecoder(AlphaOption, GammaAndColorProfileOption, size_t maxDecodedBytes);
+    PNGImageDecoder(AlphaOption, GammaAndColorProfileOption, size_t maxDecodedBytes, unsigned offset = 0);
     ~PNGImageDecoder() override;
 
     // ImageDecoder:
@@ -61,6 +61,7 @@ private:
 
     OwnPtr<PNGImageReader> m_reader;
     bool m_hasColorProfile;
+    const unsigned m_offset;
 };
 
 } // namespace blink