Unreviewed, rolling out r151957 and r152531.
authorossy@webkit.org <ossy@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 30 Jul 2013 16:32:13 +0000 (16:32 +0000)
committerossy@webkit.org <ossy@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 30 Jul 2013 16:32:13 +0000 (16:32 +0000)
http://trac.webkit.org/changeset/151957
http://trac.webkit.org/changeset/152531
https://bugs.webkit.org/show_bug.cgi?id=119267

They revealed a bug on Mac I can't fix (Requested by Ossy on
#webkit).

Patch by Commit Queue <commit-queue@webkit.org> on 2013-07-30

* platform/graphics/BitmapImage.cpp:
(WebCore::BitmapImage::frameIsCompleteAtIndex):
(WebCore::BitmapImage::frameDurationAtIndex):
* platform/graphics/ImageSource.cpp:
(WebCore::ImageSource::frameDurationAtIndex):
(WebCore::ImageSource::frameHasAlphaAtIndex):
(WebCore::ImageSource::frameIsCompleteAtIndex):
* platform/graphics/ImageSource.h:
* platform/graphics/cairo/GraphicsContext3DCairo.cpp:
(WebCore::GraphicsContext3D::ImageExtractor::extractImage):
* platform/graphics/cg/ImageSourceCG.cpp:
(WebCore::ImageSource::frameIsCompleteAtIndex):
(WebCore::ImageSource::frameDurationAtIndex):
(WebCore::ImageSource::frameHasAlphaAtIndex):
* platform/graphics/efl/GraphicsContext3DEfl.cpp:
(WebCore::GraphicsContext3D::ImageExtractor::extractImage):
* platform/image-decoders/ImageDecoder.cpp:
(WebCore::ImageDecoder::frameHasAlphaAtIndex):
* platform/image-decoders/ImageDecoder.h:
* platform/image-decoders/gif/GIFImageDecoder.cpp:
(WebCore::GIFImageDecoder::haveDecodedRow):
(WebCore::GIFImageDecoder::gifComplete):
(WebCore::GIFImageDecoder::decode):
(WebCore::GIFImageDecoder::initFrameBuffer):
* platform/image-decoders/gif/GIFImageDecoder.h:
* platform/image-decoders/gif/GIFImageReader.h:
(GIFImageReader::frameContext):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@153475 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/BitmapImage.cpp
Source/WebCore/platform/graphics/ImageSource.cpp
Source/WebCore/platform/graphics/ImageSource.h
Source/WebCore/platform/graphics/cairo/GraphicsContext3DCairo.cpp
Source/WebCore/platform/graphics/cg/ImageSourceCG.cpp
Source/WebCore/platform/graphics/efl/GraphicsContext3DEfl.cpp
Source/WebCore/platform/image-decoders/ImageDecoder.cpp
Source/WebCore/platform/image-decoders/ImageDecoder.h
Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp
Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.h
Source/WebCore/platform/image-decoders/gif/GIFImageReader.h

index febdac7..18e1ceb 100644 (file)
@@ -1,3 +1,41 @@
+2013-07-30  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r151957 and r152531.
+        http://trac.webkit.org/changeset/151957
+        http://trac.webkit.org/changeset/152531
+        https://bugs.webkit.org/show_bug.cgi?id=119267
+
+        They revealed a bug on Mac I can't fix (Requested by Ossy on
+        #webkit).
+
+        * platform/graphics/BitmapImage.cpp:
+        (WebCore::BitmapImage::frameIsCompleteAtIndex):
+        (WebCore::BitmapImage::frameDurationAtIndex):
+        * platform/graphics/ImageSource.cpp:
+        (WebCore::ImageSource::frameDurationAtIndex):
+        (WebCore::ImageSource::frameHasAlphaAtIndex):
+        (WebCore::ImageSource::frameIsCompleteAtIndex):
+        * platform/graphics/ImageSource.h:
+        * platform/graphics/cairo/GraphicsContext3DCairo.cpp:
+        (WebCore::GraphicsContext3D::ImageExtractor::extractImage):
+        * platform/graphics/cg/ImageSourceCG.cpp:
+        (WebCore::ImageSource::frameIsCompleteAtIndex):
+        (WebCore::ImageSource::frameDurationAtIndex):
+        (WebCore::ImageSource::frameHasAlphaAtIndex):
+        * platform/graphics/efl/GraphicsContext3DEfl.cpp:
+        (WebCore::GraphicsContext3D::ImageExtractor::extractImage):
+        * platform/image-decoders/ImageDecoder.cpp:
+        (WebCore::ImageDecoder::frameHasAlphaAtIndex):
+        * platform/image-decoders/ImageDecoder.h:
+        * platform/image-decoders/gif/GIFImageDecoder.cpp:
+        (WebCore::GIFImageDecoder::haveDecodedRow):
+        (WebCore::GIFImageDecoder::gifComplete):
+        (WebCore::GIFImageDecoder::decode):
+        (WebCore::GIFImageDecoder::initFrameBuffer):
+        * platform/image-decoders/gif/GIFImageDecoder.h:
+        * platform/image-decoders/gif/GIFImageReader.h:
+        (GIFImageReader::frameContext):
+
 2013-07-30  Kwang Yul Seo  <skyul@company100.net>
 
         Remove using namespace std from SpatialNavigation
index 07610d2..4a13d47 100644 (file)
@@ -305,16 +305,16 @@ PassNativeImagePtr BitmapImage::frameAtIndex(size_t index)
 
 bool BitmapImage::frameIsCompleteAtIndex(size_t index)
 {
-    if (index < m_frames.size() && m_frames[index].m_haveMetadata && m_frames[index].m_isComplete)
-        return true;
-    return m_source.frameIsCompleteAtIndex(index);
+    if (!ensureFrameIsCached(index))
+        return false;
+    return m_frames[index].m_isComplete;
 }
 
 float BitmapImage::frameDurationAtIndex(size_t index)
 {
-    if (index < m_frames.size() && m_frames[index].m_haveMetadata)
-        return m_frames[index].m_duration;
-    return m_source.frameDurationAtIndex(index);
+    if (!ensureFrameIsCached(index))
+        return 0;
+    return m_frames[index].m_duration;
 }
 
 PassNativeImagePtr BitmapImage::nativeImageForCurrentFrame()
index 25c095b..f7b4909 100644 (file)
@@ -155,16 +155,20 @@ PassNativeImagePtr ImageSource::createFrameAtIndex(size_t index)
     return buffer->asNewNativeImage();
 }
 
-float ImageSource::frameDurationAtIndex(size_t index) const
+float ImageSource::frameDurationAtIndex(size_t index)
 {
     if (!m_decoder)
         return 0;
 
+    ImageFrame* buffer = m_decoder->frameBufferAtIndex(index);
+    if (!buffer || buffer->status() == ImageFrame::FrameEmpty)
+        return 0;
+
     // Many annoying ads specify a 0 duration to make an image flash as quickly as possible.
     // We follow Firefox's behavior and use a duration of 100 ms for any frames that specify
     // a duration of <= 10 ms. See <rdar://problem/7689300> and <http://webkit.org/b/36082>
     // for more information.
-    const float duration = m_decoder->frameDurationAtIndex(index) / 1000.0f;
+    const float duration = buffer->duration() / 1000.0f;
     if (duration < 0.011f)
         return 0.100f;
     return duration;
@@ -175,14 +179,20 @@ ImageOrientation ImageSource::orientationAtIndex(size_t) const
     return m_decoder ? m_decoder->orientation() : DefaultImageOrientation;
 }
 
-bool ImageSource::frameHasAlphaAtIndex(size_t index) const
+bool ImageSource::frameHasAlphaAtIndex(size_t index)
 {
-    return !m_decoder || m_decoder->frameHasAlphaAtIndex(index);
+    if (!m_decoder)
+        return true;
+    return m_decoder->frameHasAlphaAtIndex(index);
 }
 
-bool ImageSource::frameIsCompleteAtIndex(size_t index) const
+bool ImageSource::frameIsCompleteAtIndex(size_t index)
 {
-    return m_decoder && m_decoder->frameIsCompleteAtIndex(index);
+    if (!m_decoder)
+        return false;
+
+    ImageFrame* buffer = m_decoder->frameBufferAtIndex(index);
+    return buffer && buffer->status() == ImageFrame::FrameComplete;
 }
 
 unsigned ImageSource::frameBytesAtIndex(size_t index) const
index cd0d2b7..31a2e19 100644 (file)
@@ -147,9 +147,9 @@ public:
     // see comments on clear() above.
     PassNativeImagePtr createFrameAtIndex(size_t);
 
-    float frameDurationAtIndex(size_t) const;
-    bool frameHasAlphaAtIndex(size_t) const; // Whether or not the frame actually used any alpha.
-    bool frameIsCompleteAtIndex(size_t) const; // Whether or not the frame is completely decoded.
+    float frameDurationAtIndex(size_t);
+    bool frameHasAlphaAtIndex(size_t); // Whether or not the frame actually used any alpha.
+    bool frameIsCompleteAtIndex(size_t); // Whether or not the frame is completely decoded.
     ImageOrientation orientationAtIndex(size_t) const; // EXIF image orientation
 
     // Return the number of bytes in the decoded frame. If the frame is not yet
index d31adf3..85618cd 100644 (file)
@@ -190,12 +190,9 @@ bool GraphicsContext3D::ImageExtractor::extractImage(bool premultiplyAlpha, bool
     m_alphaOp = AlphaDoNothing;
     if (m_image->data()) {
         decoder.setData(m_image->data(), true);
-        if (!decoder.frameCount())
+        if (!decoder.frameCount() || !decoder.frameIsCompleteAtIndex(0))
             return false;
-
         m_imageSurface = decoder.createFrameAtIndex(0);
-        if (!m_imageSurface || !decoder.frameIsCompleteAtIndex(0))
-            return false;
     } else {
         m_imageSurface = m_image->nativeImageForCurrentFrame();
         // 1. For texImage2D with HTMLVideoElment input, assume no PremultiplyAlpha had been applied and the alpha value is 0xFF for each pixel,
index 96e21c9..aaa2fdf 100644 (file)
@@ -312,7 +312,7 @@ CGImageRef ImageSource::createFrameAtIndex(size_t index)
     return maskedImage.leakRef();
 }
 
-bool ImageSource::frameIsCompleteAtIndex(size_t index) const
+bool ImageSource::frameIsCompleteAtIndex(size_t index)
 {
     ASSERT(frameCount());
 
@@ -332,7 +332,7 @@ bool ImageSource::frameIsCompleteAtIndex(size_t index) const
     return frameStatus == kCGImageStatusComplete;
 }
 
-float ImageSource::frameDurationAtIndex(size_t index) const
+float ImageSource::frameDurationAtIndex(size_t index)
 {
     if (!initialized())
         return 0;
@@ -361,7 +361,7 @@ float ImageSource::frameDurationAtIndex(size_t index) const
     return duration;
 }
 
-bool ImageSource::frameHasAlphaAtIndex(size_t index) const
+bool ImageSource::frameHasAlphaAtIndex(size_t index)
 {
     if (!m_decoder)
         return false; // FIXME: why doesn't this return true?
index ea25e43..12ca726 100644 (file)
@@ -265,12 +265,10 @@ bool GraphicsContext3D::ImageExtractor::extractImage(bool premultiplyAlpha, bool
     if (m_image->data()) {
         decoder.setData(m_image->data(), true);
 
-        if (!decoder.frameCount())
+        if (!decoder.frameCount() || !decoder.frameIsCompleteAtIndex(0))
             return false;
 
         m_imageSurface = decoder.createFrameAtIndex(0);
-        if (!m_imageSurface || !decoder.frameIsCompleteAtIndex(0))
-            return false;
     } else {
         m_imageSurface = m_image->nativeImageForCurrentFrame();
         // 1. For texImage2D with HTMLVideoElment input, assume no PremultiplyAlpha had been applied and the alpha value is 0xFF for each pixel,
index 2d4dff4..396c1ea 100644 (file)
@@ -284,12 +284,11 @@ template <MatchType type> int getScaledValue(const Vector<int>& scaledValues, in
 
 bool ImageDecoder::frameHasAlphaAtIndex(size_t index) const
 {
-    return !frameIsCompleteAtIndex(index) || m_frameBufferCache[index].hasAlpha();
-}
-
-bool ImageDecoder::frameIsCompleteAtIndex(size_t index) const
-{
-    return (index < m_frameBufferCache.size()) && (m_frameBufferCache[index].status() == ImageFrame::FrameComplete);
+    if (m_frameBufferCache.size() <= index)
+        return true;
+    if (m_frameBufferCache[index].status() == ImageFrame::FrameComplete)
+        return m_frameBufferCache[index].hasAlpha();
+    return true;
 }
 
 unsigned ImageDecoder::frameBytesAtIndex(size_t index) const
index eb97778..89657d0 100644 (file)
@@ -280,12 +280,6 @@ namespace WebCore {
         // Make the best effort guess to check if the requested frame has alpha channel.
         virtual bool frameHasAlphaAtIndex(size_t) const;
 
-        // Whether or not the frame is fully received.
-        virtual bool frameIsCompleteAtIndex(size_t) const;
-
-        // Duration for displaying a frame in seconds. This method is used by animated images only.
-        virtual float frameDurationAtIndex(size_t) const { return 0; }
-
         // Number of bytes in the decoded frame requested. Return 0 if not yet decoded.
         virtual unsigned frameBytesAtIndex(size_t) const;
 
index 1695d67..5c425aa 100644 (file)
@@ -127,19 +127,6 @@ ImageFrame* GIFImageDecoder::frameBufferAtIndex(size_t index)
     return &frame;
 }
 
-
-bool GIFImageDecoder::frameIsCompleteAtIndex(size_t index) const
-{
-    return m_reader && (index < m_reader->imagesCount()) && m_reader->frameContext(index)->isComplete();
-}
-
-float GIFImageDecoder::frameDurationAtIndex(size_t index) const
-{
-    return (m_reader && (index < m_reader->imagesCount()) && m_reader->frameContext(index)->isHeaderDefined())
-        ? m_reader->frameContext(index)->delayTime : 0;
-}
-
-
 bool GIFImageDecoder::setFailed()
 {
     m_reader.clear();
@@ -198,7 +185,7 @@ void GIFImageDecoder::clearFrameBufferCache(size_t clearBeforeFrame)
 
 bool GIFImageDecoder::haveDecodedRow(unsigned frameIndex, const Vector<unsigned char>& rowBuffer, size_t width, size_t rowNumber, unsigned repeatCount, bool writeTransparentPixels)
 {
-    const GIFFrameContext* frameContext = m_reader->frameContext(frameIndex);
+    const GIFFrameContext* frameContext = m_reader->frameContext();
     // The pixel data and coordinates supplied to us are relative to the frame's
     // origin within the entire image size, i.e.
     // (frameContext->xOffset, frameContext->yOffset). There is no guarantee
@@ -310,6 +297,8 @@ void GIFImageDecoder::gifComplete()
     // Cache the repetition count, which is now as authoritative as it's ever
     // going to be.
     repetitionCount();
+
+    m_reader.clear();
 }
 
 void GIFImageDecoder::decode(unsigned haltAtFrame, GIFQuery query)
@@ -346,16 +335,16 @@ void GIFImageDecoder::decode(unsigned haltAtFrame, GIFQuery query)
         return;
     }
 
-    // It is also a fatal error if all data is received and we have decoded all
-    // frames available but the file is truncated.
-    if (haltAtFrame >= m_frameBufferCache.size() && isAllDataReceived() && m_reader && !m_reader->parseCompleted())
+    // It is also a fatal error if all data is received but we failed to decode
+    // all frames completely.
+    if (isAllDataReceived() && haltAtFrame >= m_frameBufferCache.size() && m_reader)
         setFailed();
 }
 
 bool GIFImageDecoder::initFrameBuffer(unsigned frameIndex)
 {
     // Initialize the frame rect in our buffer.
-    const GIFFrameContext* frameContext = m_reader->frameContext(frameIndex);
+    const GIFFrameContext* frameContext = m_reader->frameContext();
     IntRect frameRect(frameContext->xOffset, frameContext->yOffset, frameContext->width, frameContext->height);
 
     // Make sure the frameRect doesn't extend outside the buffer.
index b1e95fa..f9e1b84 100644 (file)
@@ -49,8 +49,6 @@ namespace WebCore {
         virtual size_t frameCount();
         virtual int repetitionCount() const;
         virtual ImageFrame* frameBufferAtIndex(size_t index);
-        virtual bool frameIsCompleteAtIndex(size_t) const;
-        virtual float frameDurationAtIndex(size_t) const;
         // CAUTION: setFailed() deletes |m_reader|.  Be careful to avoid
         // accessing deleted memory, especially when calling this from inside
         // GIFImageReader!
index b2cefbe..bda8997 100644 (file)
@@ -282,13 +282,11 @@ public:
         return frame->isLocalColormapDefined ? frame->localColormapSize : 0;
     }
 
-    const GIFFrameContext* frameContext(size_t index) const
+    const GIFFrameContext* frameContext() const
     {
-        return index < m_frames.size() ? m_frames[index].get() : 0;
+        return m_currentDecodingFrame < m_frames.size() ? m_frames[m_currentDecodingFrame].get() : 0;
     }
 
-    bool parseCompleted() const { return m_parseCompleted; }
-
 private:
     bool parse(size_t dataPosition, size_t len, bool parseSizeOnly);
     void setRemainingBytes(size_t);