Bug 58030 – [WK2][Qt][GTK] Introduce common use flag for the shared UNIX domain socket IPC implementation

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Balazs Kelemen]

A common flag would be more readable then using "PLATFORM(QT) || PLATFORM(GTK)" everywhere in the IPC code.
Furthermore it would be useful if a new platform would like to use the infrastructure.

[Benjamin Poulain]

I like the idea. The keyword "Unix" is not gonna do the trick since Mac OS X is also Unix. :(
So maybe a keyword by the type of IPC used?

[Kenneth Rohde Christiansen]

Mac is Unix, and doesn't this work for Mac as well? Mac is just a specialization of Unix which has another more platform close implementation

[Zoltan Horvath]

Does it make sense to use OS(LINUX)?

[Balazs Kelemen]

(In reply to comment #3)
> Does it make sense to use OS(LINUX)?
It's too restrictive. I think it works on most of the OS-s that is in the group of OS(UNIX) (in Platform.h) - at least on Linux+*BSD+Mac.

[Balazs Kelemen]

Created attachment 88613 [details]
Patch

Note: the define has been placed in Platform.h instead of WebKit2/config.h because in the final build step the include order is broken and we pick up WebCore's config.h even for the WebKit2 API files. Annoying bug but I do not see any solution for that.

[Kenneth Rohde Christiansen]

Comment on attachment 88613 [details]
Patch

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

> Source/JavaScriptCore/wtf/Platform.h:1199
> +// This one is for WebKit2.

Strange comment; I would just remove it

[Andras Becsi]

(In reply to comment #5)
> Note: the define has been placed in Platform.h instead of WebKit2/config.h because in the final build step the include order is broken and we pick up WebCore's config.h even for the WebKit2 API files. Annoying bug but I do not see any solution for that.

One solution would be to change the config.h includes to WebKit2/config.h and WebCore/config.h respectively.

[Balazs Kelemen]

(In reply to comment #7)
> (In reply to comment #5)
> > Note: the define has been placed in Platform.h instead of WebKit2/config.h because in the final build step the include order is broken and we pick up WebCore's config.h even for the WebKit2 API files. Annoying bug but I do not see any solution for that.
> 
> One solution would be to change the config.h includes to WebKit2/config.h and WebCore/config.h respectively.

Or rename each config.h to represent the component that it belongs to. (i.e. WebCoreConfig.h, WebKit2Config.h, ...). The problem is that it would require a huge change and furthermore I guess not everybody would like that style.

[Balazs Kelemen]

(In reply to comment #6)
> (From update of attachment 88613 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=88613&action=review
> 
> > Source/JavaScriptCore/wtf/Platform.h:1199
> > +// This one is for WebKit2.
> 
> Strange comment; I would just remove it

I will do it on commit. However I forget to set the dependency. We need to wait for the GTK SharedMemory patch to be landed first.

[Martin Robinson]

Comment on attachment 88613 [details]
Patch

Thanks for cleaning this up!

[Balazs Kelemen]

Committed r83307: <http://trac.webkit.org/changeset/83307>

[WebKit Review Bot]

http://trac.webkit.org/changeset/83307 might have broken Qt Linux Release
The following tests are not passing:
editing/selection/5354455-1.html
editing/selection/5354455-2.html
editing/selection/button-right-click.html
editing/selection/context-menu-on-text.html
editing/selection/context-menu-text-selection.html
editing/selection/empty-cell-right-click.html
fast/events/context-onmousedown-event.html
fast/events/contextmenu-scrolled-page-with-frame.html
fast/events/right-click-focus.html
svg/custom/use-events-crash.svg