Diff
Modified: trunk/Source/WebCore/ChangeLog (135312 => 135313)
--- trunk/Source/WebCore/ChangeLog 2012-11-20 22:25:33 UTC (rev 135312)
+++ trunk/Source/WebCore/ChangeLog 2012-11-20 22:27:07 UTC (rev 135313)
@@ -1,3 +1,33 @@
+2012-11-20 Alpha Lam <[email protected]>
+
+ [chromium] Make lazy image decoding thread-safe
+ https://bugs.webkit.org/show_bug.cgi?id=102721
+
+ Reviewed by Stephen White.
+
+ Added mutex to LazyDecodingPixelRef such that there is no parallel
+ onLockPixels() running on multiple threads.
+
+ Added mutex to ImageFrameGenerator to protect the use of raw image
+ data.
+
+ The result is that we can decode on threads other than the main
+ thread while data is being supplied on the main thread.
+
+ New unit test:
+ DeferredImageDecoderTest.decodeOnOtherThread
+
+ * platform/graphics/chromium/ImageFrameGenerator.cpp:
+ (WebCore::ImageFrameGenerator::setData):
+ (WebCore::ImageFrameGenerator::decodeAndScale):
+ * platform/graphics/chromium/ImageFrameGenerator.h:
+ (ImageFrameGenerator):
+ * platform/graphics/chromium/LazyDecodingPixelRef.cpp:
+ (WebCore::LazyDecodingPixelRef::onLockPixels):
+ (WebCore::LazyDecodingPixelRef::onUnlockPixels):
+ * platform/graphics/chromium/LazyDecodingPixelRef.h:
+ (LazyDecodingPixelRef):
+
2012-11-20 Brent Fulgham <[email protected]>
[Qt] Build fix after r135217.
Modified: trunk/Source/WebCore/platform/graphics/chromium/ImageFrameGenerator.cpp (135312 => 135313)
--- trunk/Source/WebCore/platform/graphics/chromium/ImageFrameGenerator.cpp 2012-11-20 22:25:33 UTC (rev 135312)
+++ trunk/Source/WebCore/platform/graphics/chromium/ImageFrameGenerator.cpp 2012-11-20 22:27:07 UTC (rev 135313)
@@ -46,17 +46,31 @@
void ImageFrameGenerator::setData(PassRefPtr<SharedBuffer> data, bool allDataReceived)
{
- m_data = data;
+ // FIXME: Doing a full copy is expensive, instead copy only new data.
+ RefPtr<SharedBuffer> dataCopy = data->copy();
+
+ MutexLocker lock(m_mutex);
+ m_data = dataCopy;
m_allDataReceived = allDataReceived;
}
SkBitmap ImageFrameGenerator::decodeAndScale(const SkISize& scaledSize, const SkIRect& scaledSubset)
{
- OwnPtr<ImageDecoder> decoder(adoptPtr(ImageDecoder::create(*m_data.get(), ImageSource::AlphaPremultiplied, ImageSource::GammaAndColorProfileApplied)));
+ RefPtr<SharedBuffer> data;
+ bool allDataReceived = false;
+ {
+ MutexLocker lock(m_mutex);
+
+ // FIXME: We should do a shallow copy instead. Now we're restricted by the API of SharedBuffer.
+ data = ""
+ allDataReceived = m_allDataReceived;
+ }
+
+ OwnPtr<ImageDecoder> decoder(adoptPtr(ImageDecoder::create(*data.get(), ImageSource::AlphaPremultiplied, ImageSource::GammaAndColorProfileApplied)));
if (!decoder)
return SkBitmap();
- decoder->setData(m_data.get(), m_allDataReceived);
+ decoder->setData(data.get(), allDataReceived);
ImageFrame* frame = decoder->frameBufferAtIndex(0);
if (!frame)
return SkBitmap();
Modified: trunk/Source/WebCore/platform/graphics/chromium/ImageFrameGenerator.h (135312 => 135313)
--- trunk/Source/WebCore/platform/graphics/chromium/ImageFrameGenerator.h 2012-11-20 22:25:33 UTC (rev 135312)
+++ trunk/Source/WebCore/platform/graphics/chromium/ImageFrameGenerator.h 2012-11-20 22:27:07 UTC (rev 135313)
@@ -33,12 +33,14 @@
#include <wtf/PassRefPtr.h>
#include <wtf/RefCounted.h>
#include <wtf/RefPtr.h>
+#include <wtf/ThreadSafeRefCounted.h>
+#include <wtf/ThreadingPrimitives.h>
namespace WebCore {
class SharedBuffer;
-class ImageFrameGenerator : public RefCounted<ImageFrameGenerator> {
+class ImageFrameGenerator : public ThreadSafeRefCounted<ImageFrameGenerator> {
public:
static PassRefPtr<ImageFrameGenerator> create(PassRefPtr<SharedBuffer> data, bool allDataReceived)
{
@@ -55,6 +57,7 @@
private:
RefPtr<SharedBuffer> m_data;
bool m_allDataReceived;
+ Mutex m_mutex;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/platform/graphics/chromium/LazyDecodingPixelRef.cpp (135312 => 135313)
--- trunk/Source/WebCore/platform/graphics/chromium/LazyDecodingPixelRef.cpp 2012-11-20 22:25:33 UTC (rev 135312)
+++ trunk/Source/WebCore/platform/graphics/chromium/LazyDecodingPixelRef.cpp 2012-11-20 22:27:07 UTC (rev 135313)
@@ -57,20 +57,22 @@
void* LazyDecodingPixelRef::onLockPixels(SkColorTable**)
{
- ASSERT(isMainThread());
+ m_mutex.lock();
ASSERT(m_lockedBitmap.isNull());
m_lockedBitmap = m_frameGenerator->decodeAndScale(m_scaledSize, m_scaledSubset);
- if (m_lockedBitmap.isNull())
+ if (m_lockedBitmap.isNull()) {
+ m_mutex.unlock();
return 0;
+ }
m_lockedBitmap.lockPixels();
return m_lockedBitmap.getAddr(0, 0);
}
void LazyDecodingPixelRef::onUnlockPixels()
{
- ASSERT(isMainThread());
m_lockedBitmap.unlockPixels();
m_lockedBitmap.reset();
+ m_mutex.unlock();
}
bool LazyDecodingPixelRef::onLockPixelsAreWritable() const
Modified: trunk/Source/WebCore/platform/graphics/chromium/LazyDecodingPixelRef.h (135312 => 135313)
--- trunk/Source/WebCore/platform/graphics/chromium/LazyDecodingPixelRef.h 2012-11-20 22:25:33 UTC (rev 135312)
+++ trunk/Source/WebCore/platform/graphics/chromium/LazyDecodingPixelRef.h 2012-11-20 22:27:07 UTC (rev 135313)
@@ -33,6 +33,7 @@
#include "SkSize.h"
#include <wtf/RefPtr.h>
+#include <wtf/ThreadingPrimitives.h>
namespace WebCore {
@@ -61,6 +62,7 @@
SkIRect m_scaledSubset;
SkBitmap m_lockedBitmap;
+ Mutex m_mutex;
};
} // namespace WebCore
Modified: trunk/Source/WebKit/chromium/ChangeLog (135312 => 135313)
--- trunk/Source/WebKit/chromium/ChangeLog 2012-11-20 22:25:33 UTC (rev 135312)
+++ trunk/Source/WebKit/chromium/ChangeLog 2012-11-20 22:27:07 UTC (rev 135313)
@@ -1,3 +1,19 @@
+2012-11-20 Alpha Lam <[email protected]>
+
+ [chromium] Make lazy image decoding thread-safe
+ https://bugs.webkit.org/show_bug.cgi?id=102721
+
+ Reviewed by Stephen White.
+
+ Added a new unit test to verify that lazy image decoding operates
+ correctly on non-main threads.
+
+ * tests/DeferredImageDecoderTest.cpp:
+ (Rasterizer):
+ (WebCore):
+ (WebCore::rasterizeMain):
+ (WebCore::TEST_F):
+
2012-11-20 Sheriff Bot <[email protected]>
Unreviewed, rolling out r135295.
Modified: trunk/Source/WebKit/chromium/tests/DeferredImageDecoderTest.cpp (135312 => 135313)
--- trunk/Source/WebKit/chromium/tests/DeferredImageDecoderTest.cpp 2012-11-20 22:25:33 UTC (rev 135312)
+++ trunk/Source/WebKit/chromium/tests/DeferredImageDecoderTest.cpp 2012-11-20 22:27:07 UTC (rev 135313)
@@ -37,6 +37,7 @@
#include <gtest/gtest.h>
#include <wtf/PassRefPtr.h>
#include <wtf/RefPtr.h>
+#include <wtf/Threading.h>
using namespace WebCore;
@@ -63,6 +64,11 @@
return new SkCanvas(device);
}
+struct Rasterizer {
+ SkCanvas* canvas;
+ SkPicture* picture;
+};
+
class DeferredImageDecoderTest : public ::testing::Test, public MockImageDecoderClient {
public:
virtual void SetUp()
@@ -147,4 +153,40 @@
EXPECT_EQ(SkColorSetARGB(255, 255, 255, 255), canvasBitmap.getColor(49, 50));
}
+static void rasterizeMain(void* arg)
+{
+ Rasterizer* rasterizer = static_cast<Rasterizer*>(arg);
+ rasterizer->canvas->drawPicture(*rasterizer->picture);
+}
+
+TEST_F(DeferredImageDecoderTest, decodeOnOtherThread)
+{
+ WTF::initializeThreading();
+
+ OwnPtr<NativeImageSkia> image(adoptPtr(m_lazyDecoder->frameBufferAtIndex(0)->asNewNativeImage()));
+ EXPECT_EQ(1, image->bitmap().width());
+ EXPECT_EQ(1, image->bitmap().height());
+ EXPECT_FALSE(image->bitmap().isNull());
+ EXPECT_TRUE(image->bitmap().isImmutable());
+
+ SkCanvas* tempCanvas = m_picture.beginRecording(100, 100);
+ tempCanvas->drawBitmap(image->bitmap(), 0, 0);
+ m_picture.endRecording();
+ EXPECT_EQ(0, m_frameBufferRequestCount);
+
+ // Create a thread to rasterize SkPicture.
+ Rasterizer rasterizer;
+ rasterizer.canvas = m_canvas;
+ rasterizer.picture = &m_picture;
+ ThreadIdentifier threadID = createThread(&rasterizeMain, &rasterizer, "RasterThread");
+ waitForThreadCompletion(threadID);
+ EXPECT_EQ(0, m_frameBufferRequestCount);
+
+ SkBitmap canvasBitmap;
+ canvasBitmap.setConfig(SkBitmap::kARGB_8888_Config, 100, 100);
+ ASSERT_TRUE(m_canvas->readPixels(&canvasBitmap, 0, 0));
+ SkAutoLockPixels autoLock(canvasBitmap);
+ EXPECT_EQ(SkColorSetARGB(255, 255, 255, 255), canvasBitmap.getColor(0, 0));
+}
+
} // namespace
