Changeset 44545 in webkit


Ignore:
Timestamp:
Jun 9, 2009 3:00:51 PM (15 years ago)
Author:
pkasting@chromium.org
Message:

2009-06-08 Peter Kasting <​pkasting@google.com>

Reviewed by Eric Seidel.

​https://bugs.webkit.org/show_bug.cgi?id=25709 part three
Various minor cleanups to the Skia files. Mostly non-functional, except
for two specific changes:

  • JPEGs and PNGs were always marked as transparent; now they are only marked as transparent when they actually are. I doubt this has much of an effect but in theory it could be used to optimize their display.
  • Instead of arbitrarily disallowing images over 32 * 1024 * 1024 px2, only disallow images which are so large they will cause overflow in other parts of the code. This should fix the testcase on ​http://code.google.com/p/chromium/issues/detail?id=3643.
  • platform/image-decoders/skia/BMPImageReader.h: (WebCore::BMPImageReader::setRGBA): Use simpler non-static setRGBA() form
  • platform/image-decoders/skia/GIFImageDecoder.cpp: (WebCore::GIFImageDecoder::initFrameBuffer): Remove unneeded code, use more readable setRGBA() form (WebCore::GIFImageDecoder::haveDecodedRow): Fix style violation
  • platform/image-decoders/skia/GIFImageDecoder.h: Remove unneeded code
  • platform/image-decoders/skia/ImageDecoder.h: (WebCore::RGBA32Buffer::setSize): setSize() should just setStatus() when it fails since all callers were doing it (WebCore::ImageDecoder::isOverSize): Ease "oversized" image constraints to allow any image that won't overflow
  • platform/image-decoders/skia/JPEGImageDecoder.cpp: (WebCore::JPEGImageDecoder::outputScanlines): Remove unneeded code, mark JPEGs as non-transparent
  • platform/image-decoders/skia/PNGImageDecoder.cpp: (WebCore::PNGImageDecoder::decodingFailed): Fix style violation (WebCore::PNGImageDecoder::rowAvailable): Mark un-decoded PNGs as non-transparent (this will get reset later as needed)
  • platform/image-decoders/skia/XBMImageDecoder.cpp: (WebCore::XBMImageDecoder::frameBufferAtIndex): Remove unneeded code
Location:
trunk/WebCore
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebCore/ChangeLog

    r44544 r44545  
     12009-06-08  Peter Kasting  <pkasting@google.com>
     2
     3        Reviewed by Eric Seidel.
     4
     5        https://bugs.webkit.org/show_bug.cgi?id=25709 part three
     6        Various minor cleanups to the Skia files.  Mostly non-functional, except
     7        for two specific changes:
     8        * JPEGs and PNGs were always marked as transparent; now they are only
     9          marked as transparent when they actually are.  I doubt this has much
     10          of an effect but in theory it could be used to optimize their display.
     11        * Instead of arbitrarily disallowing images over 32 * 1024 * 1024 px^2,
     12          only disallow images which are so large they will cause overflow in
     13          other parts of the code.  This should fix the testcase on
     14          http://code.google.com/p/chromium/issues/detail?id=3643.
     15
     16        * platform/image-decoders/skia/BMPImageReader.h:
     17        (WebCore::BMPImageReader::setRGBA): Use simpler non-static setRGBA() form
     18        * platform/image-decoders/skia/GIFImageDecoder.cpp:
     19        (WebCore::GIFImageDecoder::initFrameBuffer): Remove unneeded code, use more readable setRGBA() form
     20        (WebCore::GIFImageDecoder::haveDecodedRow): Fix style violation
     21        * platform/image-decoders/skia/GIFImageDecoder.h: Remove unneeded code
     22        * platform/image-decoders/skia/ImageDecoder.h:
     23        (WebCore::RGBA32Buffer::setSize): setSize() should just setStatus() when it fails since all callers were doing it
     24        (WebCore::ImageDecoder::isOverSize): Ease "oversized" image constraints to allow any image that won't overflow
     25        * platform/image-decoders/skia/JPEGImageDecoder.cpp:
     26        (WebCore::JPEGImageDecoder::outputScanlines): Remove unneeded code, mark JPEGs as non-transparent
     27        * platform/image-decoders/skia/PNGImageDecoder.cpp:
     28        (WebCore::PNGImageDecoder::decodingFailed): Fix style violation
     29        (WebCore::PNGImageDecoder::rowAvailable): Mark un-decoded PNGs as non-transparent (this will get reset later as needed)
     30        * platform/image-decoders/skia/XBMImageDecoder.cpp:
     31        (WebCore::XBMImageDecoder::frameBufferAtIndex): Remove unneeded code
     32
    1332009-06-09  Darin Fisher  <darin@chromium.org>
    234

  • trunk/WebCore/platform/image-decoders/skia/BMPImageReader.h

    r40173 r44545  
    252252        inline void setRGBA(unsigned red, unsigned green, unsigned blue, unsigned alpha)
    253253        {
    254             RGBA32Buffer::setRGBA(
    255                 m_frameBufferCache.first().bitmap().getAddr32(m_coord.x(), m_coord.y()),
    256                 red, green, blue, alpha);
     254            m_frameBufferCache.first().setRGBA(m_coord.x(), m_coord.y(), red, green, blue, alpha);
    257255            m_coord.move(1, 0);
    258256        }

  • trunk/WebCore/platform/image-decoders/skia/GIFImageDecoder.cpp

    r44167 r44545  
    280280    if (frameIndex == 0) {
    281281        // This is the first frame, so we're not relying on any previous data.
    282         if (!prepEmptyFrameBuffer(buffer)) {
    283             buffer->setStatus(RGBA32Buffer::FrameComplete);
     282        if (!buffer->setSize(size().width(), size().height())) {
    284283            m_failed = true;
    285284            return false;
    … …  
    308307            // Preserve the last frame as the starting state for this frame.
    309308            buffer->copyBitmapData(*prevBuffer);
    310             // This next line isn't currently necessary since the alpha state is
    311             // currently carried along in the Skia bitmap data, but it's safe,
    312             // future-proof, and parallel to the Cairo code.
    313             buffer->setHasAlpha(prevBuffer->hasAlpha());
    314309        } else {
    315310            // We want to clear the previous frame to transparent, without
    … …  
    320315                // Clearing the first frame, or a frame the size of the whole
    321316                // image, results in a completely empty image.
    322                 prepEmptyFrameBuffer(buffer);
     317                if (!buffer->setSize(size().width(), size().height())) {
     318                    m_failed = true;
     319                    return false;
     320                }
    323321            } else {
    324322              // Copy the whole previous buffer, then clear just its frame.
    325323              buffer->copyBitmapData(*prevBuffer);
    326               // Unnecessary (but safe); see comments on the similar call above.
    327               buffer->setHasAlpha(prevBuffer->hasAlpha());
    328               SkBitmap& bitmap = buffer->bitmap();
    329324              for (int y = prevRect.y(); y < prevRect.bottom(); ++y) {
    330325                  for (int x = prevRect.x(); x < prevRect.right(); ++x)
    331                       buffer->setRGBA(bitmap.getAddr32(x, y), 0, 0, 0, 0);
     326                      buffer->setRGBA(x, y, 0, 0, 0, 0);
    332327              }
    333328              if ((prevRect.width() > 0) && (prevRect.height() > 0))
    … …  
    342337    // Reset the alpha pixel tracker for this frame.
    343338    m_currentBufferSawAlpha = false;
    344     return true;
    345 }
    346 
    347 bool GIFImageDecoder::prepEmptyFrameBuffer(RGBA32Buffer* buffer) const
    348 {
    349     if (!buffer->setSize(size().width(), size().height()))
    350         return false;
    351     // This next line isn't currently necessary since Skia's eraseARGB() sets
    352     // this for us, but we do it for similar reasons to the setHasAlpha() calls
    353     // in initFrameBuffer() above.
    354     buffer->setHasAlpha(true);
    355339    return true;
    356340}
    … …  
    417401        // Copy the row |repeatCount|-1 times.
    418402        unsigned num = currDst - dst;
    419         unsigned data_size = num * sizeof(unsigned);
     403        unsigned dataSize = num * sizeof(unsigned);
    420404        unsigned width = size().width();
    421405        unsigned* end = buffer.bitmap().getAddr32(0, 0) + width * size().height();
    … …  
    424408            if (currDst + num > end) // Protect against a buffer overrun from a bogus repeatCount.
    425409                break;
    426             memcpy(currDst, dst, data_size);
     410            memcpy(currDst, dst, dataSize);
    427411            currDst += width;
    428412        }

  • trunk/WebCore/platform/image-decoders/skia/GIFImageDecoder.h

    r40173 r44545  
    7979        bool initFrameBuffer(unsigned frameIndex);
    8080
    81         // A helper for initFrameBuffer(), this sets the size of the buffer, and
    82         // fills it with transparent pixels.
    83         bool prepEmptyFrameBuffer(RGBA32Buffer*) const;
    84 
    8581        bool m_frameCountValid;
    8682        bool m_currentBufferSawAlpha;

  • trunk/WebCore/platform/image-decoders/skia/ImageDecoder.h

    r44167 r44545  
    129129            ASSERT(m_bitmap.width() == 0 && m_bitmap.height() == 0);
    130130            m_bitmap.setConfig(SkBitmap::kARGB_8888_Config, width, height);
    131             if (!m_bitmap.allocPixels())
    132                 return false;  // Allocation failure, maybe the bitmap was too big.
     131            if (!m_bitmap.allocPixels()) {
     132                // Allocation failure, maybe the bitmap was too big.
     133                setStatus(FrameComplete);
     134                return false;
     135            }
    133136
    134137            // Clear the image.
    … …  
    273276
    274277    private:
    275         // This function allows us to make sure the image is not too large. Very
    276         // large images, even if the allocation succeeds, can take a very long time
    277         // to process, giving the appearance of a DoS.
    278         //
    279         // WebKit also seems to like to ask for the size before data is available
    280         // and in some cases when the failed flag is set. Some of these code paths
    281         // such as BitmapImage::resetAnimation then compute the size of the image
    282         // based on the width and height. Because of this, our total computed image
    283         // byte size must never overflow an int.
     278        // Some code paths compute the size of the image as "width * height * 4"
     279        // and return it as a (signed) int.  Avoid overflow.
    284280        static bool isOverSize(unsigned width, unsigned height)
    285281        {
     282            // width * height must not exceed (2 ^ 29) - 1, so that we don't
     283            // overflow when we multiply by 4.
    286284            unsigned long long total_size = static_cast<unsigned long long>(width)
    287285                                          * static_cast<unsigned long long>(height);
    288             if (total_size > 32 * 1024 * 1024)  // 32M = 128MB memory total (32 bpp).
    289                 return true;
    290             return false;
     286            return total_size > ((1 << 29) - 1);
    291287        }
    292288

  • trunk/WebCore/platform/image-decoders/skia/JPEGImageDecoder.cpp

    r44167 r44545  
    466466        if (!buffer.setSize(size().width(), size().height())) {
    467467            m_failed = true;
    468             buffer.setStatus(RGBA32Buffer::FrameComplete);
    469468            return false;
    470469        }
    … …  
    472471        // Update our status to be partially complete.
    473472        buffer.setStatus(RGBA32Buffer::FramePartial);
     473        buffer.setHasAlpha(false);
    474474
    475475        // For JPEGs, the frame always fills the entire image.
    476476        buffer.setRect(IntRect(0, 0, size().width(), size().height()));
    477 
    478         // We don't have alpha (this is the default when the buffer is constructed).
    479477    }
    480478

  • trunk/WebCore/platform/image-decoders/skia/PNGImageDecoder.cpp

    r44167 r44545  
    225225}
    226226
    227 void PNGImageDecoder::decodingFailed() {
     227void PNGImageDecoder::decodingFailed()
     228{
    228229    m_failed = true;
    229230}
    … …  
    331332        // Update our status to be partially complete.
    332333        buffer.setStatus(RGBA32Buffer::FramePartial);
     334        buffer.setHasAlpha(false);
    333335
    334336        // For PNGs, the frame always fills the entire image.

  • trunk/WebCore/platform/image-decoders/skia/XBMImageDecoder.cpp

    r40173 r44545  
    8787        if (!frame.setSize(size().width(), size().height())) {
    8888            m_failed = true;
    89             frame.setStatus(RGBA32Buffer::FrameComplete);
    9089            return 0;
    9190        }
Note: See TracChangeset for help on using the changeset viewer.