[WK2][Unix][Refactoring] Get rid of socket creation code duplicates

• Component: Platform
• Status: RESOLVED DUPLICATE

[Balazs Kelemen]

The socket creation code is duplicated in a lot places, let's get rid of that.

[Thiago Marcos P. Santos]

(In reply to comment #0)
> The socket creation code is duplicated in a lot places, let's get rid of that.

What socket creation code are you referring to here?

[Balazs Kelemen]

Created attachment 186838 [details]
Patch

[Early Warning System Bot]

Comment on attachment 186838 [details]
Patch

Attachment 186838 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/16389253

[Balazs Kelemen]

Created attachment 186849 [details]
Patch

[Balazs Kelemen]

Anybody? Although we have owners I think it's better if somebody how is a maintainer of an affected port would do an informal review first.

[Martin Robinson]

Carlos, the one who would know the most about this code, is on vacation until Monday. Do you think you could get a review from him then?

[Balazs Kelemen]

(In reply to comment #6)
> Carlos, the one who would know the most about this code, is on vacation until Monday. Do you think you could get a review from him then?

Sure, that's fine.

[Carlos Garcia Campos]

Comment on attachment 186849 [details]
Patch

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

This is a great cleanup, thanks!. Every time I see that code duplicated I think we should refactor it. I have a couple of questions, though.

> Source/WebKit2/Platform/CoreIPC/unix/ConnectionUnix.cpp:142
> +        while (fcntl(sockets[0], F_SETFD, FD_CLOEXEC) == -1) {

I guess this should be sockets[i].

> Source/WebKit2/PluginProcess/PluginProcess.cpp:162
> -    RefPtr<WebProcessConnection> connection = WebProcessConnection::create(sockets[1]);
> +    RefPtr<WebProcessConnection> connection = WebProcessConnection::create(serverIdentifier);

Is there any reason why you are doing the opposite of what the previous code did? serverIdentifier is sockets[0], it doesn't really matter, since both sockets are indistinguishable, but I'm curious.

> Source/WebKit2/PluginProcess/PluginProcess.cpp:165
> -    CoreIPC::Attachment clientSocket(sockets[0]);
> +    CoreIPC::Attachment clientSocket(clientIdentifier);

Ditto.

[Balazs Kelemen]

(In reply to comment #8)
> (From update of attachment 186849 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=186849&action=review
> 
> This is a great cleanup, thanks!. Every time I see that code duplicated I think we should refactor it. I have a couple of questions, though.
> 
> > Source/WebKit2/Platform/CoreIPC/unix/ConnectionUnix.cpp:142
> > +        while (fcntl(sockets[0], F_SETFD, FD_CLOEXEC) == -1) {
> 
> I guess this should be sockets[i].

Oops. Sure, will fix.

> 
> > Source/WebKit2/PluginProcess/PluginProcess.cpp:162
> > -    RefPtr<WebProcessConnection> connection = WebProcessConnection::create(sockets[1]);
> > +    RefPtr<WebProcessConnection> connection = WebProcessConnection::create(serverIdentifier);
> 
> Is there any reason why you are doing the opposite of what the previous code did? serverIdentifier is sockets[0], it doesn't really matter, since both sockets are indistinguishable, but I'm curious.

No reason, just I was finding it more straightforward to assign the first one to the first argument.

[Balazs Kelemen]

Created attachment 187444 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 187444 [details]
Patch

LGTM, thanks! We need an owner to r+ this, though.

[Brady Eidson]

Anders should look at this one.

[Balazs Kelemen]

As requested in https://bugs.webkit.org/show_bug.cgi?id=110093#c6 I am going to unify these ipc stuff across every port.

*** This bug has been marked as a duplicate of bug 110978 ***