Bug 92101 – [EFL][WK2] Add ewk_main.{cpp,h} to EFL WK2

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Mikhail Pozdnyakov]

EFL WK2 should have ewk_main.{cpp,h} files same way as EFL WK1 has.
It is necessary to have a centralized place for general initialization (like eina_init and g_type_init) 
inside UI process.

[Mikhail Pozdnyakov]

Created attachment 154025 [details]
patch

[Chris Dumez]

Comment on attachment 154025 [details]
patch

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

LGTM, except small nit.

> Source/WebKit2/UIProcess/API/efl/ewk_main.cpp:41
> +static Eina_Bool _ewk_init_body(void);

Can be removed.

> Source/WebKit2/UIProcess/API/efl/ewk_main.cpp:114
> +Eina_Bool _ewk_init_body(void)

I would just move the glib init stuff to ewk_init() and remove _ewk_init_body() completely.

[Mikhail Pozdnyakov]

Created attachment 154027 [details]
patch v2

Chris, thanks for review.

[Chris Dumez]

Comment on attachment 154027 [details]
patch v2

LGTM. We really need this. Currently the MiniBrowser does not call g_type_init() and it pretty much unusable.

[Kenneth Rohde Christiansen]

Comment on attachment 154027 [details]
patch v2

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

> Source/WebKit2/UIProcess/API/efl/ewk_main.cpp:73
> +
> +    if (!edje_init()) {
> +        EINA_LOG_DOM_CRIT(_ewk_log_dom, "could not init edje.");
> +        goto error_edje;
> +    }

Isn't it (or shouldn't it) be possible to use this without edje? Generally I do not think the html forms styles should be implemented using Edje, as it seems like an unnecessary dependency and it doesn't allow for nice rendering when scaled.

> Source/WebKit2/UIProcess/API/efl/ewk_main.cpp:79
> +    if (!ecore_main_loop_glib_integrate())
> +        EINA_LOG_DOM_WARN(_ewk_log_dom, "Ecore was not compiled with GLib support, some plugins will not "
> +            "work (ie: Adobe Flash)");

More than one actual line in the if sentence thus must use braces according to the style guide

> Source/WebKit2/UIProcess/API/efl/ewk_main.h:24
> + * @brief   The main file of WebKit2-EFL, not tied to any view object.

main file?

> Source/WebKit2/UIProcess/API/efl/ewk_main.h:37
> + * Initializes webkit's instance.

WebKit

> Source/WebKit2/UIProcess/API/efl/ewk_main.h:39
> + * - initializes components needed by Efl,

EFL

> Source/WebKit2/UIProcess/API/efl/ewk_main.h:40
> + * - increases a reference count of webkit's instance.

WebKit

[Mikhail Pozdnyakov]

Created attachment 154269 [details]
to be landed

[WebKit Review Bot]

Comment on attachment 154269 [details]
to be landed

Rejecting attachment 154269 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

ERROR: /mnt/git/webkit-commit-queue/Source/WebKit2/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/13342154

[Gyuyoung Kim]

Comment on attachment 154269 [details]
to be landed

I think WK2 also may need to have _ewk_init_body as WK1. But, looks good for now.

[Mikhail Pozdnyakov]

Created attachment 154277 [details]
to be landed

Added missing reviewer name.

[WebKit Review Bot]

Comment on attachment 154277 [details]
to be landed

Clearing flags on attachment: 154277

Committed r123587: <http://trac.webkit.org/changeset/123587>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.