Bug 227740 - Allow custom IPC::Attachment messaging in ConnectionUnix.cpp
Summary: Allow custom IPC::Attachment messaging in ConnectionUnix.cpp
Alias: None
Product: WebKit
Classification: Unclassified
Component: WPE WebKit (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Imanol Fernandez
Depends on:
Reported: 2021-07-07 03:07 PDT by Imanol Fernandez
Modified: 2022-05-05 02:57 PDT (History)
5 users (show)

See Also:

Patch (1.26 KB, patch)
2021-07-07 03:11 PDT, Imanol Fernandez
no flags Details | Formatted Diff | Diff
Patch (5.93 KB, patch)
2021-08-03 09:49 PDT, Imanol Fernandez
no flags Details | Formatted Diff | Diff
Patch (5.95 KB, patch)
2021-08-06 08:08 PDT, Imanol Fernandez
no flags Details | Formatted Diff | Diff
Patch for landing (5.96 KB, patch)
2021-08-09 02:24 PDT, Imanol Fernandez
no flags Details | Formatted Diff | Diff
Patch for landing (5.96 KB, patch)
2021-08-09 02:25 PDT, Imanol Fernandez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Imanol Fernandez 2021-07-07 03:07:59 PDT
We need to pass AHardwareBuffer instance from the UIProcess to the WebProcess, which requires calling functions such as AHardwareBuffer_sendHandleToUnixSocket(). By adding custom IPC::Attachmentmessaging we can call the AHardwareBuffer helper functions to pass and get AHardwareBuffers via IPC
Comment 1 Imanol Fernandez 2021-07-07 03:11:54 PDT
Created attachment 433021 [details]
Comment 2 Michael Catanzaro 2021-07-07 18:04:28 PDT
Comment on attachment 433021 [details]

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

> Source/WebKit/Platform/IPC/unix/ConnectionUnix.cpp:59
> -#endif // SOCK_SEQPACKET
> +#endif // SOCK_SEQPACKET 
> -namespace IPC {
> +namespace IPC { 

I assume there was some accident with this patch? :)
Comment 3 Imanol Fernandez 2021-07-12 07:23:10 PDT
yes, it was just a compilation test to discard what it looked like a intermittent compilation error bug 226919

I'll upload the real patch soon :)
Comment 4 Imanol Fernandez 2021-08-03 09:49:35 PDT
Created attachment 434836 [details]

Implement CustomWriterType Attachment
Comment 5 Imanol Fernandez 2021-08-03 09:54:58 PDT
This is an example of how the API can be used to send aHardwareBuffer via IPC:

void ArgumentCoder<PlatformXR::AHardwareBufferWrapper>::encode(Encoder& encoder, const PlatformXR::AHardwareBufferWrapper& wrapper)
    auto hardwarebuffer = wrapper.hardwarebuffer;
    Attachment attachment(Attachment::CustomWriter { [hardwarebuffer](int socketDescriptor) {
        while (true) {
            int ret = AHardwareBuffer_sendHandleToUnixSocket(hardwarebuffer, socketDescriptor);
            if (!ret || ret != -EAGAIN)
    encoder << attachment;

bool ArgumentCoder<PlatformXR::AHardwareBufferWrapper>::decode(Decoder& decoder, PlatformXR::AHardwareBufferWrapper& wrapper)
    Attachment attachment;
    if (!decoder.decode(attachment))
        return false;
    auto socketDescriptor = WTF::get<Attachment::SocketDescriptor>(attachment.customWriter());
    while (true) {
        auto ret = AHardwareBuffer_recvHandleFromUnixSocket(socketDescriptor, &wrapper.hardwarebuffer);
        if (!ret || ret != -EAGAIN)

    return true;
Comment 6 Imanol Fernandez 2021-08-06 08:08:30 PDT
Created attachment 435068 [details]

Use WTF::function instead of std::function
Comment 7 Zan Dobersek (Reviews) 2021-08-06 08:40:26 PDT
Comment on attachment 435068 [details]

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

> Source/WebKit/Platform/IPC/unix/ConnectionUnix.cpp:593
> +        for (size_t i = 0; i < attachments.size(); ++i) {

for (auto& attachment : attachments)
Comment 8 Imanol Fernandez 2021-08-09 02:24:00 PDT
Created attachment 435176 [details]
Patch for landing
Comment 9 Imanol Fernandez 2021-08-09 02:25:35 PDT
Created attachment 435177 [details]
Patch for landing
Comment 10 EWS 2021-08-09 02:58:30 PDT
Committed r280769 (240353@main): <https://commits.webkit.org/240353@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 435177 [details].
Comment 11 Kimmo Kinnunen 2022-05-05 02:57:39 PDT
Did the client for this ever land?
I think this is a fair deviation of the intended purpose of Attachment, and maybe forms undue cognitive overhead while maintenance.

Hanging arbitrary function closure to the IPC message attachment is cumbersome as it's unclear what is the contract to call the function. What thread is it ok to call? Is it ok to call many times? What's the lifetime of the closed variables?

In the example in the comments, there's a potential to try to access a already freed ANB. Message send can be delayed, and as such it's not guaranteed that the ANB pointer closed by the function is alive at the time of actual message send.

Some time in the future the non-Cocoa and Cocoa parts could/should/must share more of the implementation. In these cases, it's not ok to run arbitrary code, as the send may happen in other thread.

The intended use of IPC::Attachment is that all objects that cannot be transferred as pure data are stored in IPC::Attachment.
In this case, it'd be better to store an owning reference to a ANB in the Attachment, and then explicitly say
 "Connection knows how to transfer UnixFileDescriptor" (e.g this is already there)
 "Connection knows how to transfer ANB" (e.g. you make ConnectionUnix.cpp call AHardwareBuffer_sendHandleToUnixSocket et al)

So Unix variant of Connection would have 
class Attachment {

        , Ref<AHardwareBuffer>

(Ref<AHardwareBuffer> uses AHardwareBuffer_acquire, AHardwareBuffer_release to implement ref(), unref())