[Webkit-unassigned] [Bug 137684] Patch to build a wayland-only webkit/EFL(Without X11/Ecore_X)

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=137684





--- Comment #4 from Ryuan Choi <ryuan.choi at gmail.com>  2014-10-14 17:35:52 PST ---
(From update of attachment 239789)
View in context: https://bugs.webkit.org/attachment.cgi?id=239789&action=review

First, thanks for the patch.
Patch looks almost fine to me.

I didn't try to prepare wayland backend yet.
I will check it more when I have it.

> ewebkitn/Source/cmake/OptionsEfl.cmake:128
> +option(ENABLE_ECORE_X "Enable Ecore_X specific usage (cursor, bell)")
> +
> +option(ENABLE_WAYLAND_ONLY "Enable wayland only build.")
> +if (ENABLE_WAYLAND_ONLY)
> +    set(ENABLE_X11_TARGET OFF)
> +    set(ENABLE_ECORE_X OFF)
> +    set(ENABLE_NETSCAPE_PLUGIN_API OFF)
> +else ()
> +    set(ENABLE_ECORE_X ON)
> +endif ()

In my opinion,
We'd better to choose window system between X or WAYLAND.

Perhaps, like below. (I am bad with naming)
set(WINDOW_SYSTEM "X" CACHE STRING "Choose window system for EWebKit (X or WAYLAND)")
...

In addition, I have one question.
Doesn't we need to use ECORE_WAYLAND?

> ewebkitn/Source/cmake/OptionsEfl.cmake:192
> +    if (ENABLE_WAYLAND_ONLY)
> +        set(ENABLE_X11_TARGET OFF)
> +    else ()
> +        set(ENABLE_X11_TARGET ON)
> +    endif ()

It looks not necessary if we checked similar logic in above statement.

> ewebkitn/Source/cmake/OptionsEfl.cmake:248
> +    if (ENABLE_WAYLAND_ONLY)
> +        set(ENABLE_GLES2 ON)
> +    endif ()

Should WAYLAND backend depend on this?

> ewebkitn/Source/cmake/OptionsEfl.cmake:264
> +    if (ENABLE_WAYLAND_ONLY)
> +        set(ENABLE_EGL ON)
> +    endif ()

Ditto.

> ewebkit2/Source/WebCore/platform/efl/EflScreenUtilities.cpp:69
> +#ifdef HAVE_ECORE_X
>  #include <Ecore_X.h>
> +#endif

Usually, we moved conditional includes to the bottom of includes with empty line.

#include ...
#include ...

#if XXX
#include ...
#endif

#if YYY
#include ..
#endif

> ewebkit2/Source/WebCore/css/ViewportStyleResolver.cpp:43
> +//for undefined renderStyle(), it's a inline function in header.
> +#include "NodeRenderStyle.h"

I think that this is not related to EFL port.
So I think that we should make different bug for this.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.