[webkit-reviews] review granted: [Bug 106708] [WK2] Make it possible to send sync messages from secondary threads : [Attachment 182434] proposed patch

[bugzilla-daemon at webkit.org]

Anders Carlsson <andersca at apple.com> has granted Alexey Proskuryakov
<ap at webkit.org>'s request for review:
Bug 106708: [WK2] Make it possible to send sync messages from secondary threads
https://bugs.webkit.org/show_bug.cgi?id=106708

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

------- Additional Comments from Anders Carlsson <andersca at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=182434&action=review


> Source/WebKit2/Platform/CoreIPC/Connection.cpp:98
> +    // The reply decoder, will be null if there was an error processing the
sync
> +    // message on the other side.

No need to break this comment.

> Source/WebKit2/Platform/CoreIPC/Connection.cpp:435
> +	   didFailToSendSyncMessage();

I think's weird that didFailToSendSyncMessage can now be called from any
thread. I think just returning null is better.

> Source/WebKit2/Platform/CoreIPC/Connection.cpp:469
> +    if (!pendingReply.replyDecoder)
> +	   didFailToSendSyncMessage();

Ditto.

> Source/WebKit2/Platform/CoreIPC/Connection.cpp:557
> +    // If it's not a reply to any primary thread message, check if it is a
reply to a secondary thread one.
> +    SecondaryThreadPendingSyncReplyMap::iterator secondaryThreadReplyMapItem
= m_secondaryThreadPendingSyncReplyMap.find(decoder->destinationID());
> +    if (secondaryThreadReplyMapItem !=
m_secondaryThreadPendingSyncReplyMap.end()) {
> +	   SecondaryThreadPendingSyncReply* reply =
secondaryThreadReplyMapItem->value;
> +	   ASSERT(!reply->replyDecoder);
> +	   reply->replyDecoder = decoder.leakPtr();
> +	   reply->semaphore.signal();
> +    }

I think this needs to be protected by the mutex m_syncReplyState mutex.