[Webkit-unassigned] [Bug 30612] Remove threadsafe refcounting from tasks used with WTF::MessageQueue.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=30612


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #42245|review?                     |review+
               Flag|                            |




--- Comment #4 from David Levin <levin at chromium.org>  2009-11-02 08:31:43 PDT ---
(From update of attachment 42245)
Just a few things to fix on landing.

> diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
> +        * wtf/MessageQueue.h:
> +        (WTF::MessageQueue::alwaysTruePredicate):
> +        (WTF::::~MessageQueue):
> +        (WTF::::append):
> +        (WTF::::appendAndCheckEmpty):
> +        (WTF::::prepend):
> +        (WTF::::waitForMessage):
> +        (WTF::::waitForMessageFilteredWithTimeout):
> +        (WTF::::tryGetMessage):
> +        (WTF::::removeIf):
These functions declarations are messed up. Please fix them.


> diff --git a/JavaScriptCore/wtf/MessageQueue.h b/JavaScriptCore/wtf/MessageQueue.h
> +    inline void MessageQueue<DataType>::~MessageQueue()
> +    {
> +        MutexLocker lock(m_mutex);
> +
> +        while (!m_queue.isEmpty()) {
> +            DequeConstIterator<DataType*> found = m_queue.begin();
> +            DataType* ptr = *found;
> +            m_queue.remove(found);
> +            delete ptr;
> +        }
> +    }
Consider using "deleteAllValues" from Deque.h

> +
> +
Extra blank line.


> +    inline PassOwnPtr<DataType> MessageQueue<DataType>::waitForMessageFilteredWithTimeout(MessageQueueWaitResult& result, Predicate& predicate, double absoluteTime)
> +        DataType* ptr = *found;
ptr? (name) How about message or data.... ?


> +    inline PassOwnPtr<DataType> MessageQueue<DataType>::tryGetMessage()
> +        DataType* ptr = m_queue.first();
ptr? (name)


> @@ -157,9 +181,11 @@ namespace WTF {
>      inline void MessageQueue<DataType>::removeIf(Predicate& predicate)
> +            DataType* ptr = *found;

ptr? (name)

> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
>  2009-10-30  Dmitry Titov  <dimich at chromium.org>
>  
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Remove threadsafe refcounting from tasks used with WTF::MessageQueue.
> +        https://bugs.webkit.org/show_bug.cgi?id=30612
> +
> +        No new tests since no new functionality. Storage, MessagePorts and Workers tests cover this.
> +
> +        There is a lot of files but most changes are simply replace RefPtr and PassRefPtr to

/There is/There are/
/to/with/


> diff --git a/WebCore/dom/default/PlatformMessagePortChannel.cpp b/WebCore/dom/default/PlatformMessagePortChannel.cpp
> @@ -193,7 +193,7 @@ void PlatformMessagePortChannel::postMessageToRemote(PassOwnPtr<MessagePortChann
>  bool PlatformMessagePortChannel::tryGetMessageFromRemote(OwnPtr<MessagePortChannel::EventData>& result)
>  {
>      MutexLocker lock(m_mutex);
> +    return (result = m_incomingQueue->tryGetMessage());

It would be nice to not write this on one line, which can easily making it look
like == was intended at first glance.
  result = m_incomingQueue->tryGetMessage();
  return result;


> diff --git a/WebCore/storage/LocalStorageThread.cpp b/WebCore/storage/LocalStorageThread.cpp
>      while (true) {
> -        RefPtr<LocalStorageTask> task;
> -        if (!m_queue.waitForMessage(task))
> +        OwnPtr<LocalStorageTask> task = m_queue.waitForMessage();
> +        if (!task)
>              break;

For similar code in WebCore/storage/DatabaseThread.cpp, you re-wrote this to be
  while (OwnPtr<LocalStorageTask> task = m_queue.waitForMessage()) {

(and if you do that I think the contents will be a single line so no {} around
that.)


> diff --git a/WebCore/workers/WorkerRunLoop.cpp b/WebCore/workers/WorkerRunLoop.cpp
> +void WorkerRunLoop::Task::performTask(ScriptExecutionContext* context) { m_task->performTask(context); }

Should be multiple lines.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.