[webkit-changes] [WebKit/WebKit] cae00d: Replies from GPUConnection::Connection::sendWithAs...

[Jean-Yves Avenard]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: cae00d824a496148f16e28b0bb56bd39cb61e00a
      https://github.com/WebKit/WebKit/commit/cae00d824a496148f16e28b0bb56bd39cb61e00a
  Author: Jean-Yves Avenard <jya at apple.com>
  Date:   2024-02-29 (Thu, 29 Feb 2024)

  Changed paths:
    M Source/WebKit/Platform/IPC/Connection.cpp
    M Source/WebKit/Platform/IPC/Connection.h
    M Tools/TestWebKitAPI/Tests/IPC/ConnectionTests.cpp

  Log Message:
  -----------
  Replies from GPUConnection::Connection::sendWithAsyncReply will always be called on the main thread
https://bugs.webkit.org/show_bug.cgi?id=270118
rdar://123650202

Reviewed by Kimmo Kinnunen.

A Connection::sendWithAsyncReply was only ever designed to call the AsyncReplyHandler
used to return the reply on the Connection's dispatcher. Which in the case
of the GPUConnection's Connection is the main thread/runloop.
While the Connection's itself uses a dedicated WorkQueue to send and receive
IPC messages, and that a WorkQueueMessageReceiver let you decide on which
WorkQueue the messages should be dispatched on; there was no ability to
specify on which dispatcher (or WorkQueueMessageReceiver) the sendWithAsyncReply's
replies will be delivered on.

While Connection::sendWithPromisedReply returned a thread-safe NativePromise
which allow to define on which dispatcher's the callback should be run on,
it also used sendWithAsyncReply and used the AsyncReplyHandler to wrap
a NativePromiseProducer which would then be resolved or rejected on the
Connection's dispatcher.

This became problematic in situation where a WorkQueueMessageReceiver was
used for performance reasons; the bottleneck would still be on the main
thread's usage state. It made the current MSE in a Worker implementation's
moot.

We add two new methods: sendWithAsyncReplyOnDispatcher and sendWithPromisedReplyOnDispatcher
Which let you defines the RefCountedSerialFunctionDispatcher on which the AsyncReply
will be delivered on.

The semantics for sendWithPromisedReplyOnDispatcher is rather cumbersome as you
need to set the dispatcher twice, but at this stage there’s no possibility
to retrieve the WorkQueue set with whenSettled/then as at the time the NativePromiseProducer
Has been settled, no listener may have been attached yet. This will be fixed in a follow
up change.

Rather than expanding the existing ConnectionAsyncReplyHandler object with a dispatcher member
and using a single HashMap of AsyncReplyHandler which would have allowed less duplication
in the code, we create a new
Connection::AsyncReplyHandlerWithDispatcher and have a separate map. The reasons are two fold:
1- We don’t have to handle cases where a dispatcher would have been set for a AsyncReplyHandler
that should be run in the Connection’s dispatcher, or vice-versa: a dispatcher wasn’t set
when it should have been.
2- The vast majority of the usage is using the older sendWithAsyncReply so we don’t have to
Search in an overall larget HashMap twice.

So we favour code readability over size.

Added API tests to check several scenarios including:
- That replies are delivered on the correct dispatcher and in the right order
- The replies of messages sent to an invalidated Connection are properly delivered on
the dispatcher.

* Source/WebKit/Platform/IPC/Connection.cpp:
(IPC::Connection::sendMessageWithAsyncReplyWithDispatcher):
(IPC::Connection::processIncomingMessage):
(IPC::Connection::addAsyncReplyHandlerWithDispatcher):
(IPC::Connection::cancelAsyncReplyHandlers):
(IPC::Connection::isAsyncReplyHandlerWithDispatcher):
(IPC::Connection::takeAsyncReplyHandlerWithDispatcher):
(IPC::Connection::takeAsyncReplyHandlerWithDispatcherWithLockHeld):
* Source/WebKit/Platform/IPC/Connection.h:
(IPC::Connection::sendWithAsyncReply):
(IPC::Connection::sendWithAsyncReplyOnDispatcher):
(IPC::Connection::sendWithPromisedReplyOnDispatcher):
(IPC::Connection::AsyncReplyHandlerWithDispatcher::operator bool const):
(IPC::Connection::sendWithPromisedReply):
(IPC::Connection::waitForAsyncReplyAndDispatchImmediately):
(IPC::CompletionHandler<void):
(IPC::Connection::makeAsyncReplyHandler):
(IPC::Connection::makeAsyncReplyHandlerWithDispatcher):
* Tools/TestWebKitAPI/Tests/IPC/ConnectionTests.cpp:
(TestWebKitAPI::AutoWorkQueue::WorkQueueWithShutdown::create):
(TestWebKitAPI::AutoWorkQueue::WorkQueueWithShutdown::beginShutdown):
(TestWebKitAPI::AutoWorkQueue::WorkQueueWithShutdown::waitUntilShutdown):
(TestWebKitAPI::AutoWorkQueue::WorkQueueWithShutdown::WorkQueueWithShutdown):
(TestWebKitAPI::AutoWorkQueue::AutoWorkQueue):
(TestWebKitAPI::AutoWorkQueue::queue):
(TestWebKitAPI::AutoWorkQueue::~AutoWorkQueue):
(TestWebKitAPI::TEST_P):

Canonical link: https://commits.webkit.org/275494@main



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications