Bug 138018 - [WK2] Use C++ lambdas in IPC::Connection
Summary: [WK2] Use C++ lambdas in IPC::Connection
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zan Dobersek
URL:
Keywords:
Depends on: 138666
Blocks:
  Show dependency treegraph
 
Reported: 2014-10-23 13:31 PDT by Zan Dobersek
Modified: 2015-03-17 03:05 PDT (History)
2 users (show)

See Also:


Attachments
Patch (10.15 KB, patch)
2014-10-23 13:37 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff
Patch (10.39 KB, patch)
2014-10-29 14:13 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff
Patch (11.98 KB, patch)
2014-11-14 03:07 PST, Zan Dobersek
no flags Details | Formatted Diff | Diff
Patch (9.88 KB, patch)
2015-01-19 02:13 PST, Zan Dobersek
no flags Details | Formatted Diff | Diff
Patch (10.27 KB, patch)
2015-03-08 12:49 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff
Patch (10.97 KB, patch)
2015-03-09 04:29 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff
Patch (10.55 KB, patch)
2015-03-10 02:39 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff
Patch (10.86 KB, patch)
2015-03-10 04:16 PDT, Zan Dobersek
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2014-10-23 13:31:04 PDT
[WK2] Remove uses of WTF::bind() in IPC::Connection
Comment 1 Zan Dobersek 2014-10-23 13:37:30 PDT
Created attachment 240365 [details]
Patch
Comment 2 Zan Dobersek 2014-10-29 14:13:49 PDT
Created attachment 240621 [details]
Patch
Comment 3 Zan Dobersek 2014-11-10 04:29:59 PST
Comment on attachment 240621 [details]
Patch

Clearing flags on attachment: 240621

Committed r175806: <http://trac.webkit.org/changeset/175806>
Comment 4 Zan Dobersek 2014-11-10 04:30:07 PST
All reviewed patches have been landed.  Closing bug.
Comment 5 Antti Koivisto 2014-11-10 07:46:03 PST
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.
Comment 6 Zan Dobersek 2014-11-11 04:01:18 PST
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.
Comment 7 WebKit Commit Bot 2014-11-12 13:45:55 PST
Re-opened since this is blocked by bug 138666
Comment 8 Zan Dobersek 2014-11-14 03:07:24 PST
Created attachment 241571 [details]
Patch
Comment 9 Zan Dobersek 2015-01-19 02:13:22 PST
Created attachment 244888 [details]
Patch
Comment 10 Anders Carlsson 2015-01-19 15:32:05 PST
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.
Comment 11 Zan Dobersek 2015-03-08 12:49:11 PDT
Created attachment 248196 [details]
Patch
Comment 12 Zan Dobersek 2015-03-09 03:58:52 PDT
Comment on attachment 248196 [details]
Patch

Doesn't yet build.
Comment 13 Zan Dobersek 2015-03-09 04:29:54 PDT
Created attachment 248241 [details]
Patch
Comment 14 Zan Dobersek 2015-03-10 02:39:33 PDT
Created attachment 248321 [details]
Patch
Comment 15 Zan Dobersek 2015-03-10 04:16:04 PDT
Created attachment 248326 [details]
Patch
Comment 16 Zan Dobersek 2015-03-10 12:34:18 PDT
Comment on attachment 248326 [details]
Patch

This finally compiles everywhere, so I'm setting the r? flag.
Comment 17 Anders Carlsson 2015-03-13 08:25:23 PDT
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.
Comment 18 Zan Dobersek 2015-03-17 03:05:10 PDT
Committed r181630: <http://trac.webkit.org/changeset/181630>