JavaScriptCore:
authorlevin@chromium.org <levin@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 23 Feb 2009 22:37:17 +0000 (22:37 +0000)
committerlevin@chromium.org <levin@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 23 Feb 2009 22:37:17 +0000 (22:37 +0000)
2009-02-23  David Levin  <levin@chromium.org>

        Reviewed by Alexey Proskuryakov.

        Bug 24047: Need to simplify nested if's in WorkerRunLoop::runInMode
        <https://bugs.webkit.org/show_bug.cgi?id=24047>

        * wtf/MessageQueue.h:
        (WTF::MessageQueue::infiniteTime):
        Allows for one to call waitForMessageFilteredWithTimeout and wait forever.

        (WTF::MessageQueue::alwaysTruePredicate):
        (WTF::MessageQueue::waitForMessage):
        Made waitForMessage call waitForMessageFilteredWithTimeout, so that there is less
        duplicate code.

        (WTF::MessageQueue::waitForMessageFilteredWithTimeout):

        * wtf/ThreadingQt.cpp:
        (WTF::ThreadCondition::timedWait):
        * wtf/ThreadingWin.cpp:
        (WTF::ThreadCondition::timedWait):
        Made these two implementations consistent with the pthread and gtk implementations.
        Currently, the time calculations would overflow when passed large values.

WebCore:

2009-02-23  David Levin  <levin@chromium.org>

        Reviewed by Alexey Proskuryakov.

        Bug 24047: Need to simplify nested if's in WorkerRunLoop::runInMode
        <https://bugs.webkit.org/show_bug.cgi?id=24047>

        Made a nested if inside of WorkerRunLoop::runInMode a lot simpler by
        using only MessageQueue::waitForMessageFilteredWithTimeout instead
        of three different MessageQueue methods.

        No observable change in behavior, so no test.

        * dom/WorkerRunLoop.cpp:
        (WebCore::ModePredicate::operator()):
        Minor clean-up to able to pass a const ref point for ModePredicate into runInMode.
        (WebCore::WorkerRunLoop::runInMode):
        * dom/WorkerRunLoop.h:

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

JavaScriptCore/ChangeLog
JavaScriptCore/wtf/MessageQueue.h
JavaScriptCore/wtf/ThreadingQt.cpp
JavaScriptCore/wtf/ThreadingWin.cpp
WebCore/ChangeLog
WebCore/dom/WorkerRunLoop.cpp
WebCore/dom/WorkerRunLoop.h

index 9053eb3..d8485f4 100644 (file)
@@ -1,3 +1,28 @@
+2009-02-23  David Levin  <levin@chromium.org>
+
+        Reviewed by Alexey Proskuryakov.
+
+        Bug 24047: Need to simplify nested if's in WorkerRunLoop::runInMode
+        <https://bugs.webkit.org/show_bug.cgi?id=24047>
+
+        * wtf/MessageQueue.h:
+        (WTF::MessageQueue::infiniteTime):
+        Allows for one to call waitForMessageFilteredWithTimeout and wait forever.
+
+        (WTF::MessageQueue::alwaysTruePredicate):
+        (WTF::MessageQueue::waitForMessage):
+        Made waitForMessage call waitForMessageFilteredWithTimeout, so that there is less
+        duplicate code.
+
+        (WTF::MessageQueue::waitForMessageFilteredWithTimeout):
+
+        * wtf/ThreadingQt.cpp:
+        (WTF::ThreadCondition::timedWait):
+        * wtf/ThreadingWin.cpp:
+        (WTF::ThreadCondition::timedWait):
+        Made these two implementations consistent with the pthread and gtk implementations.
+        Currently, the time calculations would overflow when passed large values.
+
 2009-02-23  Jeremy Moskovich  <jeremy@chromium.org>
 
         Reviewed by Adam Roben.
index b6699f8..9549f37 100644 (file)
@@ -30,6 +30,7 @@
 #ifndef MessageQueue_h
 #define MessageQueue_h
 
+#include <limits>
 #include <wtf/Assertions.h>
 #include <wtf/Deque.h>
 #include <wtf/Noncopyable.h>
@@ -52,8 +53,7 @@ namespace WTF {
         void prepend(const DataType&);
         bool waitForMessage(DataType&);
         template<typename Predicate>
-        MessageQueueWaitResult waitForMessageFiltered(DataType&, Predicate&);
-        MessageQueueWaitResult waitForMessageTimed(DataType&, double absoluteTime);
+        MessageQueueWaitResult waitForMessageFilteredWithTimeout(DataType&, Predicate&, double absoluteTime);
         void kill();
 
         bool tryGetMessage(DataType&);
@@ -62,7 +62,11 @@ namespace WTF {
         // The result of isEmpty() is only valid if no other thread is manipulating the queue at the same time.
         bool isEmpty();
 
+        static double infiniteTime() { return std::numeric_limits<double>::max(); }
+
     private:
+        static bool alwaysTruePredicate(DataType&) { return true; }
+
         mutable Mutex m_mutex;
         ThreadCondition m_condition;
         Deque<DataType> m_queue;
@@ -88,57 +92,33 @@ namespace WTF {
     template<typename DataType>
     inline bool MessageQueue<DataType>::waitForMessage(DataType& result)
     {
-        MutexLocker lock(m_mutex);
-
-        while (!m_killed && m_queue.isEmpty())
-            m_condition.wait(m_mutex);
-
-        if (m_killed)
-            return false;
-
-        ASSERT(!m_queue.isEmpty());
-        result = m_queue.first();
-        m_queue.removeFirst();
-        return true;
+        MessageQueueWaitResult exitReason = waitForMessageFilteredWithTimeout(result, MessageQueue<DataType>::alwaysTruePredicate, infiniteTime());
+        ASSERT(exitReason == MessageQueueTerminated || exitReason == MessageQueueMessageReceived);
+        return exitReason == MessageQueueMessageReceived;
     }
 
     template<typename DataType>
     template<typename Predicate>
-    inline MessageQueueWaitResult MessageQueue<DataType>::waitForMessageFiltered(DataType& result, Predicate& predicate)
-    {
-        MutexLocker lock(m_mutex);
-
-        DequeConstIterator<DataType> found = m_queue.end();
-        while (!m_killed && (found = m_queue.findIf(predicate)) == m_queue.end())
-            m_condition.wait(m_mutex);
-
-        if (m_killed)
-            return MessageQueueTerminated;
-
-        ASSERT(found != m_queue.end());
-        result = *found;
-        m_queue.remove(found);
-        return MessageQueueMessageReceived;
-    }
-
-    template<typename DataType>
-    inline MessageQueueWaitResult MessageQueue<DataType>::waitForMessageTimed(DataType& result, double absoluteTime)
+    inline MessageQueueWaitResult MessageQueue<DataType>::waitForMessageFilteredWithTimeout(DataType& result, Predicate& predicate, double absoluteTime)
     {
         MutexLocker lock(m_mutex);
         bool timedOut = false;
 
-        while (!m_killed && !timedOut && m_queue.isEmpty())
+        DequeConstIterator<DataType> found = m_queue.end();
+        while (!m_killed && !timedOut && (found = m_queue.findIf(predicate)) == m_queue.end())
             timedOut = !m_condition.timedWait(m_mutex, absoluteTime);
 
+        ASSERT(!timedOut || absoluteTime != infiniteTime());
+
         if (m_killed)
             return MessageQueueTerminated;
 
         if (timedOut)
             return MessageQueueTimeout;
 
-        ASSERT(!m_queue.isEmpty());
-        result = m_queue.first();
-        m_queue.removeFirst();
+        ASSERT(found != m_queue.end());
+        result = *found;
+        m_queue.remove(found);
         return MessageQueueMessageReceived;
     }
 
index 341e4b7..df30f04 100644 (file)
@@ -242,11 +242,13 @@ bool ThreadCondition::timedWait(Mutex& mutex, double absoluteTime)
     if (absoluteTime < currentTime)
         return false;
 
-    double intervalMilliseconds = (absoluteTime - currentTime) * 1000.0;
-    // Qt defines wait for up to ULONG_MAX milliseconds.
-    if (intervalMilliseconds >= ULONG_MAX)
-        intervalMilliseconds = ULONG_MAX;
+    // Time is too far in the future (and would overflow unsigned long) - wait forever.
+    if (absoluteTime - currentTime > static_cast<double>(INT_MAX) / 1000.0) {
+        wait(mutex);
+        return true;
+    }
 
+    double intervalMilliseconds = (absoluteTime - currentTime) * 1000.0;
     return m_condition->wait(mutex.impl(), static_cast<unsigned long>(intervalMilliseconds));
 }
 
index 399fb38..99f9e1c 100644 (file)
@@ -457,10 +457,13 @@ bool ThreadCondition::timedWait(Mutex& mutex, double absoluteTime)
     if (absoluteTime < currentTime)
         return false;
 
-    double intervalMilliseconds = (absoluteTime - currentTime) * 1000.0;
-    if (intervalMilliseconds >= INT_MAX)
-        intervalMilliseconds = INT_MAX;
+    // Time is too far in the future (and would overflow unsigned long) - wait forever.
+    if (absoluteTime - currentTime > static_cast<double>(INT_MAX) / 1000.0) {
+        wait(mutex);
+        return true;
+    }
 
+    double intervalMilliseconds = (absoluteTime - currentTime) * 1000.0;
     return m_condition.timedWait(mutex.impl(), static_cast<unsigned long>(intervalMilliseconds));
 }
 
index b0efa37..4c1e1d4 100644 (file)
@@ -1,3 +1,22 @@
+2009-02-23  David Levin  <levin@chromium.org>
+
+        Reviewed by Alexey Proskuryakov.
+
+        Bug 24047: Need to simplify nested if's in WorkerRunLoop::runInMode
+        <https://bugs.webkit.org/show_bug.cgi?id=24047>
+
+        Made a nested if inside of WorkerRunLoop::runInMode a lot simpler by
+        using only MessageQueue::waitForMessageFilteredWithTimeout instead
+        of three different MessageQueue methods.
+
+        No observable change in behavior, so no test.
+
+        * dom/WorkerRunLoop.cpp:
+        (WebCore::ModePredicate::operator()):
+        Minor clean-up to able to pass a const ref point for ModePredicate into runInMode.
+        (WebCore::WorkerRunLoop::runInMode):
+        * dom/WorkerRunLoop.h:
+
 2009-02-23  David Hyatt  <hyatt@apple.com>
 
         In preparation for making layers for multicol objects (so that they can properly split child layers
index 63e3d10..65877f7 100644 (file)
@@ -98,7 +98,7 @@ public:
         return m_defaultMode;
     }
 
-    bool operator()(PassRefPtr<WorkerRunLoop::Task> task)
+    bool operator()(PassRefPtr<WorkerRunLoop::Task> task) const
     {
         return m_defaultMode || m_mode == task->mode();
     }
@@ -163,21 +163,15 @@ MessageQueueWaitResult WorkerRunLoop::runInMode(WorkerContext* context, const St
     return result;
 }
 
-MessageQueueWaitResult WorkerRunLoop::runInMode(WorkerContext* context, ModePredicate& predicate)
+MessageQueueWaitResult WorkerRunLoop::runInMode(WorkerContext* context, const ModePredicate& predicate)
 {
     ASSERT(context);
     ASSERT(context->thread());
     ASSERT(context->thread()->threadID() == currentThread());
 
+    double absoluteTime = (predicate.isDefaultMode() && m_sharedTimer->isActive()) ? m_sharedTimer->fireTime() : MessageQueue<RefPtr<Task> >::infiniteTime();
     RefPtr<Task> task;
-    MessageQueueWaitResult result;
-    if (predicate.isDefaultMode()) {
-        if (m_sharedTimer->isActive())
-            result = m_messageQueue.waitForMessageTimed(task, m_sharedTimer->fireTime());
-        else
-            result = m_messageQueue.waitForMessage(task) ? MessageQueueMessageReceived : MessageQueueTerminated;
-    } else
-        result = m_messageQueue.waitForMessageFiltered(task, predicate);
+    MessageQueueWaitResult result = m_messageQueue.waitForMessageFilteredWithTimeout(task, predicate, absoluteTime);
 
     switch (result) {
     case MessageQueueTerminated:
index 9ae9fd4..4d1eaa3 100644 (file)
@@ -65,7 +65,7 @@ namespace WebCore {
         class Task;
     private:
         friend class RunLoopSetup;
-        MessageQueueWaitResult runInMode(WorkerContext*, ModePredicate&);
+        MessageQueueWaitResult runInMode(WorkerContext*, const ModePredicate&);
 
         MessageQueue<RefPtr<Task> > m_messageQueue;
         OwnPtr<WorkerSharedTimer> m_sharedTimer;