From 787434819f79ddf7bb8d458b847f4ed6ae57bae5 Mon Sep 17 00:00:00 2001 From: Said Abou-Hallawa Date: Mon, 16 Oct 2017 13:10:27 +0000 Subject: [PATCH] Merge r222530 - Followup (r222427): SynchronizedFixedQueue should not have a public constructor https://bugs.webkit.org/show_bug.cgi?id=177458 Patch by Said Abou-Hallawa on 2017-09-26 Reviewed by Tim Horton. Source/WebCore: ImageFrameCache::decodingQueue() and frameRequestQueue() should return raw references instead of Ref objects. * platform/graphics/ImageFrameCache.cpp: (WebCore::ImageFrameCache::decodingQueue): (WebCore::ImageFrameCache::frameRequestQueue): (WebCore::ImageFrameCache::startAsyncDecodingQueue): * platform/graphics/ImageFrameCache.h: Source/WTF: Since SynchronizedFixedQueue is now derived from ThreadSafeRefCounted, the standard way to have an instance of it is to call SynchronizedFixedQueue::create() which returns a Ref. Now it does not make sense to still have the constructor public. * wtf/SynchronizedFixedQueue.h: (WTF::SynchronizedFixedQueue::SynchronizedFixedQueue): Tools: Fix the definition and the creation of SynchronizedFixedQueue. * TestWebKitAPI/Tests/WTF/SynchronizedFixedQueue.cpp: (TestWebKitAPI::ToUpperConverter::ToUpperConverter): (TestWebKitAPI::ToUpperConverter::startProducing): (TestWebKitAPI::ToUpperConverter::startConsuming): (TestWebKitAPI::ToUpperConverter::stopProducing): (TestWebKitAPI::ToUpperConverter::stopConsuming): (TestWebKitAPI::ToUpperConverter::enqueueLower): --- Source/WTF/ChangeLog | 15 +++++++++++++++ Source/WTF/wtf/SynchronizedFixedQueue.h | 10 +++++----- Source/WebCore/ChangeLog | 16 ++++++++++++++++ .../platform/graphics/ImageFrameCache.cpp | 6 +++--- .../platform/graphics/ImageFrameCache.h | 4 ++-- Tools/ChangeLog | 17 +++++++++++++++++ .../Tests/WTF/SynchronizedFixedQueue.cpp | 18 ++++++++++-------- 7 files changed, 68 insertions(+), 18 deletions(-) diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog index 2044f5e51d02..8e79d9ef1aa7 100644 --- a/Source/WTF/ChangeLog +++ b/Source/WTF/ChangeLog @@ -1,3 +1,18 @@ +2017-09-26 Said Abou-Hallawa + + Followup (r222427): SynchronizedFixedQueue should not have a public constructor + https://bugs.webkit.org/show_bug.cgi?id=177458 + + Reviewed by Tim Horton. + + Since SynchronizedFixedQueue is now derived from ThreadSafeRefCounted, + the standard way to have an instance of it is to call SynchronizedFixedQueue::create() + which returns a Ref. Now it does not make sense to still + have the constructor public. + + * wtf/SynchronizedFixedQueue.h: + (WTF::SynchronizedFixedQueue::SynchronizedFixedQueue): + 2017-09-23 Said Abou-Hallawa Images may render partial frames even after loading all the encoded data diff --git a/Source/WTF/wtf/SynchronizedFixedQueue.h b/Source/WTF/wtf/SynchronizedFixedQueue.h index 0cf24bb8280f..9bca7145ee9f 100644 --- a/Source/WTF/wtf/SynchronizedFixedQueue.h +++ b/Source/WTF/wtf/SynchronizedFixedQueue.h @@ -40,11 +40,6 @@ class SynchronizedFixedQueue : public ThreadSafeRefCounted + + Followup (r222427): SynchronizedFixedQueue should not have a public constructor + https://bugs.webkit.org/show_bug.cgi?id=177458 + + Reviewed by Tim Horton. + + ImageFrameCache::decodingQueue() and frameRequestQueue() should return + raw references instead of Ref objects. + + * platform/graphics/ImageFrameCache.cpp: + (WebCore::ImageFrameCache::decodingQueue): + (WebCore::ImageFrameCache::frameRequestQueue): + (WebCore::ImageFrameCache::startAsyncDecodingQueue): + * platform/graphics/ImageFrameCache.h: + 2017-09-23 Said Abou-Hallawa Images may render partial frames even after loading all the encoded data diff --git a/Source/WebCore/platform/graphics/ImageFrameCache.cpp b/Source/WebCore/platform/graphics/ImageFrameCache.cpp index 51780819e277..ffa7484f5377 100644 --- a/Source/WebCore/platform/graphics/ImageFrameCache.cpp +++ b/Source/WebCore/platform/graphics/ImageFrameCache.cpp @@ -262,7 +262,7 @@ void ImageFrameCache::cacheNativeImageAtIndexAsync(NativeImagePtr&& nativeImage, m_image->imageFrameAvailableAtIndex(index); } -Ref ImageFrameCache::decodingQueue() +WorkQueue& ImageFrameCache::decodingQueue() { if (!m_decodingQueue) m_decodingQueue = WorkQueue::create("org.webkit.ImageDecoder", WorkQueue::Type::Serial, WorkQueue::QOS::Default); @@ -270,7 +270,7 @@ Ref ImageFrameCache::decodingQueue() return *m_decodingQueue; } -Ref ImageFrameCache::frameRequestQueue() +ImageFrameCache::FrameRequestQueue& ImageFrameCache::frameRequestQueue() { if (!m_frameRequestQueue) m_frameRequestQueue = FrameRequestQueue::create(); @@ -284,7 +284,7 @@ void ImageFrameCache::startAsyncDecodingQueue() return; // We need to protect this, m_decodingQueue and m_decoder from being deleted while we are in the decoding loop. - decodingQueue()->dispatch([protectedThis = makeRef(*this), protectedDecodingQueue = decodingQueue(), protectedFrameRequestQueue = frameRequestQueue(), protectedDecoder = makeRef(*m_decoder), sourceURL = sourceURL().string().isolatedCopy()] { + decodingQueue().dispatch([protectedThis = makeRef(*this), protectedDecodingQueue = makeRef(decodingQueue()), protectedFrameRequestQueue = makeRef(frameRequestQueue()), protectedDecoder = makeRef(*m_decoder), sourceURL = sourceURL().string().isolatedCopy()] { ImageFrameRequest frameRequest; while (protectedFrameRequestQueue->dequeue(frameRequest)) { diff --git a/Source/WebCore/platform/graphics/ImageFrameCache.h b/Source/WebCore/platform/graphics/ImageFrameCache.h index 7b8fa6cbaa85..16e269467e0d 100644 --- a/Source/WebCore/platform/graphics/ImageFrameCache.h +++ b/Source/WebCore/platform/graphics/ImageFrameCache.h @@ -137,8 +137,8 @@ class ImageFrameCache : public ThreadSafeRefCounted { struct ImageFrameRequest; static const int BufferSize = 8; - Ref decodingQueue(); - Ref> frameRequestQueue(); + WorkQueue& decodingQueue(); + SynchronizedFixedQueue& frameRequestQueue(); const ImageFrame& frameAtIndexCacheIfNeeded(size_t, ImageFrame::Caching, const std::optional& = { }); diff --git a/Tools/ChangeLog b/Tools/ChangeLog index b5201f638254..e1585985ffb2 100644 --- a/Tools/ChangeLog +++ b/Tools/ChangeLog @@ -1,3 +1,20 @@ +2017-09-26 Said Abou-Hallawa + + Followup (r222427): SynchronizedFixedQueue should not have a public constructor + https://bugs.webkit.org/show_bug.cgi?id=177458 + + Reviewed by Tim Horton. + + Fix the definition and the creation of SynchronizedFixedQueue. + + * TestWebKitAPI/Tests/WTF/SynchronizedFixedQueue.cpp: + (TestWebKitAPI::ToUpperConverter::ToUpperConverter): + (TestWebKitAPI::ToUpperConverter::startProducing): + (TestWebKitAPI::ToUpperConverter::startConsuming): + (TestWebKitAPI::ToUpperConverter::stopProducing): + (TestWebKitAPI::ToUpperConverter::stopConsuming): + (TestWebKitAPI::ToUpperConverter::enqueueLower): + 2017-09-19 Wenson Hsieh REGRESSION (r215613): Incorrect corners clipping with border-radius diff --git a/Tools/TestWebKitAPI/Tests/WTF/SynchronizedFixedQueue.cpp b/Tools/TestWebKitAPI/Tests/WTF/SynchronizedFixedQueue.cpp index 31436bc22307..0f5f9709e500 100644 --- a/Tools/TestWebKitAPI/Tests/WTF/SynchronizedFixedQueue.cpp +++ b/Tools/TestWebKitAPI/Tests/WTF/SynchronizedFixedQueue.cpp @@ -56,6 +56,8 @@ template class ToUpperConverter { public: ToUpperConverter() + : m_lowerQueue(SynchronizedFixedQueue::create()) + , m_upperQueue(SynchronizedFixedQueue::create()) { } @@ -80,8 +82,8 @@ class ToUpperConverter { produceQueue()->dispatch([this] { CString lower; - while (m_lowerQueue.dequeue(lower)) { - m_upperQueue.enqueue(toUpper(lower)); + while (m_lowerQueue->dequeue(lower)) { + m_upperQueue->enqueue(toUpper(lower)); EXPECT_TRUE(lower == textItem(m_produceCount++)); std::this_thread::sleep_for(std::chrono::milliseconds(10)); } @@ -96,7 +98,7 @@ class ToUpperConverter { consumeQueue()->dispatch([this] { CString upper; - while (m_upperQueue.dequeue(upper)) { + while (m_upperQueue->dequeue(upper)) { EXPECT_TRUE(upper == toUpper(textItem(m_consumeCount++))); std::this_thread::sleep_for(std::chrono::milliseconds(50)); } @@ -115,7 +117,7 @@ class ToUpperConverter { if (!isProducing()) return; - m_lowerQueue.close(); + m_lowerQueue->close(); m_produceCloseSemaphore.wait(WallTime::infinity()); m_produceQueue = nullptr; } @@ -125,7 +127,7 @@ class ToUpperConverter { if (!isConsuming()) return; - m_upperQueue.close(); + m_upperQueue->close(); m_consumeCloseSemaphore.wait(WallTime::infinity()); m_consumeQueue = nullptr; } @@ -138,7 +140,7 @@ class ToUpperConverter { void enqueueLower(const CString& lower) { - m_lowerQueue.enqueue(lower); + m_lowerQueue->enqueue(lower); } bool isProducing() { return m_produceQueue; } @@ -148,8 +150,8 @@ class ToUpperConverter { size_t consumeCount() const { return m_consumeCount; } private: - SynchronizedFixedQueue m_lowerQueue; - SynchronizedFixedQueue m_upperQueue; + Ref> m_lowerQueue; + Ref> m_upperQueue; RefPtr m_produceQueue; RefPtr m_consumeQueue; BinarySemaphore m_produceCloseSemaphore;