Changeset 58589 in webkit


Ignore:
Timestamp:
Apr 30, 2010 12:00:07 PM (14 years ago)
Author:
pkasting@chromium.org
Message:

Make all image decoders set the "failed" bit if an image could not be
completely decoded, but no more data is coming. The ICO and BMP
decoders already did this.
​https://bugs.webkit.org/show_bug.cgi?id=35411

Reviewed by David Levin.

"Failed" does not cause the image to not be displayed, it simply causes
us to not bother to try to decode again if future requests are made, and
for some decoders, lets the decoder clean up some of its temporary
objects.

No layout tests because this does not change the visible output of decoding in any way.

  • platform/image-decoders/gif/GIFImageDecoder.cpp:

(WebCore::GIFImageDecoder::frameComplete): Return whether the frame could be marked as complete.
(WebCore::GIFImageDecoder::decode): Fail if read() needs more data (and thus returns false) and no more is coming.

  • platform/image-decoders/gif/GIFImageDecoder.h:
  • platform/image-decoders/gif/GIFImageReader.cpp:

(GIFImageReader::do_lzw): Instead of returning true for buffer underrun and false for failure, return false for both and set the failure flag on failure.
(GIFImageReader::read): Ditto.

  • platform/image-decoders/gif/GIFImageReader.h:
  • platform/image-decoders/jpeg/JPEGImageDecoder.cpp:

(WebCore::):
(WebCore::JPEGImageReader::decode): See do_lzw() comment above.
(WebCore::JPEGImageDecoder::decode): Fail if decode() needs more data (and thus returns false) and no more is coming.

  • platform/image-decoders/jpeg/JPEGImageDecoder.h:
  • platform/image-decoders/png/PNGImageDecoder.cpp:

(WebCore::PNGImageReader::decode): Return true for decode success, false for buffer underrun or decode failure, and set the failure flag on decode failure.
(WebCore::PNGImageDecoder::decode): See JPEGImageDecoder::decode() comment above.

  • platform/image-decoders/png/PNGImageDecoder.h:
Location:
trunk/WebCore
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebCore/ChangeLog

    r58586 r58589  
     12010-04-30  Peter Kasting  <pkasting@google.com>
     2
     3        Reviewed by David Levin.
     4
     5        Make all image decoders set the "failed" bit if an image could not be
     6        completely decoded, but no more data is coming.  The ICO and BMP
     7        decoders already did this.
     8        https://bugs.webkit.org/show_bug.cgi?id=35411
     9
     10        "Failed" does not cause the image to not be displayed, it simply causes
     11        us to not bother to try to decode again if future requests are made, and
     12        for some decoders, lets the decoder clean up some of its temporary
     13        objects.
     14
     15        No layout tests because this does not change the visible output of decoding in any way.
     16
     17        * platform/image-decoders/gif/GIFImageDecoder.cpp:
     18        (WebCore::GIFImageDecoder::frameComplete): Return whether the frame could be marked as complete.
     19        (WebCore::GIFImageDecoder::decode): Fail if read() needs more data (and thus returns false) and no more is coming.
     20        * platform/image-decoders/gif/GIFImageDecoder.h:
     21        * platform/image-decoders/gif/GIFImageReader.cpp:
     22        (GIFImageReader::do_lzw): Instead of returning true for buffer underrun and false for failure, return false for both and set the failure flag on failure.
     23        (GIFImageReader::read): Ditto.
     24        * platform/image-decoders/gif/GIFImageReader.h:
     25        * platform/image-decoders/jpeg/JPEGImageDecoder.cpp:
     26        (WebCore::):
     27        (WebCore::JPEGImageReader::decode): See do_lzw() comment above.
     28        (WebCore::JPEGImageDecoder::decode): Fail if decode() needs more data (and thus returns false) and no more is coming.
     29        * platform/image-decoders/jpeg/JPEGImageDecoder.h:
     30        * platform/image-decoders/png/PNGImageDecoder.cpp:
     31        (WebCore::PNGImageReader::decode): Return true for decode success, false for buffer underrun or decode failure, and set the failure flag on decode failure.
     32        (WebCore::PNGImageDecoder::decode): See JPEGImageDecoder::decode() comment above.
     33        * platform/image-decoders/png/PNGImageDecoder.h:
     34
    1352010-04-30  Brady Eidson  <beidson@apple.com>
    236

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

    r56007 r58589  
    236236}
    237237
    238 void GIFImageDecoder::frameComplete(unsigned frameIndex, unsigned frameDuration, RGBA32Buffer::FrameDisposalMethod disposalMethod)
     238bool GIFImageDecoder::frameComplete(unsigned frameIndex, unsigned frameDuration, RGBA32Buffer::FrameDisposalMethod disposalMethod)
    239239{
    240240    // Initialize the frame if necessary.  Some GIFs insert do-nothing frames,
    … …  
    242242    RGBA32Buffer& buffer = m_frameBufferCache[frameIndex];
    243243    if ((buffer.status() == RGBA32Buffer::FrameEmpty) && !initFrameBuffer(frameIndex))
    244         return;
     244        return false; // initFrameBuffer() has already called setFailed().
    245245
    246246    buffer.setStatus(RGBA32Buffer::FrameComplete);
    … …  
    278278        }
    279279    }
     280
     281    return true;
    280282}
    281283
    … …  
    295297        m_reader.set(new GIFImageReader(this));
    296298
    297     if (!m_reader->read((const unsigned char*)m_data->data() + m_readOffset, m_data->size() - m_readOffset, query, haltAtFrame))
     299    // If we couldn't decode the image but we've received all the data, decoding
     300    // has failed.
     301    if (!m_reader->read((const unsigned char*)m_data->data() + m_readOffset, m_data->size() - m_readOffset, query, haltAtFrame) && isAllDataReceived())
    298302        setFailed();
    299303

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

    r56007 r58589  
    5555        void decodingHalted(unsigned bytesLeft);
    5656        bool haveDecodedRow(unsigned frameIndex, unsigned char* rowBuffer, unsigned char* rowEnd, unsigned rowNumber, unsigned repeatCount, bool writeTransparentPixels);
    57         void frameComplete(unsigned frameIndex, unsigned frameDuration, RGBA32Buffer::FrameDisposalMethod disposalMethod);
     57        bool frameComplete(unsigned frameIndex, unsigned frameDuration, RGBA32Buffer::FrameDisposalMethod disposalMethod);
    5858        void gifComplete();
    5959
    … …  
    6161        // If the query is GIFFullQuery, decodes the image up to (but not
    6262        // including) |haltAtFrame|.  Otherwise, decodes as much as is needed to
    63         // answer the query, ignoring bitmap data.
     63        // answer the query, ignoring bitmap data.  If decoding fails but there
     64        // is no more data coming, sets the "decode failure" flag.
    6465        void decode(unsigned haltAtFrame, GIFQuery);
    6566

  • trunk/WebCore/platform/image-decoders/gif/GIFImageReader.cpp

    r56007 r58589  
    291291      if (code == (clear_code + 1)) {
    292292        /* end-of-stream should only appear after all image data */
    293         return !rows_remaining;
     293        if (!rows_remaining)
     294          return true;
     295        return clientptr ? clientptr->setFailed() : false;
    294296      }
    295297
    … …  
    309311
    310312        if (stackp == stack + MAX_BITS)
    311           return false;
     313          return clientptr ? clientptr->setFailed() : false;
    312314      }
    313315
    … …  
    315317      {
    316318        if (code >= MAX_BITS || code == prefix[code])
    317           return false;
     319          return clientptr ? clientptr->setFailed() : false;
    318320
    319321        // Even though suffix[] only holds characters through suffix[avail - 1],
    … …  
    325327
    326328        if (stackp == stack + MAX_BITS)
    327           return false;
     329          return clientptr ? clientptr->setFailed() : false;
    328330      }
    329331
    … …  
    416418      if (clientptr)
    417419        clientptr->decodingHalted(0);
    418       return true;
     420      return false;
    419421    }
    420422    // Reset hold buffer count
    … …  
    441443    {
    442444    case gif_lzw:
    443       if (!do_lzw(q)) {
    444         state = gif_error;
    445         break;
    446       }
     445      if (!do_lzw(q))
     446        return false; // If do_lzw() encountered an error, it has already called
     447                      // clientptr->setFailed().
    447448      GETN(1, gif_sub_block);
    448449      break;
    … …  
    455456      // that our datasize is strictly less than the MAX_LZW_BITS value (12).
    456457      // This sets the largest possible codemask correctly at 4095.
    457       if (datasize >= MAX_LZW_BITS) {
    458         state = gif_error;
    459         break;
    460       }
     458      if (datasize >= MAX_LZW_BITS)
     459        return clientptr ? clientptr->setFailed() : false;
    461460      int clear_code = 1 << datasize;
    462       if (clear_code >= MAX_BITS) {
    463         state = gif_error;
    464         break;
    465       }
     461      if (clear_code >= MAX_BITS)
     462        return clientptr ? clientptr->setFailed() : false;
    466463
    467464      if (frame_reader) {
    … …  
    499496      else if (!strncmp((char*)q, "GIF87a", 6))
    500497        version = 87;
    501       else {
    502         state = gif_error;
    503         break;
    504       }
     498      else
     499        return clientptr ? clientptr->setFailed() : false;
    505500      GETN(7, gif_global_header);
    506501    }
    … …  
    587582        } else {
    588583          /* No images decoded, there is nothing to display. */
    589           state = gif_error;
     584          return clientptr ? clientptr->setFailed() : false;
    590585        }
    591586        break;
    … …  
    715710      } else {
    716711        // 0,3-7 are yet to be defined netscape extension codes
    717         state = gif_error;
     712        return clientptr ? clientptr->setFailed() : false;
    718713      }
    719714
    … …  
    756751        height = screen_height;
    757752        width = screen_width;
    758         if (!height || !width) {
    759           state = gif_error;
    760           break;
    761         }
     753        if (!height || !width)
     754          return clientptr ? clientptr->setFailed() : false;
    762755      }
    763756
    … …  
    795788        }
    796789
    797         if (!frame_reader->rowbuf) {
    798           state = gif_oom;
    799           break;
    800         }
     790        if (!frame_reader->rowbuf)
     791          return clientptr ? clientptr->setFailed() : false;
    801792        if (screen_height < height)
    802793          screen_height = height;
    … …  
    839830          delete []map;
    840831          map = new unsigned char[size];
    841           if (!map) {
    842             state = gif_oom;
    843             break;
    844           }
     832          if (!map)
     833            return clientptr ? clientptr->setFailed() : false;
    845834        }
    846835
    … …  
    894883
    895884        // CALLBACK: The frame is now complete.
    896         if (clientptr && frame_reader)
    897           clientptr->frameComplete(images_decoded - 1, frame_reader->delay_time, frame_reader->disposal_method);
     885        if (clientptr && frame_reader && !clientptr->frameComplete(images_decoded - 1, frame_reader->delay_time, frame_reader->disposal_method))
     886          return false; // frameComplete() has already called
     887                        // clientptr->setFailed().
    898888
    899889        /* Clear state from this image */
    … …  
    913903        clientptr->gifComplete();
    914904      return true;
    915 
    916     // Handle out of memory errors
    917     case gif_oom:
    918       return false;
    919 
    920     // Handle general errors
    921     case gif_error:
    922       return false;
    923905
    924906    // We shouldn't ever get here.
    … …  
    946928  if (clientptr)
    947929    clientptr->decodingHalted(0);
    948   return true;
     930  return false;
    949931}

  • trunk/WebCore/platform/image-decoders/gif/GIFImageReader.h

    r54823 r58589  
    7070    gif_skip_block,
    7171    gif_done,
    72     gif_oom,
    73     gif_error,
    7472    gif_comment_extension,
    7573    gif_application_extension,

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

    r56007 r58589  
    7070    JPEG_DECOMPRESS_SEQUENTIAL,  // Output sequential pixels
    7171    JPEG_DONE,
    72     JPEG_SINK_NON_JPEG_TRAILER,  // Some image files have a non-JPEG trailer
    7372    JPEG_ERROR   
    7473};
    … …  
    170169        // We need to do the setjmp here. Otherwise bad things will happen
    171170        if (setjmp(m_err.setjmp_buffer)) {
    172             m_state = JPEG_SINK_NON_JPEG_TRAILER;
    173171            close();
    174             return false;
     172            return m_decoder->setFailed();
    175173        }
    176174
    … …  
    179177            // Read file parameters with jpeg_read_header().
    180178            if (jpeg_read_header(&m_info, true) == JPEG_SUSPENDED)
    181                 return true; // I/O suspension.
     179                return false; // I/O suspension.
    182180
    183181            // Let libjpeg take care of gray->RGB and YCbCr->RGB conversions.
    … …  
    195193                break;
    196194            default:
    197                 return false;
     195                return m_decoder->setFailed();
    198196            }
    199197
    … …  
    215213            // We can fill in the size now that the header is available.
    216214            if (!m_decoder->setSize(m_info.image_width, m_info.image_height))
    217                 return false;
     215                return m_decoder->setFailed();
    218216
    219217            if (m_decodingSizeOnly) {
    … …  
    238236            // Start decompressor.
    239237            if (!jpeg_start_decompress(&m_info))
    240                 return true; // I/O suspension.
     238                return false; // I/O suspension.
    241239
    242240            // If this is a progressive JPEG ...
    … …  
    248246 
    249247                if (!m_decoder->outputScanlines())
    250                     return true; // I/O suspension.
     248                    return false; // I/O suspension.
    251249 
    252250                // If we've completed image output...
    … …  
    274272
    275273                        if (!jpeg_start_output(&m_info, scan))
    276                             return true; // I/O suspension.
     274                            return false; // I/O suspension.
    277275                    }
    278276
    … …  
    286284                            // the same scan.
    287285                            m_info.output_scanline = 0xffffff;
    288                         return true; // I/O suspension.
     286                        return false; // I/O suspension.
    289287                    }
    290288
    291289                    if (m_info.output_scanline == m_info.output_height) {
    292290                        if (!jpeg_finish_output(&m_info))
    293                             return true; // I/O suspension.
     291                            return false; // I/O suspension.
    294292
    295293                        if (jpeg_input_complete(&m_info) && (m_info.input_scan_number == m_info.output_scan_number))
    … …  
    306304        case JPEG_DONE:
    307305            // Finish decompression.
    308             if (!jpeg_finish_decompress(&m_info))
    309                 return true; // I/O suspension.
    310 
    311             m_state = JPEG_SINK_NON_JPEG_TRAILER;
    312             break;
     306            return jpeg_finish_decompress(&m_info);
    313307       
    314         case JPEG_SINK_NON_JPEG_TRAILER:
    315             break;
    316 
    317308        case JPEG_ERROR:
    318309            // We can get here if the constructor failed.
    … …  
    488479        m_reader.set(new JPEGImageReader(this));
    489480
    490     if (!m_reader->decode(m_data->buffer(), onlySize))
     481    // If we couldn't decode the image but we've received all the data, decoding
     482    // has failed.
     483    if (!m_reader->decode(m_data->buffer(), onlySize) && isAllDataReceived())
    491484        setFailed();
    492485

  • trunk/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.h

    r56007 r58589  
    5353    private:
    5454        // Decodes the image.  If |onlySize| is true, stops decoding after
    55         // calculating the image size.
     55        // calculating the image size.  If decoding fails but there is no more
     56        // data coming, sets the "decode failure" flag.
    5657        void decode(bool onlySize);
    5758

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

    r56007 r58589  
    120120    unsigned currentBufferSize() const { return m_currentBufferSize; }
    121121
    122     void decode(const SharedBuffer& data, bool sizeOnly)
     122    bool decode(const SharedBuffer& data, bool sizeOnly)
    123123    {
    124124        m_decodingSizeOnly = sizeOnly;
    … …  
    128128        if (setjmp(m_png->jmpbuf)) {
    129129            close();
    130             decoder->setFailed();
    131             return;
     130            return decoder->setFailed();
    132131        }
    133132
    … …  
    141140            // (recursively) trigger additional decoding if we haven't.
    142141            if (sizeOnly ? decoder->ImageDecoder::isSizeAvailable() : decoder->isComplete())
    143                 return;
    144         }
    145         if (!decoder->isComplete() && decoder->isAllDataReceived())
    146             decoder->pngComplete();
     142                return true;
     143        }
     144        return false;
    147145    }
    148146
    … …  
    372370        m_reader.set(new PNGImageReader(this));
    373371
    374     m_reader->decode(*m_data, onlySize);
     372    // If we couldn't decode the image but we've received all the data, decoding
     373    // has failed.
     374    if (!m_reader->decode(*m_data, onlySize) && isAllDataReceived())
     375        setFailed();
    375376   
    376377    if (failed() || isComplete())

  • trunk/WebCore/platform/image-decoders/png/PNGImageDecoder.h

    r56007 r58589  
    5858    private:
    5959        // Decodes the image.  If |onlySize| is true, stops decoding after
    60         // calculating the image size.
     60        // calculating the image size.  If decoding fails but there is no more
     61        // data coming, sets the "decode failure" flag.
    6162        void decode(bool onlySize);
    6263
Note: See TracChangeset for help on using the changeset viewer.