[webkit-reviews] review granted: [Bug 92101] [EFL][WK2] Add ewk_main.{cpp, h} to EFL WK2 : [Attachment 154027] patch v2

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has granted Mikhail Pozdnyakov
<mikhail.pozdnyakov at intel.com>'s request for review:
Bug 92101: [EFL][WK2] Add ewk_main.{cpp,h} to EFL WK2
https://bugs.webkit.org/show_bug.cgi?id=92101

Attachment 154027: patch v2
https://bugs.webkit.org/attachment.cgi?id=154027&action=review

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
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