Bug 92101 - [EFL][WK2] Add ewk_main.{cpp,h} to EFL WK2
Summary: [EFL][WK2] Add ewk_main.{cpp,h} to EFL WK2
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mikhail Pozdnyakov
Depends on:
Blocks: 61838
  Show dependency treegraph
Reported: 2012-07-24 04:57 PDT by Mikhail Pozdnyakov
Modified: 2012-07-25 01:30 PDT (History)
7 users (show)

See Also:

patch (10.11 KB, patch)
2012-07-24 05:20 PDT, Mikhail Pozdnyakov
no flags Details | Formatted Diff | Diff
patch v2 (9.89 KB, patch)
2012-07-24 05:34 PDT, Mikhail Pozdnyakov
kenneth: review+
kenneth: commit-queue-
Details | Formatted Diff | Diff
to be landed (9.75 KB, patch)
2012-07-24 23:39 PDT, Mikhail Pozdnyakov
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
to be landed (9.81 KB, patch)
2012-07-25 00:10 PDT, Mikhail Pozdnyakov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mikhail Pozdnyakov 2012-07-24 04:57:34 PDT
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.
Comment 1 Mikhail Pozdnyakov 2012-07-24 05:20:26 PDT
Created attachment 154025 [details]
Comment 2 Chris Dumez 2012-07-24 05:27:44 PDT
Comment on attachment 154025 [details]

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.
Comment 3 Mikhail Pozdnyakov 2012-07-24 05:34:15 PDT
Created attachment 154027 [details]
patch v2

Chris, thanks for review.
Comment 4 Chris Dumez 2012-07-24 05:40:26 PDT
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.
Comment 5 Kenneth Rohde Christiansen 2012-07-24 10:48:02 PDT
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.


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


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

Comment 6 Mikhail Pozdnyakov 2012-07-24 23:39:29 PDT
Created attachment 154269 [details]
to be landed
Comment 7 WebKit Review Bot 2012-07-24 23:57:00 PDT
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
Comment 8 Gyuyoung Kim 2012-07-25 00:05:10 PDT
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.
Comment 9 Mikhail Pozdnyakov 2012-07-25 00:10:51 PDT
Created attachment 154277 [details]
to be landed

Added missing reviewer name.
Comment 10 WebKit Review Bot 2012-07-25 01:30:21 PDT
Comment on attachment 154277 [details]
to be landed

Clearing flags on attachment: 154277

Committed r123587: <http://trac.webkit.org/changeset/123587>
Comment 11 WebKit Review Bot 2012-07-25 01:30:27 PDT
All reviewed patches have been landed.  Closing bug.