[webkit-reviews] review granted: [Bug 59954] Need a way to handle CoreIPC messages on the connection work queue : [Attachment 91957] Patch

[bugzilla-daemon at webkit.org]

Adam Roben (:aroben) <aroben at apple.com> has granted Anders Carlsson
<andersca at apple.com>'s request for review:
Bug 59954: Need a way to handle CoreIPC messages on the connection work queue
https://bugs.webkit.org/show_bug.cgi?id=59954

Attachment 91957: Patch
https://bugs.webkit.org/attachment.cgi?id=91957&action=review

------- Additional Comments from Adam Roben (:aroben) <aroben at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=91957&action=review

> Source/WebKit2/ChangeLog:10
> +	   member function, willProcessMessageOnClientRunLoop which should
return true if the message
> +	   was processed and false otherwise. Users of Connection can use
addQueueClient to add clients

I'd say "should return true if the message should be forwarded to the
Connection::Client's run loop for processing and false otherwise". (Note that
this reverses the meaning of the return value.) That seems slightly closer in
spirit to Cocoa delegate methods that start with "will".

> Source/WebKit2/Platform/CoreIPC/Connection.cpp:242
> +void Connection::addQueueClient(QueueClient* queueClient)
> +{
> +    MutexLocker locker(m_connectionQueueClientsMutex);
> +    m_connectionQueueClients.append(queueClient);
> +}

Should we assert that the client isn't already in m_connectionQueueClients?

> Source/WebKit2/Platform/CoreIPC/Connection.h:108
> +    protected:
> +	   virtual ~QueueClient() { }
> +
> +    public:
> +	   virtual bool willProcessMessageOnClientRunLoop(Connection*,
MessageID, ArgumentDecoder*) = 0;

Maybe reverse these sections?