1 files changed, 39 insertions, 16 deletions

diff --git a/WebCore/platform/image-decoders/skia/GIFImageDecoder.cpp b/WebCore/platform/image-decoders/skia/GIFImageDecoder.cpp
index 38bfa97..2d77943 100644
--- a/WebCore/platform/image-decoders/skia/GIFImageDecoder.cpp
+++ b/WebCore/platform/image-decoders/skia/GIFImageDecoder.cpp
@@ -193,6 +193,7 @@ void GIFImageDecoder::clearFrameBufferCache(size_t clearBeforeFrame)
     // can be asked to clear more frames than we currently have.
     if (m_frameBufferCache.isEmpty())
         return; // Nothing to do.
+
     // The "-1" here is tricky.  It does not mean that |clearBeforeFrame| is the
     // last frame we wish to preserve, but rather that we never want to clear
     // the very last frame in the cache: it's empty (so clearing it is
@@ -203,21 +204,36 @@ void GIFImageDecoder::clearFrameBufferCache(size_t clearBeforeFrame)
     // this case.
     clearBeforeFrame = std::min(clearBeforeFrame, m_frameBufferCache.size() - 1);
     const Vector<RGBA32Buffer>::iterator end(m_frameBufferCache.begin() + clearBeforeFrame);
-    for (Vector<RGBA32Buffer>::iterator i(m_frameBufferCache.begin()); i != end; ++i) {
-        if (i->status() == RGBA32Buffer::FrameEmpty)
-            continue; // Nothing to do.
-
-        // The layout of frames is:
-        // [empty frames][complete frames][partial frame][empty frames]
-        // ...where each of these groups may be empty.  We should not clear a
-        // partial frame since that's what's being decoded right now, and we
-        // also should not clear the last complete frame, since it may be needed
-        // when constructing the next frame.  Note that "i + 1" is safe since
-        // i < end < m_frameBufferCache.end().
-        if ((i->status() == RGBA32Buffer::FramePartial) || ((i + 1)->status() != RGBA32Buffer::FrameComplete))
-            break;
-
-        i->clear();
+
+    // We need to preserve frames such that:
+    //   * We don't clear |end|
+    //   * We don't clear the frame we're currently decoding
+    //   * We don't clear any frame from which a future initFrameBuffer() call
+    //     will copy bitmap data
+    // All other frames can be cleared.  Because of the constraints on when
+    // ImageSource::clear() can be called (see ImageSource.h), we're guaranteed
+    // not to have non-empty frames after the frame we're currently decoding.
+    // So, scan backwards from |end| as follows:
+    //   * If the frame is empty, we're still past any frames we care about.
+    //   * If the frame is complete, but is DisposeOverwritePrevious, we'll
+    //     skip over it in future initFrameBuffer() calls.  We can clear it
+    //     unless it's |end|, and keep scanning.  For any other disposal method,
+    //     stop scanning, as we've found the frame initFrameBuffer() will need
+    //     next.
+    //   * If the frame is partial, we're decoding it, so don't clear it; if it
+    //     has a disposal method other than DisposeOverwritePrevious, stop
+    //     scanning, as we'll only need this frame when decoding the next one.
+    Vector<RGBA32Buffer>::iterator i(end);
+    for (; (i != m_frameBufferCache.begin()) && ((i->status() == RGBA32Buffer::FrameEmpty) || (i->disposalMethod() == RGBA32Buffer::DisposeOverwritePrevious)); --i) {
+        if ((i->status() == RGBA32Buffer::FrameComplete) && (i != end))
+            i->clear();
+    }
+
+    // Now |i| holds the last frame we need to preserve; clear prior frames.
+    for (Vector<RGBA32Buffer>::iterator j(m_frameBufferCache.begin()); j != i; ++j) {
+        ASSERT(j->status() != RGBA32Buffer::FramePartial);
+        if (j->status() != RGBA32Buffer::FrameEmpty)
+            j->clear();
     }
 }
 
@@ -278,7 +294,6 @@ bool GIFImageDecoder::initFrameBuffer(unsigned frameIndex)
         // first frame specifies this method, it will get treated like
         // DisposeOverwriteBgcolor below and reset to a completely empty image.)
         const RGBA32Buffer* prevBuffer = &m_frameBufferCache[--frameIndex];
-        ASSERT(prevBuffer->status() == RGBA32Buffer::FrameComplete);
         RGBA32Buffer::FrameDisposalMethod prevMethod =
             prevBuffer->disposalMethod();
         while ((frameIndex > 0)
@@ -286,6 +301,7 @@ bool GIFImageDecoder::initFrameBuffer(unsigned frameIndex)
             prevBuffer = &m_frameBufferCache[--frameIndex];
             prevMethod = prevBuffer->disposalMethod();
         }
+        ASSERT(prevBuffer->status() == RGBA32Buffer::FrameComplete);
 
         if ((prevMethod == RGBA32Buffer::DisposeNotSpecified) ||
                 (prevMethod == RGBA32Buffer::DisposeKeep)) {
@@ -310,6 +326,9 @@ bool GIFImageDecoder::initFrameBuffer(unsigned frameIndex)
               // Unnecessary (but safe); see comments on the similar call above.
               buffer->setHasAlpha(prevBuffer->hasAlpha());
               SkBitmap& bitmap = buffer->bitmap();
+#ifdef ANDROID_ANIMATED_GIF
+              SkAutoLockPixels alp(bitmap);
+#endif
               for (int y = prevRect.y(); y < prevRect.bottom(); ++y) {
                   for (int x = prevRect.x(); x < prevRect.right(); ++x)
                       buffer->setRGBA(bitmap.getAddr32(x, y), 0, 0, 0, 0);
@@ -361,6 +380,10 @@ void GIFImageDecoder::haveDecodedRow(unsigned frameIndex,
     if (!colorMap)
         return;
 
+#ifdef ANDROID_ANIMATED_GIF
+    // Lock the pixels properly.  Should be submitted back to webkit.
+    SkAutoLockPixels alp(buffer.bitmap());
+#endif
     // The buffers that we draw are the entire image's width and height, so a final output frame is
     // width * height RGBA32 values in size.
     //