[webkit-reviews] review granted: [Bug 30805] Add MessageQueue::removeWithPredicate to remove certain tasks without pulling them from the queue. : [Attachment 42047] Proposed patch.

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has granted Dmitry Titov
<dimich at chromium.org>'s request for review:
Bug 30805: Add MessageQueue::removeWithPredicate to remove certain tasks
without pulling them from the queue.
https://bugs.webkit.org/show_bug.cgi?id=30805

Attachment 42047: Proposed patch.
https://bugs.webkit.org/attachment.cgi?id=42047&action=review

------- Additional Comments from David Levin <levin at chromium.org>
Just a few nits. Please fix and submit.


> diff --git a/JavaScriptCore/wtf/MessageQueue.h
b/JavaScriptCore/wtf/MessageQueue.h
> +	   void removeWithPredicate(Predicate&);

Consider using "removeIf" instead of "removeWithPredicate" (It is similar in
naming to the findIf method on Deque).


> +    inline void MessageQueue<DataType>::removeWithPredicate(Predicate&
predicate)
> +    {
> +	   MutexLocker lock(m_mutex);
> +	   DequeConstIterator<DataType> found = m_queue.end();
> +	   while ((found = m_queue.findIf(predicate)) != m_queue.end()) {
> +	       m_queue.remove(found); ASSERT(0);

Please remove "ASSERT(0);"



> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> +	   (WebCore::SameDatabasePredicate::SameDatabasePredicate): Added
predicate that flags the tasks belonging to specified database.

Consider: to "a" specified database.

> +	   (WebCore::DatabaseThread::unscheduleDatabaseTasks): changed to use
new removeWithPredicate method.

Consider: use "the" new removeWithPredicate