Attachment 343324 Details for Bug 186920 – Patch

[patch] Patch

bug-186920-20180622234034.patch (text/plain), 17.09 KB, created by Yusuke Suzuki on 2018-06-22 07:40:35 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2018-06-22 07:40:35 PDT

Size: 17.09 KB

patch

obsolete

>Subversion Revision: 233077
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 4415c67a0b3a371531377df0356c475557772811..58671492a918e85586e70d375c3256b959c5d222 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,25 @@
>+2018-06-22  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        [WTF] Use Ref<> for the result type of non-failing factory functions
>+        https://bugs.webkit.org/show_bug.cgi?id=186920
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * dfg/DFGWorklist.cpp:
>+        (JSC::DFG::Worklist::ThreadBody::ThreadBody):
>+        (JSC::DFG::Worklist::finishCreation):
>+        * dfg/DFGWorklist.h:
>+        * heap/Heap.cpp:
>+        (JSC::Heap::Thread::Thread):
>+        * heap/Heap.h:
>+        * jit/JITWorklist.cpp:
>+        (JSC::JITWorklist::Thread::Thread):
>+        * jit/JITWorklist.h:
>+        * runtime/VMTraps.cpp:
>+        * runtime/VMTraps.h:
>+        * wasm/WasmWorklist.cpp:
>+        * wasm/WasmWorklist.h:
>+
> 2018-06-22  Carlos Garcia Campos  <cgarcia@igalia.com>
> 
>         [GTK] WebDriver: use a dictionary for session capabilities in StartAutomationSession message
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index 42022b3c7f11cac66dac1d1766527bc2b986d184..3535552dc1b32092228b69e12bfea8684da3e236 100644
>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
>@@ -1,3 +1,26 @@
>+2018-06-22  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        [WTF] Use Ref<> for the result type of non-failing factory functions
>+        https://bugs.webkit.org/show_bug.cgi?id=186920
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Use Ref<> instead of RefPtr<> if the `create` function do not return nullptr.
>+
>+        * wtf/AutomaticThread.cpp:
>+        (WTF::AutomaticThreadCondition::create):
>+        (WTF::AutomaticThread::AutomaticThread):
>+        * wtf/AutomaticThread.h:
>+        * wtf/ParallelHelperPool.cpp:
>+        (WTF::ParallelHelperPool::Thread::Thread):
>+        * wtf/ParallelHelperPool.h:
>+        * wtf/WorkerPool.cpp:
>+        (WTF::WorkerPool::WorkerPool):
>+        * wtf/WorkerPool.h:
>+        * wtf/win/WorkQueueWin.cpp:
>+        (WTF::TimerContext::create):
>+        (WTF::WorkQueue::dispatchAfter):
>+
> 2018-06-21  Carlos Garcia Campos  <cgarcia@igalia.com>
> 
>         [GLIB] improve get_type() fast path in WEBKIT_DEFINE_TYPE
>diff --git a/Source/JavaScriptCore/dfg/DFGWorklist.cpp b/Source/JavaScriptCore/dfg/DFGWorklist.cpp
>index 5bc88c4050aba9c6701d566e6d78e0849ea193fc..7536d0ce2088335ebf7caae1910369c7003a018b 100644
>--- a/Source/JavaScriptCore/dfg/DFGWorklist.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGWorklist.cpp
>@@ -39,8 +39,8 @@ namespace JSC { namespace DFG {
> 
> class Worklist::ThreadBody : public AutomaticThread {
> public:
>-    ThreadBody(const AbstractLocker& locker, Worklist& worklist, ThreadData& data, Box<Lock> lock, RefPtr<AutomaticThreadCondition> condition, int relativePriority)
>-        : AutomaticThread(locker, lock, condition)
>+    ThreadBody(const AbstractLocker& locker, Worklist& worklist, ThreadData& data, Box<Lock> lock, Ref<AutomaticThreadCondition>&& condition, int relativePriority)
>+        : AutomaticThread(locker, lock, WTFMove(condition))
>         , m_worklist(worklist)
>         , m_data(data)
>         , m_relativePriority(relativePriority)
>@@ -201,7 +201,7 @@ void Worklist::finishCreation(unsigned numberOfThreads, int relativePriority)
>     LockHolder locker(*m_lock);
>     for (unsigned i = numberOfThreads; i--;) {
>         std::unique_ptr<ThreadData> data = std::make_unique<ThreadData>(this);
>-        data->m_thread = adoptRef(new ThreadBody(locker, *this, *data, m_lock, m_planEnqueued, relativePriority));
>+        data->m_thread = adoptRef(new ThreadBody(locker, *this, *data, m_lock, m_planEnqueued.copyRef(), relativePriority));
>         m_threads.append(WTFMove(data));
>     }
> }
>diff --git a/Source/JavaScriptCore/dfg/DFGWorklist.h b/Source/JavaScriptCore/dfg/DFGWorklist.h
>index 41205594bd69c0e623ca28c914b3cf1bd3b3b631..7b681d1e5432cefa50831b647fd4ffd2b72eff8f 100644
>--- a/Source/JavaScriptCore/dfg/DFGWorklist.h
>+++ b/Source/JavaScriptCore/dfg/DFGWorklist.h
>@@ -113,7 +113,7 @@ class Worklist : public RefCounted<Worklist> {
>     Lock m_suspensionLock;
>     
>     Box<Lock> m_lock;
>-    RefPtr<AutomaticThreadCondition> m_planEnqueued;
>+    Ref<AutomaticThreadCondition> m_planEnqueued;
>     Condition m_planCompiled;
>     
>     Vector<std::unique_ptr<ThreadData>> m_threads;
>diff --git a/Source/JavaScriptCore/heap/Heap.cpp b/Source/JavaScriptCore/heap/Heap.cpp
>index 1a35a7548146a893da89add611ec9b56a70cbefd..57fbed3de24f081313e12851bfa4ea704a190bbe 100644
>--- a/Source/JavaScriptCore/heap/Heap.cpp
>+++ b/Source/JavaScriptCore/heap/Heap.cpp
>@@ -234,7 +234,7 @@ class TimingScope {
> class Heap::Thread : public AutomaticThread {
> public:
>     Thread(const AbstractLocker& locker, Heap& heap)
>-        : AutomaticThread(locker, heap.m_threadLock, heap.m_threadCondition)
>+        : AutomaticThread(locker, heap.m_threadLock, heap.m_threadCondition.copyRef())
>         , m_heap(heap)
>     {
>     }
>diff --git a/Source/JavaScriptCore/heap/Heap.h b/Source/JavaScriptCore/heap/Heap.h
>index ecc86b1ef07b9a5294daccb26830bc89d47ee949..020226bb742350016d40fe21383984cb863d41cb 100644
>--- a/Source/JavaScriptCore/heap/Heap.h
>+++ b/Source/JavaScriptCore/heap/Heap.h
>@@ -701,7 +701,7 @@ class Heap {
>     uint64_t m_mutatorExecutionVersion { 0 };
>     uint64_t m_phaseVersion { 0 };
>     Box<Lock> m_threadLock;
>-    RefPtr<AutomaticThreadCondition> m_threadCondition; // The mutator must not wait on this. It would cause a deadlock.
>+    Ref<AutomaticThreadCondition> m_threadCondition; // The mutator must not wait on this. It would cause a deadlock.
>     RefPtr<AutomaticThread> m_thread;
> 
> #if PLATFORM(IOS)
>diff --git a/Source/JavaScriptCore/jit/JITWorklist.cpp b/Source/JavaScriptCore/jit/JITWorklist.cpp
>index 0b32e1f7938c6c6afc76c7f1998c0666c35fd3bf..982102fbcaa985452e3f6fd5fb5b41c70ddb3760 100644
>--- a/Source/JavaScriptCore/jit/JITWorklist.cpp
>+++ b/Source/JavaScriptCore/jit/JITWorklist.cpp
>@@ -100,7 +100,7 @@ class JITWorklist::Plan : public ThreadSafeRefCounted<JITWorklist::Plan> {
> class JITWorklist::Thread : public AutomaticThread {
> public:
>     Thread(const AbstractLocker& locker, JITWorklist& worklist)
>-        : AutomaticThread(locker, worklist.m_lock, worklist.m_condition)
>+        : AutomaticThread(locker, worklist.m_lock, worklist.m_condition.copyRef())
>         , m_worklist(worklist)
>     {
>         m_worklist.m_numAvailableThreads++;
>diff --git a/Source/JavaScriptCore/jit/JITWorklist.h b/Source/JavaScriptCore/jit/JITWorklist.h
>index 7ea1c1e91a607c375b7e3385fc9eabf41db869b3..adf9fb9bb3fec53d61c7f8c1c8699e98d715e3c3 100644
>--- a/Source/JavaScriptCore/jit/JITWorklist.h
>+++ b/Source/JavaScriptCore/jit/JITWorklist.h
>@@ -72,7 +72,7 @@ class JITWorklist {
>     HashSet<CodeBlock*> m_planned;
>     
>     Box<Lock> m_lock;
>-    RefPtr<AutomaticThreadCondition> m_condition; // We use One True Condition for everything because that's easier.
>+    Ref<AutomaticThreadCondition> m_condition; // We use One True Condition for everything because that's easier.
>     RefPtr<AutomaticThread> m_thread;
>     
>     unsigned m_numAvailableThreads { 0 };
>diff --git a/Source/JavaScriptCore/runtime/VMTraps.cpp b/Source/JavaScriptCore/runtime/VMTraps.cpp
>index 6c14c0a92df0cd3a63ae091955742fcfbda25d7d..c26341946c448b2c1c31bf2876d3d47a579b2a90 100644
>--- a/Source/JavaScriptCore/runtime/VMTraps.cpp
>+++ b/Source/JavaScriptCore/runtime/VMTraps.cpp
>@@ -180,7 +180,7 @@ class VMTraps::SignalSender final : public AutomaticThread {
> public:
>     using Base = AutomaticThread;
>     SignalSender(const AbstractLocker& locker, VM& vm)
>-        : Base(locker, vm.traps().m_lock, vm.traps().m_trapSet)
>+        : Base(locker, vm.traps().m_lock, vm.traps().m_trapSet.copyRef())
>         , m_vm(vm)
>     {
>         static std::once_flag once;
>diff --git a/Source/JavaScriptCore/runtime/VMTraps.h b/Source/JavaScriptCore/runtime/VMTraps.h
>index 9b06cdd12db59e2a07445381279bb74f1362c047..9beef99ede9912850982a2df22b11c6c4539e5c7 100644
>--- a/Source/JavaScriptCore/runtime/VMTraps.h
>+++ b/Source/JavaScriptCore/runtime/VMTraps.h
>@@ -146,7 +146,7 @@ class VMTraps {
> #endif
> 
>     Box<Lock> m_lock;
>-    RefPtr<AutomaticThreadCondition> m_trapSet;
>+    Ref<AutomaticThreadCondition> m_trapSet;
>     union {
>         BitField m_needTrapHandling { 0 };
>         BitField m_trapsBitField;
>diff --git a/Source/JavaScriptCore/wasm/WasmWorklist.cpp b/Source/JavaScriptCore/wasm/WasmWorklist.cpp
>index 6b32533160440cbed13a224bd8bf8bea8b997e6e..51e7840d4d8c3d962f373712cda0a3cdbe6964d2 100644
>--- a/Source/JavaScriptCore/wasm/WasmWorklist.cpp
>+++ b/Source/JavaScriptCore/wasm/WasmWorklist.cpp
>@@ -58,7 +58,7 @@ class Worklist::Thread final : public AutomaticThread {
> public:
>     using Base = AutomaticThread;
>     Thread(const AbstractLocker& locker, Worklist& work)
>-        : Base(locker, work.m_lock, work.m_planEnqueued)
>+        : Base(locker, work.m_lock, work.m_planEnqueued.copyRef())
>         , worklist(work)
>     {
> 
>diff --git a/Source/JavaScriptCore/wasm/WasmWorklist.h b/Source/JavaScriptCore/wasm/WasmWorklist.h
>index 748a9ec42d93d2bfa4d1249e707aceb0989c1409..8edf249cf64ac8e42aebe066795e5653ea4ebcea 100644
>--- a/Source/JavaScriptCore/wasm/WasmWorklist.h
>+++ b/Source/JavaScriptCore/wasm/WasmWorklist.h
>@@ -82,7 +82,7 @@ class Worklist {
>     }
> 
>     Box<Lock> m_lock;
>-    RefPtr<AutomaticThreadCondition> m_planEnqueued;
>+    Ref<AutomaticThreadCondition> m_planEnqueued;
>     // Technically, this could overflow but that's unlikely. Even if it did, we will just compile things of the same
>     // Priority it the wrong order, which isn't wrong, just suboptimal.
>     Ticket m_lastGrantedTicket { 0 };
>diff --git a/Source/WTF/wtf/AutomaticThread.cpp b/Source/WTF/wtf/AutomaticThread.cpp
>index f1156ac14a47980b6945d1b7f8351a09227480ce..a2541c0300eb06fe5c9d4040e96125e66ebbb850 100644
>--- a/Source/WTF/wtf/AutomaticThread.cpp
>+++ b/Source/WTF/wtf/AutomaticThread.cpp
>@@ -33,9 +33,9 @@ namespace WTF {
> 
> static const bool verbose = false;
> 
>-RefPtr<AutomaticThreadCondition> AutomaticThreadCondition::create()
>+Ref<AutomaticThreadCondition> AutomaticThreadCondition::create()
> {
>-    return adoptRef(new AutomaticThreadCondition());
>+    return adoptRef(*new AutomaticThreadCondition());
> }
> 
> AutomaticThreadCondition::AutomaticThreadCondition()
>@@ -104,9 +104,9 @@ bool AutomaticThreadCondition::contains(const AbstractLocker&, AutomaticThread*
>     return m_threads.contains(thread);
> }
> 
>-AutomaticThread::AutomaticThread(const AbstractLocker& locker, Box<Lock> lock, RefPtr<AutomaticThreadCondition> condition, Seconds timeout)
>+AutomaticThread::AutomaticThread(const AbstractLocker& locker, Box<Lock> lock, Ref<AutomaticThreadCondition>&& condition, Seconds timeout)
>     : m_lock(lock)
>-    , m_condition(condition)
>+    , m_condition(WTFMove(condition))
>     , m_timeout(timeout)
> {
>     if (verbose)
>diff --git a/Source/WTF/wtf/AutomaticThread.h b/Source/WTF/wtf/AutomaticThread.h
>index 387fd8f7871a8d01ce16b82efbfdfc2bef14d1c0..a314628bd5b6e981cc5db7f39b99e66384f0d131 100644
>--- a/Source/WTF/wtf/AutomaticThread.h
>+++ b/Source/WTF/wtf/AutomaticThread.h
>@@ -69,7 +69,7 @@ class AutomaticThread;
> 
> class AutomaticThreadCondition : public ThreadSafeRefCounted<AutomaticThreadCondition> {
> public:
>-    static WTF_EXPORT_PRIVATE RefPtr<AutomaticThreadCondition> create();
>+    static WTF_EXPORT_PRIVATE Ref<AutomaticThreadCondition> create();
>     
>     WTF_EXPORT_PRIVATE ~AutomaticThreadCondition();
>     
>@@ -131,7 +131,7 @@ class WTF_EXPORT_PRIVATE AutomaticThread : public ThreadSafeRefCounted<Automatic
> protected:
>     // This logically creates the thread, but in reality the thread won't be created until someone
>     // calls AutomaticThreadCondition::notifyOne() or notifyAll().
>-    AutomaticThread(const AbstractLocker&, Box<Lock>, RefPtr<AutomaticThreadCondition>, Seconds timeout = 10_s);
>+    AutomaticThread(const AbstractLocker&, Box<Lock>, Ref<AutomaticThreadCondition>&&, Seconds timeout = 10_s);
>     
>     // To understand PollResult and WorkResult, imagine that poll() and work() are being called like
>     // so:
>@@ -182,7 +182,7 @@ class WTF_EXPORT_PRIVATE AutomaticThread : public ThreadSafeRefCounted<Automatic
>     void start(const AbstractLocker&);
>     
>     Box<Lock> m_lock;
>-    RefPtr<AutomaticThreadCondition> m_condition;
>+    Ref<AutomaticThreadCondition> m_condition;
>     Seconds m_timeout;
>     bool m_isRunning { true };
>     bool m_isWaiting { false };
>diff --git a/Source/WTF/wtf/ParallelHelperPool.cpp b/Source/WTF/wtf/ParallelHelperPool.cpp
>index 8d0f9e4b310084be841b3cf5d4b79f021ca30bdc..48a7bc70233f5187cadc3e70d342d25b2f24bf7b 100644
>--- a/Source/WTF/wtf/ParallelHelperPool.cpp
>+++ b/Source/WTF/wtf/ParallelHelperPool.cpp
>@@ -171,7 +171,7 @@ void ParallelHelperPool::doSomeHelping()
> class ParallelHelperPool::Thread : public AutomaticThread {
> public:
>     Thread(const AbstractLocker& locker, ParallelHelperPool& pool)
>-        : AutomaticThread(locker, pool.m_lock, pool.m_workAvailableCondition)
>+        : AutomaticThread(locker, pool.m_lock, pool.m_workAvailableCondition.copyRef())
>         , m_pool(pool)
>     {
>     }
>diff --git a/Source/WTF/wtf/ParallelHelperPool.h b/Source/WTF/wtf/ParallelHelperPool.h
>index 21859cb2b9b087022448186ef9bc39106be90fc0..734cdf476c8874fd3a8d980230f07fa5d1d9835a 100644
>--- a/Source/WTF/wtf/ParallelHelperPool.h
>+++ b/Source/WTF/wtf/ParallelHelperPool.h
>@@ -200,7 +200,7 @@ class ParallelHelperPool : public ThreadSafeRefCounted<ParallelHelperPool> {
>     ParallelHelperClient* waitForClientWithTask(const AbstractLocker&);
>     
>     Box<Lock> m_lock; // AutomaticThread wants this in a box for safety.
>-    RefPtr<AutomaticThreadCondition> m_workAvailableCondition;
>+    Ref<AutomaticThreadCondition> m_workAvailableCondition;
>     Condition m_workCompleteCondition;
> 
>     WeakRandom m_random;
>diff --git a/Source/WTF/wtf/WorkerPool.cpp b/Source/WTF/wtf/WorkerPool.cpp
>index bdbdfc5db7efa8de65cf834184305ca524655c40..484b57c2988f87db6d7ab9ba04051b22a66e86d9 100644
>--- a/Source/WTF/wtf/WorkerPool.cpp
>+++ b/Source/WTF/wtf/WorkerPool.cpp
>@@ -34,8 +34,8 @@ class WorkerPool::Worker final : public AutomaticThread {
> public:
>     friend class WorkerPool;
> 
>-    Worker(const AbstractLocker& locker, WorkerPool& pool, Box<Lock> lock, RefPtr<AutomaticThreadCondition> condition, Seconds timeout)
>-        : AutomaticThread(locker, lock, condition, timeout)
>+    Worker(const AbstractLocker& locker, WorkerPool& pool, Box<Lock> lock, Ref<AutomaticThreadCondition>&& condition, Seconds timeout)
>+        : AutomaticThread(locker, lock, WTFMove(condition), timeout)
>         , m_pool(pool)
>     {
>     }
>@@ -91,7 +91,7 @@ WorkerPool::WorkerPool(ASCIILiteral name, unsigned numberOfWorkers, Seconds time
> {
>     LockHolder locker(*m_lock);
>     for (unsigned i = 0; i < numberOfWorkers; ++i)
>-        m_workers.append(adoptRef(*new Worker(locker, *this, m_lock, m_condition, timeout)));
>+        m_workers.append(adoptRef(*new Worker(locker, *this, m_lock, m_condition.copyRef(), timeout)));
> }
> 
> WorkerPool::~WorkerPool()
>diff --git a/Source/WTF/wtf/WorkerPool.h b/Source/WTF/wtf/WorkerPool.h
>index 7676ac9e8dd1ff01d2d3082ac43db979320661b5..6235e731ed75b80125095359068b08d4c1dcfec2 100644
>--- a/Source/WTF/wtf/WorkerPool.h
>+++ b/Source/WTF/wtf/WorkerPool.h
>@@ -58,7 +58,7 @@ class WorkerPool : public ThreadSafeRefCounted<WorkerPool> {
>     bool shouldSleep(const AbstractLocker&);
> 
>     Box<Lock> m_lock;
>-    RefPtr<AutomaticThreadCondition> m_condition;
>+    Ref<AutomaticThreadCondition> m_condition;
>     Seconds m_timeout;
>     MonotonicTime m_lastTimeoutTime { MonotonicTime::nan() };
>     unsigned m_numberOfActiveWorkers { 0 };
>diff --git a/Source/WTF/wtf/win/WorkQueueWin.cpp b/Source/WTF/wtf/win/WorkQueueWin.cpp
>index 78230c0c1b1aa5d3ff53b259e96576bd76b2db35..365f188ec14bf0522fff54c55d12fc7d78fb78ec 100644
>--- a/Source/WTF/wtf/win/WorkQueueWin.cpp
>+++ b/Source/WTF/wtf/win/WorkQueueWin.cpp
>@@ -116,7 +116,7 @@ void WorkQueue::dispatch(Function<void()>&& function)
> }
> 
> struct TimerContext : public ThreadSafeRefCounted<TimerContext> {
>-    static RefPtr<TimerContext> create() { return adoptRef(new TimerContext); }
>+    static Ref<TimerContext> create() { return adoptRef(*new TimerContext); }
> 
>     Lock timerLock;
>     WorkQueue* queue { nullptr };
>@@ -151,7 +151,7 @@ void WorkQueue::dispatchAfter(Seconds duration, Function<void()>&& function)
>     ASSERT(m_timerQueue);
>     ref();
> 
>-    RefPtr<TimerContext> context = TimerContext::create();
>+    Ref<TimerContext> context = TimerContext::create();
>     context->queue = this;
>     context->function = WTFMove(function);
> 
>@@ -177,7 +177,7 @@ void WorkQueue::dispatchAfter(Seconds duration, Function<void()>&& function)
> 
>         // Since our timer callback is quick, we can execute in the timer thread itself and avoid
>         // an extra thread switch over to a worker thread.
>-        if (!::CreateTimerQueueTimer(&context->timer, m_timerQueue, timerCallback, context.get(), clampTo<DWORD>(milliseconds), 0, WT_EXECUTEINTIMERTHREAD)) {
>+        if (!::CreateTimerQueueTimer(&context->timer, m_timerQueue, timerCallback, context.ptr(), clampTo<DWORD>(milliseconds), 0, WT_EXECUTEINTIMERTHREAD)) {
>             ASSERT_WITH_MESSAGE(false, "::CreateTimerQueueTimer failed with error %lu", ::GetLastError());
>             return;
>         }

Flags:

darin: review+
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 186920: 343324 | 343427