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 <sabouhallawa@apple.com> 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<SynchronizedFixedQueue>, the standard way to have an instance of it is to call SynchronizedFixedQueue::create() which returns a Ref<SynchronizedFixedQueue>. 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):
WebKit · Oct 16, 2017 · 7874348 · 7874348
1 parent ec125d1
commit 7874348
Show file tree

Hide file tree

Showing 7 changed files with 68 additions and 18 deletions.
diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
@@ -1,3 +1,18 @@
+2017-09-26  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        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<SynchronizedFixedQueue>,
+        the standard way to have an instance of it is to call SynchronizedFixedQueue::create()
+        which returns a Ref<SynchronizedFixedQueue>. Now it does not make sense to still
+        have the constructor public.
+
+        * wtf/SynchronizedFixedQueue.h:
+        (WTF::SynchronizedFixedQueue::SynchronizedFixedQueue):
+
 2017-09-23  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         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
@@ -40,11 +40,6 @@ class SynchronizedFixedQueue : public ThreadSafeRefCounted<SynchronizedFixedQueu
         return adoptRef(*new SynchronizedFixedQueue());
     }
 
-    SynchronizedFixedQueue()
-    {
-        static_assert(!((BufferSize - 1) & BufferSize), "BufferSize must be power of 2.");
-    }
-
     void open()
     {
         LockHolder lockHolder(m_mutex);
@@ -113,6 +108,11 @@ class SynchronizedFixedQueue : public ThreadSafeRefCounted<SynchronizedFixedQueu
     }
 
 private:
+    SynchronizedFixedQueue()
+    {
+        static_assert(!((BufferSize - 1) & BufferSize), "BufferSize must be power of 2.");
+    }
+
     Lock m_mutex;
     Condition m_condition;
 

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,19 @@
+2017-09-26  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        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  <sabouhallawa@apple.com>
 
         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
@@ -262,15 +262,15 @@ void ImageFrameCache::cacheNativeImageAtIndexAsync(NativeImagePtr&& nativeImage,
         m_image->imageFrameAvailableAtIndex(index);
 }
 
-Ref<WorkQueue> ImageFrameCache::decodingQueue()
+WorkQueue& ImageFrameCache::decodingQueue()
 {
     if (!m_decodingQueue)
         m_decodingQueue = WorkQueue::create("org.webkit.ImageDecoder", WorkQueue::Type::Serial, WorkQueue::QOS::Default);
 
     return *m_decodingQueue;
 }
 
-Ref<ImageFrameCache::FrameRequestQueue> 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
@@ -137,8 +137,8 @@ class ImageFrameCache : public ThreadSafeRefCounted<ImageFrameCache> {
 
     struct ImageFrameRequest;
     static const int BufferSize = 8;
-    Ref<WorkQueue> decodingQueue();
-    Ref<SynchronizedFixedQueue<ImageFrameRequest, BufferSize>> frameRequestQueue();
+    WorkQueue& decodingQueue();
+    SynchronizedFixedQueue<ImageFrameRequest, BufferSize>& frameRequestQueue();
 
     const ImageFrame& frameAtIndexCacheIfNeeded(size_t, ImageFrame::Caching, const std::optional<SubsamplingLevel>& = { });
 

diff --git a/Tools/ChangeLog b/Tools/ChangeLog
@@ -1,3 +1,20 @@
+2017-09-26  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        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  <wenson_hsieh@apple.com>
 
         REGRESSION (r215613): Incorrect corners clipping with border-radius

diff --git a/Tools/TestWebKitAPI/Tests/WTF/SynchronizedFixedQueue.cpp b/Tools/TestWebKitAPI/Tests/WTF/SynchronizedFixedQueue.cpp
@@ -56,6 +56,8 @@ template <size_t BufferSize>
 class ToUpperConverter {
 public:
     ToUpperConverter()
+        : m_lowerQueue(SynchronizedFixedQueue<CString, BufferSize>::create())
+        , m_upperQueue(SynchronizedFixedQueue<CString, BufferSize>::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<CString, BufferSize> m_lowerQueue;
-    SynchronizedFixedQueue<CString, BufferSize> m_upperQueue;
+    Ref<SynchronizedFixedQueue<CString, BufferSize>> m_lowerQueue;
+    Ref<SynchronizedFixedQueue<CString, BufferSize>> m_upperQueue;
     RefPtr<WorkQueue> m_produceQueue;
     RefPtr<WorkQueue> m_consumeQueue;
     BinarySemaphore m_produceCloseSemaphore;