diff options
Diffstat (limited to 'WebCore/platform/image-decoders')
14 files changed, 169 insertions, 80 deletions
diff --git a/WebCore/platform/image-decoders/ImageDecoder.h b/WebCore/platform/image-decoders/ImageDecoder.h index b063db2..f32536c 100644 --- a/WebCore/platform/image-decoders/ImageDecoder.h +++ b/WebCore/platform/image-decoders/ImageDecoder.h @@ -41,6 +41,7 @@ #include "NativeImageSkia.h" #include "SkColorPriv.h" #elif PLATFORM(QT) +#include <QPixmap> #include <QImage> #endif @@ -136,8 +137,7 @@ namespace WebCore { } #if PLATFORM(QT) - void setDecodedImage(const QImage& image); - QImage decodedImage() const { return m_image; } + void setPixmap(const QPixmap& pixmap); #endif private: @@ -149,6 +149,8 @@ namespace WebCore { #if PLATFORM(SKIA) return m_bitmap.getAddr32(x, y); #elif PLATFORM(QT) + m_image = m_pixmap.toImage(); + m_pixmap = QPixmap(); return reinterpret_cast<QRgb*>(m_image.scanLine(y)) + x; #else return m_bytes.data() + (y * width()) + x; @@ -178,6 +180,7 @@ namespace WebCore { #if PLATFORM(SKIA) NativeImageSkia m_bitmap; #elif PLATFORM(QT) + mutable QPixmap m_pixmap; mutable QImage m_image; bool m_hasAlpha; IntSize m_size; @@ -236,7 +239,7 @@ namespace WebCore { virtual void setData(SharedBuffer* data, bool allDataReceived) { - if (failed()) + if (m_failed) return; m_data = data; m_isAllDataReceived = allDataReceived; @@ -305,7 +308,8 @@ namespace WebCore { // Sets the "decode failure" flag. For caller convenience (since so // many callers want to return false after calling this), returns false - // to enable easy tailcalling. + // to enable easy tailcalling. Subclasses may override this to also + // clean up any local data. virtual bool setFailed() { m_failed = true; diff --git a/WebCore/platform/image-decoders/bmp/BMPImageDecoder.cpp b/WebCore/platform/image-decoders/bmp/BMPImageDecoder.cpp index a6d36ef..901f60d 100644 --- a/WebCore/platform/image-decoders/bmp/BMPImageDecoder.cpp +++ b/WebCore/platform/image-decoders/bmp/BMPImageDecoder.cpp @@ -77,6 +77,12 @@ RGBA32Buffer* BMPImageDecoder::frameBufferAtIndex(size_t index) return buffer; } +bool BMPImageDecoder::setFailed() +{ + m_reader.clear(); + return ImageDecoder::setFailed(); +} + void BMPImageDecoder::decode(bool onlySize) { if (failed()) @@ -86,6 +92,10 @@ void BMPImageDecoder::decode(bool onlySize) // has failed. if (!decodeHelper(onlySize) && isAllDataReceived()) setFailed(); + // If we're done decoding the image, we don't need the BMPImageReader + // anymore. (If we failed, |m_reader| has already been cleared.) + else if (!m_frameBufferCache.isEmpty() && (m_frameBufferCache.first().status() == RGBA32Buffer::FrameComplete)) + m_reader.clear(); } bool BMPImageDecoder::decodeHelper(bool onlySize) diff --git a/WebCore/platform/image-decoders/bmp/BMPImageDecoder.h b/WebCore/platform/image-decoders/bmp/BMPImageDecoder.h index 15be0a2..b08b32b 100644 --- a/WebCore/platform/image-decoders/bmp/BMPImageDecoder.h +++ b/WebCore/platform/image-decoders/bmp/BMPImageDecoder.h @@ -46,6 +46,10 @@ namespace WebCore { virtual void setData(SharedBuffer*, bool allDataReceived); virtual bool isSizeAvailable(); virtual RGBA32Buffer* frameBufferAtIndex(size_t index); + // CAUTION: setFailed() deletes |m_reader|. Be careful to avoid + // accessing deleted memory, especially when calling this from inside + // BMPImageReader! + virtual bool setFailed(); private: inline uint32_t readUint32(int offset) const diff --git a/WebCore/platform/image-decoders/bmp/BMPImageReader.cpp b/WebCore/platform/image-decoders/bmp/BMPImageReader.cpp index 2f3bffa..93bedf3 100644 --- a/WebCore/platform/image-decoders/bmp/BMPImageReader.cpp +++ b/WebCore/platform/image-decoders/bmp/BMPImageReader.cpp @@ -79,7 +79,7 @@ bool BMPImageReader::decodeBMP(bool onlySize) ASSERT(m_buffer); // Parent should set this before asking us to decode! if (m_buffer->status() == RGBA32Buffer::FrameEmpty) { if (!m_buffer->setSize(m_parent->size().width(), m_parent->size().height())) - return setFailed(); // Unable to allocate. + return m_parent->setFailed(); // Unable to allocate. m_buffer->setStatus(RGBA32Buffer::FramePartial); // setSize() calls eraseARGB(), which resets the alpha flag, so we force // it back to false here. We'll set it true below in all cases where @@ -95,10 +95,11 @@ bool BMPImageReader::decodeBMP(bool onlySize) // Decode the data. if ((m_andMaskState != Decoding) && !pastEndOfImage(0)) { - if ((m_infoHeader.biCompression == RLE4) || (m_infoHeader.biCompression == RLE8) || (m_infoHeader.biCompression == RLE24)) { - if (!processRLEData()) - return false; - } else if (!processNonRLEData(false, 0)) + if ((m_infoHeader.biCompression != RLE4) && (m_infoHeader.biCompression != RLE8) && (m_infoHeader.biCompression != RLE24)) { + const ProcessingResult result = processNonRLEData(false, 0); + if (result != Success) + return (result == Failure) ? m_parent->setFailed() : false; + } else if (!processRLEData()) return false; } @@ -114,8 +115,11 @@ bool BMPImageReader::decodeBMP(bool onlySize) m_andMaskState = Decoding; } - if ((m_andMaskState == Decoding) && !processNonRLEData(false, 0)) - return false; + if (m_andMaskState == Decoding) { + const ProcessingResult result = processNonRLEData(false, 0); + if (result != Success) + return (result == Failure) ? m_parent->setFailed() : false; + } // Done! m_buffer->setStatus(RGBA32Buffer::FrameComplete); @@ -136,7 +140,7 @@ bool BMPImageReader::readInfoHeaderSize() // problematic or at least confusing in other places), or to overrun the // image data. if (((m_headerOffset + m_infoHeader.biSize) < m_headerOffset) || (m_imgDataOffset && (m_imgDataOffset < (m_headerOffset + m_infoHeader.biSize)))) - return setFailed(); + return m_parent->setFailed(); // See if this is a header size we understand: // OS/2 1.x: 12 @@ -149,7 +153,7 @@ bool BMPImageReader::readInfoHeaderSize() else if ((m_infoHeader.biSize >= 16) && (m_infoHeader.biSize <= 64) && (!(m_infoHeader.biSize & 3) || (m_infoHeader.biSize == 42) || (m_infoHeader.biSize == 46))) m_isOS22x = true; else - return setFailed(); + return m_parent->setFailed(); return true; } @@ -164,11 +168,11 @@ bool BMPImageReader::processInfoHeader() // Sanity-check header values. if (!isInfoHeaderValid()) - return setFailed(); + return m_parent->setFailed(); // Set our size. if (!m_parent->setSize(m_infoHeader.biWidth, m_infoHeader.biHeight)) - return setFailed(); + return false; // For paletted images, bitmaps can set biClrUsed to 0 to mean "all // colors", so set it to the maximum number of colors for this bit depth. @@ -228,7 +232,7 @@ bool BMPImageReader::readInfoHeader() m_infoHeader.biCompression = RLE24; m_isOS22x = true; } else if (biCompression > 5) - return setFailed(); // Some type we don't understand. + return m_parent->setFailed(); // Some type we don't understand. else m_infoHeader.biCompression = static_cast<CompressionType>(biCompression); } @@ -395,7 +399,7 @@ bool BMPImageReader::processBitmasks() // Fail if we don't have enough file space for the bitmasks. static const size_t SIZEOF_BITMASKS = 12; if (((m_headerOffset + m_infoHeader.biSize + SIZEOF_BITMASKS) < (m_headerOffset + m_infoHeader.biSize)) || (m_imgDataOffset && (m_imgDataOffset < (m_headerOffset + m_infoHeader.biSize + SIZEOF_BITMASKS)))) - return setFailed(); + return m_parent->setFailed(); // Read bitmasks. if ((m_data->size() - m_decodedOffset) < SIZEOF_BITMASKS) @@ -435,7 +439,7 @@ bool BMPImageReader::processBitmasks() // Make sure bitmask does not overlap any other bitmasks. for (int j = 0; j < i; ++j) { if (tempMask & m_bitMasks[j]) - return setFailed(); + return m_parent->setFailed(); } // Count offset into pixel data. @@ -448,7 +452,7 @@ bool BMPImageReader::processBitmasks() // Make sure bitmask is contiguous. if (tempMask) - return setFailed(); + return m_parent->setFailed(); // Since RGBABuffer tops out at 8 bits per channel, adjust the shift // amounts to use the most significant 8 bits of the channel. @@ -467,7 +471,7 @@ bool BMPImageReader::processColorTable() // Fail if we don't have enough file space for the color table. if (((m_headerOffset + m_infoHeader.biSize + m_tableSizeInBytes) < (m_headerOffset + m_infoHeader.biSize)) || (m_imgDataOffset && (m_imgDataOffset < (m_headerOffset + m_infoHeader.biSize + m_tableSizeInBytes)))) - return setFailed(); + return m_parent->setFailed(); // Read color table. if ((m_decodedOffset > m_data->size()) || ((m_data->size() - m_decodedOffset) < m_tableSizeInBytes)) @@ -529,10 +533,10 @@ bool BMPImageReader::processRLEData() const uint8_t count = m_data->data()[m_decodedOffset]; const uint8_t code = m_data->data()[m_decodedOffset + 1]; if ((count || (code != 1)) && pastEndOfImage(0)) - return setFailed(); + return m_parent->setFailed(); // Decode. - if (count == 0) { + if (!count) { switch (code) { case 0: // Magic token: EOL // Skip any remaining pixels in this row. @@ -562,7 +566,7 @@ bool BMPImageReader::processRLEData() if (dx || dy) m_buffer->setHasAlpha(true); if (((m_coord.x() + dx) > m_parent->size().width()) || pastEndOfImage(dy)) - return setFailed(); + return m_parent->setFailed(); // Skip intervening pixels. m_coord.move(dx, m_isTopDown ? dy : -dy); @@ -571,19 +575,23 @@ bool BMPImageReader::processRLEData() break; } - default: // Absolute mode + default: { // Absolute mode // |code| pixels specified as in BI_RGB, zero-padded at the end // to a multiple of 16 bits. // Because processNonRLEData() expects m_decodedOffset to // point to the beginning of the pixel data, bump it past // the escape bytes and then reset if decoding failed. m_decodedOffset += 2; - if (!processNonRLEData(true, code)) { + const ProcessingResult result = processNonRLEData(true, code); + if (result == Failure) + return m_parent->setFailed(); + if (result == InsufficientData) { m_decodedOffset -= 2; return false; } break; } + } } else { // Encoded mode // The following color data is repeated for |count| total pixels. // Strangely, some BMPs seem to specify excessively large counts @@ -608,7 +616,7 @@ bool BMPImageReader::processRLEData() colorIndexes[1] &= 0xf; } if ((colorIndexes[0] >= m_infoHeader.biClrUsed) || (colorIndexes[1] >= m_infoHeader.biClrUsed)) - return setFailed(); + return m_parent->setFailed(); for (int which = 0; m_coord.x() < endX; ) { setI(colorIndexes[which]); which = !which; @@ -620,10 +628,10 @@ bool BMPImageReader::processRLEData() } } -bool BMPImageReader::processNonRLEData(bool inRLE, int numPixels) +BMPImageReader::ProcessingResult BMPImageReader::processNonRLEData(bool inRLE, int numPixels) { if (m_decodedOffset > m_data->size()) - return false; + return InsufficientData; if (!inRLE) numPixels = m_parent->size().width(); @@ -631,7 +639,7 @@ bool BMPImageReader::processNonRLEData(bool inRLE, int numPixels) // Fail if we're being asked to decode more pixels than remain in the row. const int endX = m_coord.x() + numPixels; if (endX > m_parent->size().width()) - return setFailed(); + return Failure; // Determine how many bytes of data the requested number of pixels // requires. @@ -648,7 +656,7 @@ bool BMPImageReader::processNonRLEData(bool inRLE, int numPixels) while (!pastEndOfImage(0)) { // Bail if we don't have enough data for the desired number of pixels. if ((m_data->size() - m_decodedOffset) < paddedNumBytes) - return false; + return InsufficientData; if (m_infoHeader.biBitCount < 16) { // Paletted data. Pixels are stored little-endian within bytes. @@ -672,7 +680,7 @@ bool BMPImageReader::processNonRLEData(bool inRLE, int numPixels) m_coord.move(1, 0); } else { if (colorIndex >= m_infoHeader.biClrUsed) - return setFailed(); + return Failure; setI(colorIndex); } pixelData <<= m_infoHeader.biBitCount; @@ -713,12 +721,12 @@ bool BMPImageReader::processNonRLEData(bool inRLE, int numPixels) // Success, keep going. m_decodedOffset += paddedNumBytes; if (inRLE) - return true; + return Success; moveBufferToNextRow(); } // Finished decoding whole image. - return true; + return Success; } void BMPImageReader::moveBufferToNextRow() @@ -726,11 +734,4 @@ void BMPImageReader::moveBufferToNextRow() m_coord.move(-m_coord.x(), m_isTopDown ? 1 : -1); } -bool BMPImageReader::setFailed() -{ - m_parent->setFailed(); - m_colorTable.clear(); - return false; -} - } // namespace WebCore diff --git a/WebCore/platform/image-decoders/bmp/BMPImageReader.h b/WebCore/platform/image-decoders/bmp/BMPImageReader.h index a30a721..0a6dc84 100644 --- a/WebCore/platform/image-decoders/bmp/BMPImageReader.h +++ b/WebCore/platform/image-decoders/bmp/BMPImageReader.h @@ -97,6 +97,11 @@ namespace WebCore { NotYetDecoded, Decoding, }; + enum ProcessingResult { + Success, + Failure, + InsufficientData, + }; // These are based on the Windows BITMAPINFOHEADER and RGBTRIPLE // structs, but with unnecessary entries removed. @@ -161,14 +166,18 @@ namespace WebCore { // Processes a set of non-RLE-compressed pixels. Two cases: // * inRLE = true: the data is inside an RLE-encoded bitmap. Tries to - // process |numPixels| pixels on the current row; returns true on - // success. + // process |numPixels| pixels on the current row. // * inRLE = false: the data is inside a non-RLE-encoded bitmap. // |numPixels| is ignored. Expects |m_coord| to point at the // beginning of the next row to be decoded. Tries to process as - // many complete rows as possible. Returns true if the whole image - // was decoded. - bool processNonRLEData(bool inRLE, int numPixels); + // many complete rows as possible. Returns InsufficientData if + // there wasn't enough data to decode the whole image. + // + // This function returns a ProcessingResult instead of a bool so that it + // can avoid calling m_parent->setFailed(), which could lead to memory + // corruption since that will delete |this| but some callers still want + // to access member variables after this returns. + ProcessingResult processNonRLEData(bool inRLE, int numPixels); // Returns true if the current y-coordinate plus |numRows| would be past // the end of the image. Here "plus" means "toward the end of the @@ -261,11 +270,6 @@ namespace WebCore { // depending on the value of |m_isTopDown|. void moveBufferToNextRow(); - // Sets the "decode failure" flag and clears any local storage. For - // caller convenience (since so many callers want to return false after - // calling this), returns false to enable easy tailcalling. - bool setFailed(); - // The decoder that owns us. ImageDecoder* m_parent; diff --git a/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp b/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp index 3cf516e..a590a6c 100644 --- a/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp +++ b/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp @@ -119,6 +119,12 @@ RGBA32Buffer* GIFImageDecoder::frameBufferAtIndex(size_t index) return &frame; } +bool GIFImageDecoder::setFailed() +{ + m_reader.clear(); + return ImageDecoder::setFailed(); +} + void GIFImageDecoder::clearFrameBufferCache(size_t clearBeforeFrame) { // In some cases, like if the decoder was destroyed while animating, we @@ -300,9 +306,6 @@ void GIFImageDecoder::decode(unsigned haltAtFrame, GIFQuery query) // has failed. if (!m_reader->read((const unsigned char*)m_data->data() + m_readOffset, m_data->size() - m_readOffset, query, haltAtFrame) && isAllDataReceived()) setFailed(); - - if (failed()) - m_reader.clear(); } bool GIFImageDecoder::initFrameBuffer(unsigned frameIndex) diff --git a/WebCore/platform/image-decoders/gif/GIFImageDecoder.h b/WebCore/platform/image-decoders/gif/GIFImageDecoder.h index 1c3378c..e0f8173 100644 --- a/WebCore/platform/image-decoders/gif/GIFImageDecoder.h +++ b/WebCore/platform/image-decoders/gif/GIFImageDecoder.h @@ -49,6 +49,10 @@ namespace WebCore { virtual size_t frameCount(); virtual int repetitionCount() const; virtual RGBA32Buffer* frameBufferAtIndex(size_t index); + // CAUTION: setFailed() deletes |m_reader|. Be careful to avoid + // accessing deleted memory, especially when calling this from inside + // GIFImageReader! + virtual bool setFailed(); virtual void clearFrameBufferCache(size_t clearBeforeFrame); // Callbacks from the GIF reader. diff --git a/WebCore/platform/image-decoders/ico/ICOImageDecoder.cpp b/WebCore/platform/image-decoders/ico/ICOImageDecoder.cpp index 325c506..d667795 100644 --- a/WebCore/platform/image-decoders/ico/ICOImageDecoder.cpp +++ b/WebCore/platform/image-decoders/ico/ICOImageDecoder.cpp @@ -116,6 +116,13 @@ RGBA32Buffer* ICOImageDecoder::frameBufferAtIndex(size_t index) return buffer; } +bool ICOImageDecoder::setFailed() +{ + m_bmpReaders.clear(); + m_pngDecoders.clear(); + return ImageDecoder::setFailed(); +} + // static bool ICOImageDecoder::compareEntries(const IconDirectoryEntry& a, const IconDirectoryEntry& b) { @@ -147,6 +154,13 @@ void ICOImageDecoder::decode(size_t index, bool onlySize) // has failed. if ((!decodeDirectory() || (!onlySize && !decodeAtIndex(index))) && isAllDataReceived()) setFailed(); + // If we're done decoding this frame, we don't need the BMPImageReader or + // PNGImageDecoder anymore. (If we failed, these have already been + // cleared.) + else if ((m_frameBufferCache.size() > index) && (m_frameBufferCache[index].status() == RGBA32Buffer::FrameComplete)) { + m_bmpReaders[index].clear(); + m_pngDecoders[index].clear(); + } } bool ICOImageDecoder::decodeDirectory() @@ -241,8 +255,9 @@ bool ICOImageDecoder::processDirectoryEntries() // The image size is the size of the largest entry. const IconDirectoryEntry& dirEntry = m_dirEntries.first(); - setSize(dirEntry.m_size.width(), dirEntry.m_size.height()); - return true; + // Technically, this next call shouldn't be able to fail, since the width + // and height here are each <= 256, and |m_frameSize| is empty. + return setSize(dirEntry.m_size.width(), dirEntry.m_size.height()); } ICOImageDecoder::IconDirectoryEntry ICOImageDecoder::readDirectoryEntry() diff --git a/WebCore/platform/image-decoders/ico/ICOImageDecoder.h b/WebCore/platform/image-decoders/ico/ICOImageDecoder.h index c1f29c9..48024a2 100644 --- a/WebCore/platform/image-decoders/ico/ICOImageDecoder.h +++ b/WebCore/platform/image-decoders/ico/ICOImageDecoder.h @@ -52,6 +52,10 @@ namespace WebCore { virtual bool setSize(unsigned width, unsigned height); virtual size_t frameCount(); virtual RGBA32Buffer* frameBufferAtIndex(size_t); + // CAUTION: setFailed() deletes all readers and decoders. Be careful to + // avoid accessing deleted memory, especially when calling this from + // inside BMPImageReader! + virtual bool setFailed(); private: enum ImageType { diff --git a/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp b/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp index 8375693..cce4f64 100644 --- a/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp +++ b/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp @@ -167,10 +167,8 @@ public: m_bufferLength = data.size(); // We need to do the setjmp here. Otherwise bad things will happen - if (setjmp(m_err.setjmp_buffer)) { - close(); + if (setjmp(m_err.setjmp_buffer)) return m_decoder->setFailed(); - } switch (m_state) { case JPEG_HEADER: @@ -304,7 +302,7 @@ public: case JPEG_DONE: // Finish decompression. return jpeg_finish_decompress(&m_info); - + case JPEG_ERROR: // We can get here if the constructor failed. return m_decoder->setFailed(); @@ -402,6 +400,12 @@ RGBA32Buffer* JPEGImageDecoder::frameBufferAtIndex(size_t index) return &frame; } +bool JPEGImageDecoder::setFailed() +{ + m_reader.clear(); + return ImageDecoder::setFailed(); +} + bool JPEGImageDecoder::outputScanlines() { if (m_frameBufferCache.isEmpty()) @@ -482,8 +486,9 @@ void JPEGImageDecoder::decode(bool onlySize) // has failed. if (!m_reader->decode(m_data->buffer(), onlySize) && isAllDataReceived()) setFailed(); - - if (failed() || (!m_frameBufferCache.isEmpty() && m_frameBufferCache[0].status() == RGBA32Buffer::FrameComplete)) + // If we're done decoding the image, we don't need the JPEGImageReader + // anymore. (If we failed, |m_reader| has already been cleared.) + else if (!m_frameBufferCache.isEmpty() && (m_frameBufferCache[0].status() == RGBA32Buffer::FrameComplete)) m_reader.clear(); } diff --git a/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.h b/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.h index 79bad47..43b35fd 100644 --- a/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.h +++ b/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.h @@ -46,6 +46,10 @@ namespace WebCore { virtual bool setSize(unsigned width, unsigned height); virtual RGBA32Buffer* frameBufferAtIndex(size_t index); virtual bool supportsAlpha() const { return false; } + // CAUTION: setFailed() deletes |m_reader|. Be careful to avoid + // accessing deleted memory, especially when calling this from inside + // JPEGImageReader! + virtual bool setFailed(); bool outputScanlines(); void jpegComplete(); diff --git a/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp b/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp index 1dcf6c7..56cf05f 100644 --- a/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp +++ b/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp @@ -125,10 +125,8 @@ public: PNGImageDecoder* decoder = static_cast<PNGImageDecoder*>(png_get_progressive_ptr(m_png)); // We need to do the setjmp here. Otherwise bad things will happen. - if (setjmp(m_png->jmpbuf)) { - close(); + if (setjmp(m_png->jmpbuf)) return decoder->setFailed(); - } const char* segment; while (unsigned segmentLength = data.getSomeData(segment, m_readOffset)) { @@ -166,6 +164,7 @@ private: }; PNGImageDecoder::PNGImageDecoder() + : m_doNothingOnFailure(false) { } @@ -204,6 +203,14 @@ RGBA32Buffer* PNGImageDecoder::frameBufferAtIndex(size_t index) return &frame; } +bool PNGImageDecoder::setFailed() +{ + if (m_doNothingOnFailure) + return false; + m_reader.clear(); + return ImageDecoder::setFailed(); +} + void PNGImageDecoder::headerAvailable() { png_structp png = m_reader->pngPtr(); @@ -217,8 +224,15 @@ void PNGImageDecoder::headerAvailable() return; } - // We can fill in the size now that the header is available. - if (!setSize(width, height)) { + // We can fill in the size now that the header is available. Avoid memory + // corruption issues by neutering setFailed() during this call; if we don't + // do this, failures will cause |m_reader| to be deleted, and our jmpbuf + // will cease to exist. Note that we'll still properly set the failure flag + // in this case as soon as we longjmp(). + m_doNothingOnFailure = true; + bool result = setSize(width, height); + m_doNothingOnFailure = false; + if (!result) { longjmp(png->jmpbuf, 1); return; } @@ -341,7 +355,9 @@ void PNGImageDecoder::rowAvailable(unsigned char* rowBuffer, unsigned rowIndex, // Copy the data into our buffer. int width = scaledSize().width(); int destY = scaledY(rowIndex); - if (destY < 0) + + // Check that the row is within the image bounds. LibPNG may supply an extra row. + if (destY < 0 || destY >= scaledSize().height()) return; bool sawAlpha = buffer.hasAlpha(); for (int x = 0; x < width; ++x) { @@ -373,8 +389,9 @@ void PNGImageDecoder::decode(bool onlySize) // has failed. if (!m_reader->decode(*m_data, onlySize) && isAllDataReceived()) setFailed(); - - if (failed() || isComplete()) + // If we're done decoding the image, we don't need the PNGImageReader + // anymore. (If we failed, |m_reader| has already been cleared.) + else if (isComplete()) m_reader.clear(); } diff --git a/WebCore/platform/image-decoders/png/PNGImageDecoder.h b/WebCore/platform/image-decoders/png/PNGImageDecoder.h index 287a794..145fc4d 100644 --- a/WebCore/platform/image-decoders/png/PNGImageDecoder.h +++ b/WebCore/platform/image-decoders/png/PNGImageDecoder.h @@ -44,6 +44,10 @@ namespace WebCore { virtual bool isSizeAvailable(); virtual bool setSize(unsigned width, unsigned height); virtual RGBA32Buffer* frameBufferAtIndex(size_t index); + // CAUTION: setFailed() deletes |m_reader|. Be careful to avoid + // accessing deleted memory, especially when calling this from inside + // PNGImageReader! + virtual bool setFailed(); // Callbacks from libpng void headerAvailable(); @@ -62,6 +66,7 @@ namespace WebCore { void decode(bool onlySize); OwnPtr<PNGImageReader> m_reader; + bool m_doNothingOnFailure; }; } // namespace WebCore diff --git a/WebCore/platform/image-decoders/qt/RGBA32BufferQt.cpp b/WebCore/platform/image-decoders/qt/RGBA32BufferQt.cpp index b2e5e17..044515a 100644 --- a/WebCore/platform/image-decoders/qt/RGBA32BufferQt.cpp +++ b/WebCore/platform/image-decoders/qt/RGBA32BufferQt.cpp @@ -57,6 +57,7 @@ RGBA32Buffer& RGBA32Buffer::operator=(const RGBA32Buffer& other) void RGBA32Buffer::clear() { + m_pixmap = QPixmap(); m_image = QImage(); m_status = FrameEmpty; // NOTE: Do not reset other members here; clearFrameBufferCache() @@ -67,7 +68,11 @@ void RGBA32Buffer::clear() void RGBA32Buffer::zeroFill() { - m_image.fill(0); + if (m_pixmap.isNull() && !m_image.isNull()) { + m_pixmap = QPixmap(m_image.width(), m_image.height()); + m_image = QImage(); + } + m_pixmap.fill(QColor(0, 0, 0, 0)); } void RGBA32Buffer::copyBitmapData(const RGBA32Buffer& other) @@ -76,6 +81,7 @@ void RGBA32Buffer::copyBitmapData(const RGBA32Buffer& other) return; m_image = other.m_image; + m_pixmap = other.m_pixmap; m_size = other.m_size; m_hasAlpha = other.m_hasAlpha; } @@ -87,8 +93,9 @@ bool RGBA32Buffer::setSize(int newWidth, int newHeight) ASSERT(width() == 0 && height() == 0); m_size = IntSize(newWidth, newHeight); - m_image = QImage(newWidth, newHeight, QImage::Format_ARGB32_Premultiplied); - if (m_image.isNull()) + m_image = QImage(); + m_pixmap = QPixmap(newWidth, newHeight); + if (m_pixmap.isNull()) return false; // Zero the image. @@ -99,10 +106,11 @@ bool RGBA32Buffer::setSize(int newWidth, int newHeight) QPixmap* RGBA32Buffer::asNewNativeImage() const { - QPixmap pix = QPixmap::fromImage(m_image); - m_image = QImage(); - - return new QPixmap(pix); + if (m_pixmap.isNull() && !m_image.isNull()) { + m_pixmap = QPixmap::fromImage(m_image); + m_image = QImage(); + } + return new QPixmap(m_pixmap); } bool RGBA32Buffer::hasAlpha() const @@ -121,11 +129,12 @@ void RGBA32Buffer::setStatus(FrameStatus status) } // The image must not have format 8888 pre multiplied... -void RGBA32Buffer::setDecodedImage(const QImage& image) +void RGBA32Buffer::setPixmap(const QPixmap& pixmap) { - m_image = image; - m_size = image.size(); - m_hasAlpha = image.hasAlphaChannel(); + m_pixmap = pixmap; + m_image = QImage(); + m_size = pixmap.size(); + m_hasAlpha = pixmap.hasAlphaChannel(); } int RGBA32Buffer::width() const |