[chromium] Minimize memcpy for encoded image data in ImageFrameGenerator
authorhclam@chromium.org <hclam@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Dec 2012 21:29:25 +0000 (21:29 +0000)
committerhclam@chromium.org <hclam@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Dec 2012 21:29:25 +0000 (21:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=103797

Reviewed by Stephen White.

Source/WebCore:

Added a new class ThreadSafeDataTransport to help transporting data in
SharedBuffer from one thread to another. This class is designed to minimize
thread contention and memory copy.

It is implemented using a data queue. New data segments are copied into
this queue. The queue is then consolidated with existing data when user
requests access to the final SharedBuffer.

Memory is copied into and out of the queue but this approach avoid
copying the entire SharedBuffer.

ImageFrameGenerator is now using this class for data transport between
threads.

Unit tests in ThreadSafeDataTransportTest. Also existing ImageFrameGeneratorTest passes.

* WebCore.gypi:
* platform/graphics/chromium/ImageFrameGenerator.cpp:
(WebCore::ImageFrameGenerator::ImageFrameGenerator):
(WebCore::ImageFrameGenerator::setData):
(WebCore::ImageFrameGenerator::tryToResumeDecodeAndScale):
Added logic to skip decoding if there's no new data.
(WebCore::ImageFrameGenerator::decode):
* platform/graphics/chromium/ImageFrameGenerator.h:
(ImageFrameGenerator):
* platform/graphics/chromium/ThreadSafeDataTransport.cpp: Added.
(WebCore):
(WebCore::ThreadSafeDataTransport::ThreadSafeDataTransport):
(WebCore::ThreadSafeDataTransport::~ThreadSafeDataTransport):
(WebCore::ThreadSafeDataTransport::setData):
(WebCore::ThreadSafeDataTransport::data):
(WebCore::ThreadSafeDataTransport::hasNewData):
* platform/graphics/chromium/ThreadSafeDataTransport.h: Added.
(WebCore):
(ThreadSafeDataTransport):

Source/WebKit/chromium:

Added unit tests for ThreadSafeDataTransport.

Added unit tests for ImageFrameGenerator to test behavior with and without
new data.

New tests:
ThreadSafeDataTransportTest.hasNewData
ThreadSafeDataTransportTest.setData
ImageFrameGeneratorTest.cacheMissWithIncompleteDecodeNoNewData

* WebKit.gypi:
* tests/ThreadSafeDataTransportTest.cpp: Added.
(WebCore):
(WebCore::TEST):

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

Source/WebCore/ChangeLog
Source/WebCore/WebCore.gypi
Source/WebCore/platform/graphics/chromium/ImageFrameGenerator.cpp
Source/WebCore/platform/graphics/chromium/ImageFrameGenerator.h
Source/WebCore/platform/graphics/chromium/ThreadSafeDataTransport.cpp [new file with mode: 0644]
Source/WebCore/platform/graphics/chromium/ThreadSafeDataTransport.h [new file with mode: 0644]
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/WebKit.gypi
Source/WebKit/chromium/tests/ImageFrameGeneratorTest.cpp
Source/WebKit/chromium/tests/ThreadSafeDataTransportTest.cpp [new file with mode: 0644]

index f1759ce..63cfeae 100644 (file)
@@ -1,3 +1,46 @@
+2012-12-12  Alpha Lam  <hclam@chromium.org>
+
+        [chromium] Minimize memcpy for encoded image data in ImageFrameGenerator
+        https://bugs.webkit.org/show_bug.cgi?id=103797
+
+        Reviewed by Stephen White.
+
+        Added a new class ThreadSafeDataTransport to help transporting data in
+        SharedBuffer from one thread to another. This class is designed to minimize
+        thread contention and memory copy.
+
+        It is implemented using a data queue. New data segments are copied into
+        this queue. The queue is then consolidated with existing data when user
+        requests access to the final SharedBuffer.
+
+        Memory is copied into and out of the queue but this approach avoid
+        copying the entire SharedBuffer.
+
+        ImageFrameGenerator is now using this class for data transport between
+        threads.
+
+        Unit tests in ThreadSafeDataTransportTest. Also existing ImageFrameGeneratorTest passes.
+
+        * WebCore.gypi:
+        * platform/graphics/chromium/ImageFrameGenerator.cpp:
+        (WebCore::ImageFrameGenerator::ImageFrameGenerator):
+        (WebCore::ImageFrameGenerator::setData):
+        (WebCore::ImageFrameGenerator::tryToResumeDecodeAndScale):
+        Added logic to skip decoding if there's no new data.
+        (WebCore::ImageFrameGenerator::decode):
+        * platform/graphics/chromium/ImageFrameGenerator.h:
+        (ImageFrameGenerator):
+        * platform/graphics/chromium/ThreadSafeDataTransport.cpp: Added.
+        (WebCore):
+        (WebCore::ThreadSafeDataTransport::ThreadSafeDataTransport):
+        (WebCore::ThreadSafeDataTransport::~ThreadSafeDataTransport):
+        (WebCore::ThreadSafeDataTransport::setData):
+        (WebCore::ThreadSafeDataTransport::data):
+        (WebCore::ThreadSafeDataTransport::hasNewData):
+        * platform/graphics/chromium/ThreadSafeDataTransport.h: Added.
+        (WebCore):
+        (ThreadSafeDataTransport):
+
 2012-12-12  Roger Fong  <roger_fong@apple.com>
 
         Enable VIDEO_TRACK on Windows.
index a673404..32acbc1 100644 (file)
             'platform/graphics/chromium/ScaledImageFragment.cpp',
             'platform/graphics/chromium/ScaledImageFragment.h',
             'platform/graphics/chromium/SimpleFontDataChromiumWin.cpp',
+            'platform/graphics/chromium/ThreadSafeDataTransport.cpp',
+            'platform/graphics/chromium/ThreadSafeDataTransport.h',
             'platform/graphics/chromium/TransparencyWin.cpp',
             'platform/graphics/chromium/TransparencyWin.h',
             'platform/graphics/chromium/UniscribeHelper.cpp',
index 2c2a1d2..3c98df7 100644 (file)
@@ -47,10 +47,9 @@ skia::ImageOperations::ResizeMethod resizeMethod()
 
 ImageFrameGenerator::ImageFrameGenerator(const SkISize& fullSize, PassRefPtr<SharedBuffer> data, bool allDataReceived)
     : m_fullSize(fullSize)
-    , m_allDataReceived(false)
     , m_decodeFailedAndEmpty(false)
 {
-    setData(data, allDataReceived);
+    setData(data.get(), allDataReceived);
 }
 
 ImageFrameGenerator::~ImageFrameGenerator()
@@ -64,12 +63,7 @@ ImageFrameGenerator::~ImageFrameGenerator()
 
 void ImageFrameGenerator::setData(PassRefPtr<SharedBuffer> data, bool allDataReceived)
 {
-    // FIXME: Doing a full copy is expensive, instead copy only new data.
-    RefPtr<SharedBuffer> dataCopy = data->copy();
-
-    MutexLocker lock(m_dataMutex);
-    m_data = dataCopy;
-    m_allDataReceived = allDataReceived;
+    m_data.setData(data.get(), allDataReceived);
 }
 
 const ScaledImageFragment* ImageFrameGenerator::decodeAndScale(const SkISize& scaledSize)
@@ -141,14 +135,15 @@ const ScaledImageFragment* ImageFrameGenerator::tryToResumeDecodeAndScale(const
         return 0;
     ASSERT(cachedDecoder);
 
-    OwnPtr<ScaledImageFragment> fullSizeImage = decode(&cachedDecoder);
-
-    const ScaledImageFragment* cachedFullSizeImage = ImageDecodingStore::instance()->overwriteAndLockCache(
-        this, cachedImage, fullSizeImage.release());
+    if (m_data.hasNewData()) {
+        // Only do decoding if there is new data.
+        OwnPtr<ScaledImageFragment> fullSizeImage = decode(&cachedDecoder);
+        cachedImage = ImageDecodingStore::instance()->overwriteAndLockCache(this, cachedImage, fullSizeImage.release());
+    }
 
     if (m_fullSize == scaledSize)
-        return cachedFullSizeImage;
-    return tryToScale(cachedFullSizeImage, scaledSize);
+        return cachedImage;
+    return tryToScale(cachedImage, scaledSize);
 }
 
 const ScaledImageFragment* ImageFrameGenerator::tryToDecodeAndScale(const SkISize& scaledSize)
@@ -179,13 +174,13 @@ const ScaledImageFragment* ImageFrameGenerator::tryToDecodeAndScale(const SkISiz
 PassOwnPtr<ScaledImageFragment> ImageFrameGenerator::decode(ImageDecoder** decoder)
 {
     ASSERT(decoder);
-    RefPtr<SharedBuffer> data;
+    SharedBuffer* data = 0;
     bool allDataReceived = false;
-    prepareData(&data, &allDataReceived);
+    m_data.data(&data, &allDataReceived);
 
     // Try to create an ImageDecoder if we are not given one.
     if (!*decoder) {
-        *decoder = ImageDecoder::create(*data.get(), ImageSource::AlphaPremultiplied, ImageSource::GammaAndColorProfileApplied);
+        *decoder = ImageDecoder::create(*data, ImageSource::AlphaPremultiplied, ImageSource::GammaAndColorProfileApplied);
 
         if (!*decoder && m_imageDecoderFactory)
             *decoder = m_imageDecoderFactory->create().leakPtr();
@@ -194,7 +189,7 @@ PassOwnPtr<ScaledImageFragment> ImageFrameGenerator::decode(ImageDecoder** decod
             return nullptr;
     }
 
-    (*decoder)->setData(data.get(), allDataReceived);
+    (*decoder)->setData(data, allDataReceived);
     ImageFrame* frame = (*decoder)->frameBufferAtIndex(0);
     (*decoder)->setData(0, false); // Unref SharedBuffer from ImageDecoder.
 
@@ -208,13 +203,4 @@ PassOwnPtr<ScaledImageFragment> ImageFrameGenerator::decode(ImageDecoder** decod
     return ScaledImageFragment::create(m_fullSize, fullSizeBitmap, isComplete);
 }
 
-void ImageFrameGenerator::prepareData(RefPtr<SharedBuffer>* data, bool* allDataReceived)
-{
-    MutexLocker lock(m_dataMutex);
-
-    // FIXME: We should do a shallow copy instead. Now we're restricted by the API of SharedBuffer.
-    *data = m_data->copy();
-    *allDataReceived = m_allDataReceived;
-}
-
 } // namespace WebCore
index ba49aa9..1c0037c 100644 (file)
@@ -29,6 +29,7 @@
 #include "SkTypes.h"
 #include "SkBitmap.h"
 #include "SkSize.h"
+#include "ThreadSafeDataTransport.h"
 #include <wtf/PassOwnPtr.h>
 #include <wtf/PassRefPtr.h>
 #include <wtf/RefCounted.h>
@@ -74,20 +75,15 @@ private:
 
     // Use the given decoder to decode. If a decoder is not given then try to create one.
     PassOwnPtr<ScaledImageFragment> decode(ImageDecoder**);
-    void prepareData(RefPtr<SharedBuffer>*, bool* allDataReceived);
 
     SkISize m_fullSize;
-    RefPtr<SharedBuffer> m_data;
-    bool m_allDataReceived;
+    ThreadSafeDataTransport m_data;
     bool m_decodeFailedAndEmpty;
 
     OwnPtr<ImageDecoderFactory> m_imageDecoderFactory;
 
     // Prevents multiple decode operations on the same data.
     Mutex m_decodeMutex;
-
-    // Prevents concurrent access to m_data.
-    Mutex m_dataMutex;
 };
 
 } // namespace WebCore
diff --git a/Source/WebCore/platform/graphics/chromium/ThreadSafeDataTransport.cpp b/Source/WebCore/platform/graphics/chromium/ThreadSafeDataTransport.cpp
new file mode 100644 (file)
index 0000000..05b6b46
--- /dev/null
@@ -0,0 +1,81 @@
+/*
+ * Copyright (C) 2012 Google Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE COMPUTER, INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+
+#include "ThreadSafeDataTransport.h"
+
+#include "SharedBuffer.h"
+
+namespace WebCore {
+
+ThreadSafeDataTransport::ThreadSafeDataTransport()
+    : m_readBuffer(SharedBuffer::create())
+    , m_readPosition(0)
+{
+}
+
+ThreadSafeDataTransport::~ThreadSafeDataTransport()
+{
+}
+
+void ThreadSafeDataTransport::setData(SharedBuffer* buffer, bool allDataReceived)
+{
+    ASSERT(buffer->size() >= m_readPosition);
+    Vector<RefPtr<SharedBuffer> > newBufferQueue;
+
+    const char* segment = 0;
+    while (size_t length = buffer->getSomeData(segment, m_readPosition)) {
+        m_readPosition += length;
+        newBufferQueue.append(SharedBuffer::create(segment, length));
+    }
+
+    MutexLocker locker(m_mutex);
+    m_newBufferQueue.append(newBufferQueue);
+    m_allDataReceived = allDataReceived;
+}
+
+void ThreadSafeDataTransport::data(SharedBuffer** buffer, bool* allDataReceived)
+{
+    Vector<RefPtr<SharedBuffer> > newBufferQueue;
+    {
+        MutexLocker lock(m_mutex);
+        m_newBufferQueue.swap(newBufferQueue);
+    }
+    for (size_t i = 0; i < newBufferQueue.size(); ++i)
+        m_readBuffer->append(newBufferQueue[i].get());
+    ASSERT(buffer);
+    ASSERT(allDataReceived);
+    *buffer = m_readBuffer.get();
+    *allDataReceived = m_allDataReceived;
+}
+
+bool ThreadSafeDataTransport::hasNewData()
+{
+    MutexLocker lock(m_mutex);
+    return !m_newBufferQueue.isEmpty();
+}
+
+} // namespace WebCore
diff --git a/Source/WebCore/platform/graphics/chromium/ThreadSafeDataTransport.h b/Source/WebCore/platform/graphics/chromium/ThreadSafeDataTransport.h
new file mode 100644 (file)
index 0000000..81cc64b
--- /dev/null
@@ -0,0 +1,72 @@
+/*
+ * Copyright (C) 2012 Google Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE COMPUTER, INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef ThreadSafeDataTransport_h
+#define ThreadSafeDataTransport_h
+
+#include <wtf/OwnPtr.h>
+#include <wtf/PassOwnPtr.h>
+#include <wtf/RefPtr.h>
+#include <wtf/ThreadingPrimitives.h>
+#include <wtf/Vector.h>
+
+namespace WebCore {
+
+class SharedBuffer;
+
+// The purpose of this class is to allow the transfer of data stored in
+// SharedBuffer in a thread-safe manner, and to minimize memory copies
+// and thread contention.
+//
+// This class is designed such that there is only one producer and
+// one consumer.
+class ThreadSafeDataTransport {
+public:
+    ThreadSafeDataTransport();
+    ~ThreadSafeDataTransport();
+
+    // This method is being called subsequently with an expanding
+    // SharedBuffer.
+    void setData(SharedBuffer*, bool allDataReceived);
+
+    // Get the data submitted to this class so far.
+    void data(SharedBuffer**, bool* allDataReceived);
+
+    // Return true of there is new data submitted to this class
+    // since last time data() was called.
+    bool hasNewData();
+
+private:
+    Mutex m_mutex;
+
+    Vector<RefPtr<SharedBuffer> > m_newBufferQueue;
+    RefPtr<SharedBuffer> m_readBuffer;
+    bool m_allDataReceived;
+    size_t m_readPosition;
+};
+
+} // namespace WebCore
+
+#endif
index b7126ce..862dd4a 100644 (file)
@@ -1,3 +1,25 @@
+2012-12-12  Alpha Lam  <hclam@chromium.org>
+
+        [chromium] Minimize memcpy for encoded image data in ImageFrameGenerator
+        https://bugs.webkit.org/show_bug.cgi?id=103797
+
+        Reviewed by Stephen White.
+
+        Added unit tests for ThreadSafeDataTransport.
+
+        Added unit tests for ImageFrameGenerator to test behavior with and without
+        new data.
+
+        New tests:
+        ThreadSafeDataTransportTest.hasNewData
+        ThreadSafeDataTransportTest.setData
+        ImageFrameGeneratorTest.cacheMissWithIncompleteDecodeNoNewData
+
+        * WebKit.gypi:
+        * tests/ThreadSafeDataTransportTest.cpp: Added.
+        (WebCore):
+        (WebCore::TEST):
+
 2012-12-12  Mark Pilgrim  <pilgrim@chromium.org>
 
         [Chromium] Remove all references to sharedWorkerRepository()
index 26f509a..2349367 100644 (file)
             'tests/RenderTableCellTest.cpp',
             'tests/RenderTableRowTest.cpp',
             'tests/ScrollingCoordinatorChromiumTest.cpp',
+            'tests/ThreadSafeDataTransportTest.cpp',
             'tests/TilingDataTest.cpp',
             'tests/TreeTestHelpers.cpp',
             'tests/TreeTestHelpers.h',
index 36f407b..b4169a1 100644 (file)
@@ -105,6 +105,12 @@ protected:
         return ScaledImageFragment::create(size, bitmap, true);
     }
 
+    void addNewData()
+    {
+        m_data->append("g", 1);
+        m_generator->setData(m_data, false);
+    }
+
     void setFrameStatus(ImageFrame::FrameStatus status)  { m_frameStatus = status; }
 
     RefPtr<SharedBuffer> m_data;
@@ -191,6 +197,7 @@ TEST_F(ImageFrameGeneratorTest, cacheMissWithIncompleteDecode)
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), tempImage);
     EXPECT_EQ(1u, ImageDecodingStore::instance()->cacheEntries());
 
+    addNewData();
     tempImage = m_generator->decodeAndScale(fullSize());
     EXPECT_FALSE(tempImage->isComplete());
     EXPECT_EQ(2, m_frameBufferRequestCount);
@@ -199,6 +206,24 @@ TEST_F(ImageFrameGeneratorTest, cacheMissWithIncompleteDecode)
     EXPECT_EQ(0, m_decodersDestroyed);
 }
 
+TEST_F(ImageFrameGeneratorTest, cacheMissWithIncompleteDecodeNoNewData)
+{
+    setFrameStatus(ImageFrame::FramePartial);
+
+    const ScaledImageFragment* tempImage= m_generator->decodeAndScale(fullSize());
+    EXPECT_FALSE(tempImage->isComplete());
+    EXPECT_EQ(1, m_frameBufferRequestCount);
+    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);
+    ImageDecodingStore::instance()->unlockCache(m_generator.get(), tempImage);
+    EXPECT_EQ(1u, ImageDecodingStore::instance()->cacheEntries());
+    EXPECT_EQ(0, m_decodersDestroyed);
+}
+
 TEST_F(ImageFrameGeneratorTest, cacheMissWithIncompleteDecodeAndScale)
 {
     setFrameStatus(ImageFrame::FramePartial);
@@ -209,6 +234,7 @@ TEST_F(ImageFrameGeneratorTest, cacheMissWithIncompleteDecodeAndScale)
     ImageDecodingStore::instance()->unlockCache(m_generator.get(), tempImage);
     EXPECT_EQ(2u, ImageDecodingStore::instance()->cacheEntries());
 
+    addNewData();
     tempImage = m_generator->decodeAndScale(scaledSize());
     EXPECT_FALSE(tempImage->isComplete());
     EXPECT_EQ(2, m_frameBufferRequestCount);
@@ -229,6 +255,7 @@ TEST_F(ImageFrameGeneratorTest, incompleteDecodeBecomesComplete)
     EXPECT_EQ(1u, ImageDecodingStore::instance()->cacheEntries());
 
     setFrameStatus(ImageFrame::FrameComplete);
+    addNewData();
 
     tempImage = m_generator->decodeAndScale(fullSize());
     EXPECT_TRUE(tempImage->isComplete());
@@ -255,6 +282,7 @@ TEST_F(ImageFrameGeneratorTest, incompleteDecodeAndScaleBecomesComplete)
     EXPECT_EQ(2u, ImageDecodingStore::instance()->cacheEntries());
 
     setFrameStatus(ImageFrame::FrameComplete);
+    addNewData();
 
     tempImage = m_generator->decodeAndScale(scaledSize());
     EXPECT_TRUE(tempImage->isComplete());
@@ -295,6 +323,7 @@ TEST_F(ImageFrameGeneratorTest, incompleteDecodeBecomesCompleteMultiThreaded)
 
     // Frame can now be decoded completely.
     setFrameStatus(ImageFrame::FrameComplete);
+    addNewData();
     ThreadIdentifier threadID = createThread(&decodeThreadMain, m_generator.get(), "DecodeThread");
     waitForThreadCompletion(threadID);
 
diff --git a/Source/WebKit/chromium/tests/ThreadSafeDataTransportTest.cpp b/Source/WebKit/chromium/tests/ThreadSafeDataTransportTest.cpp
new file mode 100644 (file)
index 0000000..0a8958d
--- /dev/null
@@ -0,0 +1,79 @@
+/*
+ * Copyright (C) 2012 Google Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1.  Redistributions of source code must retain the above copyright
+ *     notice, this list of conditions and the following disclaimer.
+ * 2.  Redistributions in binary form must reproduce the above copyright
+ *     notice, this list of conditions and the following disclaimer in the
+ *     documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+
+#include "ThreadSafeDataTransport.h"
+
+#include "SharedBuffer.h"
+#include <gtest/gtest.h>
+
+using namespace WebCore;
+
+namespace {
+
+TEST(ThreadSafeDataTransportTest, hasNewData)
+{
+    ThreadSafeDataTransport transport;
+
+    const char testString[] = "123456789";
+    RefPtr<SharedBuffer> buffer = SharedBuffer::create(testString, sizeof(testString));
+
+    transport.setData(buffer.get(), false);
+    EXPECT_TRUE(transport.hasNewData());
+
+    SharedBuffer* tempBuffer = 0;
+    bool allDataReceived = false;
+    transport.data(&tempBuffer, &allDataReceived);
+    EXPECT_FALSE(transport.hasNewData());
+
+    transport.setData(buffer.get(), false);
+    EXPECT_FALSE(transport.hasNewData());
+}
+
+TEST(ThreadSafeDataTransportTest, setData)
+{
+    ThreadSafeDataTransport transport;
+
+    const char testString1[] = "123";
+    RefPtr<SharedBuffer> buffer1 = SharedBuffer::create(testString1, sizeof(testString1) - 1);
+    const char testString2[] = "12345";
+    RefPtr<SharedBuffer> buffer2 = SharedBuffer::create(testString2, sizeof(testString2) - 1);
+    const char testString3[] = "1234567890";
+    RefPtr<SharedBuffer> buffer3 = SharedBuffer::create(testString3, sizeof(testString3) - 1);
+
+    transport.setData(buffer1.get(), false);
+    transport.setData(buffer2.get(), false);
+    transport.setData(buffer3.get(), true);
+    EXPECT_TRUE(transport.hasNewData());
+
+    SharedBuffer* tempBuffer = 0;
+    bool allDataReceived = false;
+    transport.data(&tempBuffer, &allDataReceived);
+    EXPECT_TRUE(allDataReceived);
+    EXPECT_FALSE(memcmp(testString3, tempBuffer->data(), tempBuffer->size()));
+}
+
+} // namespace