Bug 92101

Summary: [EFL][WK2] Add ewk_main.{cpp,h} to EFL WK2
Product: WebKit Reporter: Mikhail Pozdnyakov <mikhail.pozdnyakov>
Component: WebKit2Assignee: Mikhail Pozdnyakov <mikhail.pozdnyakov>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, gyuyoung.kim, kenneth, rakuco, ryuan.choi, tonikitoo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 61838    
Attachments:
Description Flags
patch
none
patch v2
kenneth: review+, kenneth: commit-queue-
to be landed
webkit.review.bot: commit-queue-
to be landed none

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]
patch
Comment 2 Chris Dumez 2012-07-24 05:27:44 PDT
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.
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.

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
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.