[webkit-changes] [144242] trunk/Source

Wed, 27 Feb 2013 14:47:32 -0800

Title: [144242] trunk/Source
Revision
144242
Author
[email protected]
Date
2013-02-27 14:49:32 -0800 (Wed, 27 Feb 2013)

Log Message

Unlock partially decoded images after passing them to the ImageDecodingStore
https://bugs.webkit.org/show_bug.cgi?id=110778

Patch by Min Qin <[email protected]> on 2013-02-27
Reviewed by Stephen White.

Source/WebCore:

For partially decoded images, we need to unlock them so that the memory can be freed.
This change unlocks all the image frames after they are passed to ImageDecodingStore.
Unit tests are added in ImageFrameGeneratorTest.

* platform/graphics/chromium/ImageFrameGenerator.cpp:
(WebCore::ImageFrameGenerator::tryToResumeDecodeAndScale):
(WebCore::ImageFrameGenerator::tryToDecodeAndScale):
(WebCore::ImageFrameGenerator::decode):
* platform/image-decoders/ImageDecoder.h:
(ImageDecoder):
(WebCore::ImageDecoder::lockFrameBuffers):
(WebCore::ImageDecoder::unlockFrameBuffers):

Source/WebKit/chromium:

Test for testing that image frames are unlocked after passing to ImageDecodingStore.

* tests/ImageFrameGeneratorTest.cpp:
(WebCore::ImageFrameGeneratorTest::SetUp):
(WebCore::ImageFrameGeneratorTest::frameBuffersUnlocked):
(ImageFrameGeneratorTest):
(WebCore::ImageFrameGeneratorTest::frameBuffersLocked):
(WebCore::TEST_F):
* tests/MockImageDecoder.h:
(WebCore::MockImageDecoderClient::frameBuffersLocked):
(WebCore::MockImageDecoderClient::frameBuffersUnlocked):
(WebCore::MockImageDecoder::unlockFrameBuffers):
(WebCore::MockImageDecoder::lockFrameBuffers):
(MockImageDecoder):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (144241 => 144242)


--- trunk/Source/WebCore/ChangeLog	2013-02-27 22:44:29 UTC (rev 144241)
+++ trunk/Source/WebCore/ChangeLog	2013-02-27 22:49:32 UTC (rev 144242)
@@ -1,3 +1,23 @@
+2013-02-27  Min Qin  <[email protected]>
+
+        Unlock partially decoded images after passing them to the ImageDecodingStore
+        https://bugs.webkit.org/show_bug.cgi?id=110778
+
+        Reviewed by Stephen White.
+
+        For partially decoded images, we need to unlock them so that the memory can be freed.
+        This change unlocks all the image frames after they are passed to ImageDecodingStore.
+        Unit tests are added in ImageFrameGeneratorTest.
+
+        * platform/graphics/chromium/ImageFrameGenerator.cpp:
+        (WebCore::ImageFrameGenerator::tryToResumeDecodeAndScale):
+        (WebCore::ImageFrameGenerator::tryToDecodeAndScale):
+        (WebCore::ImageFrameGenerator::decode):
+        * platform/image-decoders/ImageDecoder.h:
+        (ImageDecoder):
+        (WebCore::ImageDecoder::lockFrameBuffers):
+        (WebCore::ImageDecoder::unlockFrameBuffers):
+
 2013-02-27  Kenneth Russell  <[email protected]>
 
         Insufficient validation when uploading depth textures to WebGL

Modified: trunk/Source/WebCore/platform/graphics/chromium/ImageFrameGenerator.cpp (144241 => 144242)


--- trunk/Source/WebCore/platform/graphics/chromium/ImageFrameGenerator.cpp	2013-02-27 22:44:29 UTC (rev 144241)
+++ trunk/Source/WebCore/platform/graphics/chromium/ImageFrameGenerator.cpp	2013-02-27 22:49:32 UTC (rev 144242)
@@ -142,9 +142,21 @@
     ASSERT(cachedDecoder);
 
     if (m_data.hasNewData()) {
+        // For single frame images, the above ImageDecodingStore::lockCache()
+        // call should lock the pixelRef. As a result, this lockFrameBuffers()
+        // call should always succeed.
+        // TODO: this does not work for the multiframe images, which are not
+        // yet supported by this class.
+        bool frameBuffersLocked = cachedDecoder->lockFrameBuffers();
+        ASSERT_UNUSED(frameBuffersLocked, frameBuffersLocked);
         // Only do decoding if there is new data.
         OwnPtr<ScaledImageFragment> fullSizeImage = decode(&cachedDecoder);
         cachedImage = ImageDecodingStore::instance()->overwriteAndLockCache(this, cachedImage, fullSizeImage.release());
+        // If the image is partially decoded, unlock the frames so that it
+        // can be evicted from the memory. For fully decoded images,
+        // ImageDecodingStore should have deleted the decoder here.
+        if (!cachedImage->isComplete())
+            cachedDecoder->unlockFrameBuffers();
     }
 
     if (m_fullSize == scaledSize)
@@ -171,6 +183,10 @@
 
     const ScaledImageFragment* cachedFullSizeImage = ImageDecodingStore::instance()->insertAndLockCache(
         this, fullSizeImage.release(), decoderContainer.release());
+    // The newly created SkBitmap in the decoder is locked. Unlock it here
+    // if the image is partially decoded.
+    if (!cachedFullSizeImage->isComplete())
+        decoder->unlockFrameBuffers();
 
     if (m_fullSize == scaledSize)
         return cachedFullSizeImage;
@@ -201,9 +217,8 @@
     (*decoder)->setData(data, allDataReceived);
     // If this call returns a newly allocated DiscardablePixelRef, then
     // ImageFrame::m_bitmap and the contained DiscardablePixelRef are locked.
-    // They will be unlocked when ImageDecoder is destroyed since ImageDecoder
-    // owns the ImageFrame. Partially decoded SkBitmap is thus inserted into the
-    // ImageDecodingStore while locked.
+    // They will be unlocked after the image fragment is inserted into
+    // ImageDecodingStore.
     ImageFrame* frame = (*decoder)->frameBufferAtIndex(0);
     (*decoder)->setData(0, false); // Unref SharedBuffer from ImageDecoder.

Modified: trunk/Source/WebCore/platform/image-decoders/ImageDecoder.h (144241 => 144242)


--- trunk/Source/WebCore/platform/image-decoders/ImageDecoder.h	2013-02-27 22:44:29 UTC (rev 144241)
+++ trunk/Source/WebCore/platform/image-decoders/ImageDecoder.h	2013-02-27 22:49:32 UTC (rev 144242)
@@ -422,6 +422,23 @@
                 m_frameBufferCache.resize(1);
             m_frameBufferCache[0].setMemoryAllocator(allocator);
         }
+
+        virtual bool lockFrameBuffers()
+        {
+            bool ret = true;
+            for (unsigned i = 0; i < m_frameBufferCache.size(); ++i) {
+                const SkBitmap& bitmap = m_frameBufferCache[i].getSkBitmap();
+                bitmap.lockPixels();
+                ret = ret && bitmap.getPixels();
+            }
+            return ret;
+        }
+
+        virtual void unlockFrameBuffers()
+        {
+            for (unsigned i = 0; i < m_frameBufferCache.size(); ++i)
+                m_frameBufferCache[i].getSkBitmap().unlockPixels();
+        }
 #endif
     protected:
         void prepareScaleDataIfNecessary();

Modified: trunk/Source/WebKit/chromium/ChangeLog (144241 => 144242)


--- trunk/Source/WebKit/chromium/ChangeLog	2013-02-27 22:44:29 UTC (rev 144241)
+++ trunk/Source/WebKit/chromium/ChangeLog	2013-02-27 22:49:32 UTC (rev 144242)
@@ -1,3 +1,25 @@
+2013-02-27  Min Qin  <[email protected]>
+
+        Unlock partially decoded images after passing them to the ImageDecodingStore
+        https://bugs.webkit.org/show_bug.cgi?id=110778
+
+        Reviewed by Stephen White.
+
+        Test for testing that image frames are unlocked after passing to ImageDecodingStore.
+
+        * tests/ImageFrameGeneratorTest.cpp:
+        (WebCore::ImageFrameGeneratorTest::SetUp):
+        (WebCore::ImageFrameGeneratorTest::frameBuffersUnlocked):
+        (ImageFrameGeneratorTest):
+        (WebCore::ImageFrameGeneratorTest::frameBuffersLocked):
+        (WebCore::TEST_F):
+        * tests/MockImageDecoder.h:
+        (WebCore::MockImageDecoderClient::frameBuffersLocked):
+        (WebCore::MockImageDecoderClient::frameBuffersUnlocked):
+        (WebCore::MockImageDecoder::unlockFrameBuffers):
+        (WebCore::MockImageDecoder::lockFrameBuffers):
+        (MockImageDecoder):
+
 2013-02-27  John Bauman  <[email protected]>
 
         Plugin in iframe may not display

Modified: trunk/Source/WebKit/chromium/tests/ImageFrameGeneratorTest.cpp (144241 => 144242)


--- trunk/Source/WebKit/chromium/tests/ImageFrameGeneratorTest.cpp	2013-02-27 22:44:29 UTC (rev 144241)
+++ trunk/Source/WebKit/chromium/tests/ImageFrameGeneratorTest.cpp	2013-02-27 22:49:32 UTC (rev 144242)
@@ -73,6 +73,8 @@
         m_generator->setImageDecoderFactoryForTesting(MockImageDecoderFactory::create(this));
         m_decodersDestroyed = 0;
         m_frameBufferRequestCount = 0;
+        m_frameBufferLockCount = 0;
+        m_frameBufferUnlockCount = 0;
         m_frameStatus = ImageFrame::FrameEmpty;
     }
 
@@ -91,6 +93,16 @@
         ++m_frameBufferRequestCount;
     }
 
+    virtual void frameBuffersUnlocked()
+    {
+        ++m_frameBufferUnlockCount;
+    }
+
+    virtual void frameBuffersLocked()
+    {
+        ++m_frameBufferLockCount;
+    }
+
     virtual ImageFrame::FrameStatus frameStatus()
     {
         return m_frameStatus;
@@ -117,6 +129,8 @@
     RefPtr<ImageFrameGenerator> m_generator;
     int m_decodersDestroyed;
     int m_frameBufferRequestCount;
+    int m_frameBufferLockCount;
+    int m_frameBufferUnlockCount;
     ImageFrame::FrameStatus m_frameStatus;
 };
 
@@ -141,6 +155,7 @@
     EXPECT_TRUE(m_generator->hasAlpha());
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), tempImage);
     EXPECT_EQ(0, m_frameBufferRequestCount);
+    EXPECT_EQ(0, m_frameBufferLockCount);
 }
 
 TEST_F(ImageFrameGeneratorTest, cacheMissWithScale)
@@ -164,6 +179,8 @@
     EXPECT_TRUE(m_generator->hasAlpha());
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), tempImage);
     EXPECT_EQ(0, m_frameBufferRequestCount);
+    EXPECT_EQ(0, m_frameBufferLockCount);
+    EXPECT_EQ(0, m_frameBufferUnlockCount);
 }
 
 TEST_F(ImageFrameGeneratorTest, cacheMissWithDecodeAndScale)
@@ -173,6 +190,8 @@
     // Cache miss.
     const ScaledImageFragment* scaledImage = m_generator->decodeAndScale(scaledSize());
     EXPECT_EQ(1, m_frameBufferRequestCount);
+    EXPECT_EQ(0, m_frameBufferLockCount);
+    EXPECT_EQ(0, m_frameBufferUnlockCount);
     EXPECT_EQ(scaledSize(), scaledImage->scaledSize());
     EXPECT_FALSE(m_generator->hasAlpha());
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), scaledImage);
@@ -192,6 +211,8 @@
     EXPECT_FALSE(m_generator->hasAlpha());
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), tempImage);
     EXPECT_EQ(1, m_frameBufferRequestCount);
+    EXPECT_EQ(0, m_frameBufferLockCount);
+    EXPECT_EQ(0, m_frameBufferUnlockCount);
 }
 
 TEST_F(ImageFrameGeneratorTest, cacheMissWithIncompleteDecode)
@@ -201,6 +222,8 @@
     const ScaledImageFragment* tempImage= m_generator->decodeAndScale(fullSize());
     EXPECT_FALSE(tempImage->isComplete());
     EXPECT_EQ(1, m_frameBufferRequestCount);
+    EXPECT_EQ(0, m_frameBufferLockCount);
+    EXPECT_EQ(1, m_frameBufferUnlockCount);
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), tempImage);
     EXPECT_EQ(1u, ImageDecodingStore::instance()->cacheEntries());
 
@@ -208,6 +231,8 @@
     tempImage = m_generator->decodeAndScale(fullSize());
     EXPECT_FALSE(tempImage->isComplete());
     EXPECT_EQ(2, m_frameBufferRequestCount);
+    EXPECT_EQ(1, m_frameBufferLockCount);
+    EXPECT_EQ(2, m_frameBufferUnlockCount);
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), tempImage);
     EXPECT_EQ(1u, ImageDecodingStore::instance()->cacheEntries());
     EXPECT_EQ(0, m_decodersDestroyed);
@@ -220,12 +245,16 @@
     const ScaledImageFragment* tempImage= m_generator->decodeAndScale(fullSize());
     EXPECT_FALSE(tempImage->isComplete());
     EXPECT_EQ(1, m_frameBufferRequestCount);
+    EXPECT_EQ(0, m_frameBufferLockCount);
+    EXPECT_EQ(1, m_frameBufferUnlockCount);
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), tempImage);
     EXPECT_EQ(1u, ImageDecodingStore::instance()->cacheEntries());
 
     tempImage = m_generator->decodeAndScale(fullSize());
     EXPECT_FALSE(tempImage->isComplete());
     EXPECT_EQ(1, m_frameBufferRequestCount);
+    EXPECT_EQ(0, m_frameBufferLockCount);
+    EXPECT_EQ(1, m_frameBufferUnlockCount);
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), tempImage);
     EXPECT_EQ(1u, ImageDecodingStore::instance()->cacheEntries());
     EXPECT_EQ(0, m_decodersDestroyed);
@@ -238,6 +267,8 @@
     const ScaledImageFragment* tempImage= m_generator->decodeAndScale(scaledSize());
     EXPECT_FALSE(tempImage->isComplete());
     EXPECT_EQ(1, m_frameBufferRequestCount);
+    EXPECT_EQ(0, m_frameBufferLockCount);
+    EXPECT_EQ(1, m_frameBufferUnlockCount);
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), tempImage);
     EXPECT_EQ(2u, ImageDecodingStore::instance()->cacheEntries());
 
@@ -245,6 +276,8 @@
     tempImage = m_generator->decodeAndScale(scaledSize());
     EXPECT_FALSE(tempImage->isComplete());
     EXPECT_EQ(2, m_frameBufferRequestCount);
+    EXPECT_EQ(1, m_frameBufferLockCount);
+    EXPECT_EQ(2, m_frameBufferUnlockCount);
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), tempImage);
     EXPECT_EQ(2u, ImageDecodingStore::instance()->cacheEntries());
     EXPECT_EQ(0, m_decodersDestroyed);
@@ -258,6 +291,8 @@
     EXPECT_FALSE(tempImage->isComplete());
     EXPECT_EQ(1, m_frameBufferRequestCount);
     EXPECT_EQ(0, m_decodersDestroyed);
+    EXPECT_EQ(0, m_frameBufferLockCount);
+    EXPECT_EQ(1, m_frameBufferUnlockCount);
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), tempImage);
     EXPECT_EQ(1u, ImageDecodingStore::instance()->cacheEntries());
 
@@ -268,6 +303,8 @@
     EXPECT_TRUE(tempImage->isComplete());
     EXPECT_EQ(2, m_frameBufferRequestCount);
     EXPECT_EQ(1, m_decodersDestroyed);
+    EXPECT_EQ(1, m_frameBufferLockCount);
+    EXPECT_EQ(1, m_frameBufferUnlockCount);
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), tempImage);
     EXPECT_EQ(1u, ImageDecodingStore::instance()->cacheEntries());
 
@@ -285,6 +322,8 @@
     EXPECT_FALSE(tempImage->isComplete());
     EXPECT_EQ(1, m_frameBufferRequestCount);
     EXPECT_EQ(0, m_decodersDestroyed);
+    EXPECT_EQ(0, m_frameBufferLockCount);
+    EXPECT_EQ(1, m_frameBufferUnlockCount);
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), tempImage);
     EXPECT_EQ(2u, ImageDecodingStore::instance()->cacheEntries());
 
@@ -295,6 +334,8 @@
     EXPECT_TRUE(tempImage->isComplete());
     EXPECT_EQ(2, m_frameBufferRequestCount);
     EXPECT_EQ(1, m_decodersDestroyed);
+    EXPECT_EQ(1, m_frameBufferLockCount);
+    EXPECT_EQ(1, m_frameBufferUnlockCount);
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), tempImage);
     EXPECT_EQ(2u, ImageDecodingStore::instance()->cacheEntries());
 
@@ -325,6 +366,8 @@
     EXPECT_FALSE(tempImage->isComplete());
     EXPECT_EQ(1, m_frameBufferRequestCount);
     EXPECT_EQ(0, m_decodersDestroyed);
+    EXPECT_EQ(0, m_frameBufferLockCount);
+    EXPECT_EQ(1, m_frameBufferUnlockCount);
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), tempImage);
     EXPECT_EQ(1u, ImageDecodingStore::instance()->cacheEntries());
 
@@ -336,11 +379,15 @@
 
     EXPECT_EQ(2, m_frameBufferRequestCount);
     EXPECT_EQ(1, m_decodersDestroyed);
+    EXPECT_EQ(1, m_frameBufferLockCount);
+    EXPECT_EQ(1, m_frameBufferUnlockCount);
     EXPECT_EQ(1u, ImageDecodingStore::instance()->cacheEntries());
 
     tempImage = m_generator->decodeAndScale(fullSize());
     EXPECT_TRUE(tempImage->isComplete());
     EXPECT_EQ(2, m_frameBufferRequestCount);
+    EXPECT_EQ(1, m_frameBufferLockCount);
+    EXPECT_EQ(1, m_frameBufferUnlockCount);
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), tempImage);
 }

Modified: trunk/Source/WebKit/chromium/tests/MockImageDecoder.h (144241 => 144242)


--- trunk/Source/WebKit/chromium/tests/MockImageDecoder.h	2013-02-27 22:44:29 UTC (rev 144241)
+++ trunk/Source/WebKit/chromium/tests/MockImageDecoder.h	2013-02-27 22:49:32 UTC (rev 144242)
@@ -33,6 +33,8 @@
 public:
     virtual void decoderBeingDestroyed() = 0;
     virtual void frameBufferRequested() = 0;
+    virtual void frameBuffersLocked() { }
+    virtual void frameBuffersUnlocked() { }
     virtual ImageFrame::FrameStatus frameStatus() = 0;
 };
 
@@ -75,6 +77,13 @@
         return &m_frameBufferCache[0];
     }
 
+    virtual bool lockFrameBuffers()
+    {
+        m_client->frameBuffersLocked();
+        return true;
+    }
+    virtual void unlockFrameBuffers() { m_client->frameBuffersUnlocked(); }
+
     int frameBufferRequestCount() const { return m_frameBufferRequestCount; }
 
 private:

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to