[webkit-reviews] review granted: [Bug 231970] RemoteRenderingBackend::CreateImageBuffer should be an async IPC stream message : [Attachment 441970] Alternate approach, #2

[bugzilla-daemon at webkit.org]

Kimmo Kinnunen <kkinnunen at apple.com> has granted  review:
Bug 231970: RemoteRenderingBackend::CreateImageBuffer should be an async IPC
stream message
https://bugs.webkit.org/show_bug.cgi?id=231970

Attachment 441970: Alternate approach, #2

https://bugs.webkit.org/attachment.cgi?id=441970&action=review




--- Comment #11 from Kimmo Kinnunen <kkinnunen at apple.com> ---
Comment on attachment 441970
  --> https://bugs.webkit.org/attachment.cgi?id=441970
Alternate approach, #2

View in context: https://bugs.webkit.org/attachment.cgi?id=441970&action=review

Nice!

> Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp:95
> +   
m_streamConnection->startReceivingMessages(Messages::RemoteDisplayListRecorder:
:messageReceiverName());

So I remember this after two weeks, could you add a comment about the
subtleness?
I can come up with something like below, but feel free to word it differently
too.
// RemoteDisplayListRecorder messages depend on RemoteRenderingBackend, because
RemoteRenderingBackend creates RemoteDisplayListRecorder and
// makes a receive queue for it. In order to guarantee correct ordering, ensure
that all RemoteDisplayListRecorder messages are processed in the same sequence
// as RemoteRenderingBackend.

> Source/WebKit/Platform/IPC/StreamServerConnection.cpp:43
> +    // FIXME: Can we avoid synchronous dispatch here by adjusting the
assertion in `Connection::enqueueMatchingMessagesToMessageReceiveQueue`?

Yeah, this should be done, along with fixing m_workQueue.addStreamConnection to
be thread-safe.