Bug 138018 – [WK2] Use C++ lambdas in IPC::Connection

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Zan Dobersek]

[WK2] Remove uses of WTF::bind() in IPC::Connection

[Zan Dobersek]

Created attachment 240365 [details]
Patch

[Zan Dobersek]

Created attachment 240621 [details]
Patch

[Zan Dobersek]

Comment on attachment 240621 [details]
Patch

Clearing flags on attachment: 240621

Committed r175806: <http://trac.webkit.org/changeset/175806>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.

[Antti Koivisto]

Comment on attachment 240621 [details]
Patch

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

> Source/WebKit2/Platform/IPC/Connection.cpp:647
> +        CString messageReceiverName = message->messageReceiverName().isEmpty() ? "<unknown message>" : message->messageReceiverName().toString();
> +        CString messageName = message->messageName().isEmpty() ? String::format("<message length: %zu bytes>", message->length()).utf8() : message->messageReceiverName().toString();
> +
> +        RefPtr<Connection> protectedThis(this);
> +        m_clientRunLoop.dispatch([protectedThis, messageReceiverName, messageName] {

Is m_clientRunLoop the current run loop? If not the capture of messageReceiverName and messageName does not look thread safe. You could use StringCapture instead.

> Source/WebKit2/Platform/IPC/mac/ConnectionMac.mm:516
> +            CString messageReceiverNameString = decoder->messageReceiverName().toString();
> +            CString messageNameString = decoder->messageName().toString();
> +            m_clientRunLoop.dispatch([protectedThis, messageReceiverNameString, messageNameString] {
> +                protectedThis->dispatchDidReceiveInvalidMessage(messageReceiverNameString, messageNameString);
> +            });

Here too.

[Zan Dobersek]

Comment on attachment 240621 [details]
Patch

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

>> Source/WebKit2/Platform/IPC/Connection.cpp:647
>> +        m_clientRunLoop.dispatch([protectedThis, messageReceiverName, messageName] {
> 
> Is m_clientRunLoop the current run loop? If not the capture of messageReceiverName and messageName does not look thread safe. You could use StringCapture instead.

Can we avoid formatting the "<message length ...>" string and use something static instead? That way we wouldn't have to handle CStrings at all, and could capture StringReference objects instead.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 138666

[Zan Dobersek]

Created attachment 241571 [details]
Patch

[Zan Dobersek]

Created attachment 244888 [details]
Patch

[Anders Carlsson]

Comment on attachment 244888 [details]
Patch

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

> Source/WebKit2/Platform/IPC/Connection.cpp:646
> +        m_clientRunLoop.dispatch([protectedThis, messageReceiverName, messageName] {
> +            protectedThis->dispatchDidReceiveInvalidMessage(messageReceiverName, messageName);
> +        });

messageReceiverName and messageName could point to into a MessageDecoder that has been freed here.

[Zan Dobersek]

Created attachment 248196 [details]
Patch

[Zan Dobersek]

Comment on attachment 248196 [details]
Patch

Doesn't yet build.

[Zan Dobersek]

Created attachment 248241 [details]
Patch

[Zan Dobersek]

Created attachment 248321 [details]
Patch

[Zan Dobersek]

Created attachment 248326 [details]
Patch

[Zan Dobersek]

Comment on attachment 248326 [details]
Patch

This finally compiles everywhere, so I'm setting the r? flag.

[Anders Carlsson]

Comment on attachment 248326 [details]
Patch

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

> Source/WebKit2/Platform/IPC/Connection.cpp:348
>      // Reset the client.

Can remove this comment.

[Zan Dobersek]

Committed r181630: <http://trac.webkit.org/changeset/181630>