Bug 98231 - [EFL] Enable use of X11 in DumpRenderTree / WebKitTestRunner
Summary: [EFL] Enable use of X11 in DumpRenderTree / WebKitTestRunner
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on: 98162 98341 98389
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-02 22:38 PDT by Chris Dumez
Modified: 2012-10-04 02:44 PDT (History)
10 users (show)

See Also:


Attachments
Patch (1.75 KB, patch)
2012-10-02 23:13 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (2.50 KB, patch)
2012-10-02 23:30 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (7.25 KB, patch)
2012-10-02 23:52 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2012-10-02 22:38:36 PDT
We should start using X11 in EFL's DumpRenderTree now that we are using XvfbDriver for the layout tests.
Comment 1 Chris Dumez 2012-10-02 23:13:59 PDT
Created attachment 166810 [details]
Patch
Comment 2 Chris Dumez 2012-10-02 23:25:38 PDT
Comment on attachment 166810 [details]
Patch

Needs the same in WKTR.
Comment 3 Chris Dumez 2012-10-02 23:30:07 PDT
Created attachment 166813 [details]
Patch

Apply same change to WebKitTestRunner.
Comment 4 Chris Dumez 2012-10-02 23:52:01 PDT
Created attachment 166816 [details]
Patch

Properly initialize ecore_x library in ewk_main.
Comment 5 Alexander Shalamov 2012-10-03 00:43:10 PDT
Comment on attachment 166816 [details]
Patch

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

> Source/WebKit/efl/ewk/ewk_main.cpp:101
> +    if (!ecore_x_init(0)) {

Are those init calls are order dependant?
ecore_x_init is not, but I'm just curious how edje and evas objects are initialized.

> Source/WebKit/efl/ewk/ewk_main.cpp:116
> +    edje_shutdown();

Should it be ecore_x_shutdown() ?
Comment 6 Chris Dumez 2012-10-03 00:48:06 PDT
Comment on attachment 166816 [details]
Patch

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

>> Source/WebKit/efl/ewk/ewk_main.cpp:101
>> +    if (!ecore_x_init(0)) {
> 
> Are those init calls are order dependant?
> ecore_x_init is not, but I'm just curious how edje and evas objects are initialized.

I don't believe they are.

>> Source/WebKit/efl/ewk/ewk_main.cpp:116
>> +    edje_shutdown();
> 
> Should it be ecore_x_shutdown() ?

No, this is correct. We come to this label if ecore_x_init() failed so there is no need to call ecore_x_shutdown(). We however, need to shutdown the previously intialized library (edje). See error_edje: 2 lines below, it calls ecore_evas_shutdown(), not edje_shutdown().
Comment 7 Alexander Shalamov 2012-10-03 00:56:44 PDT
(In reply to comment #6)
> (From update of attachment 166816 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=166816&action=review
> 
> >> Source/WebKit/efl/ewk/ewk_main.cpp:101
> >> +    if (!ecore_x_init(0)) {
> > 
> > Are those init calls are order dependant?
> > ecore_x_init is not, but I'm just curious how edje and evas objects are initialized.
> 
> I don't believe they are.
> 
> >> Source/WebKit/efl/ewk/ewk_main.cpp:116
> >> +    edje_shutdown();
> > 
> > Should it be ecore_x_shutdown() ?
> 
> No, this is correct. We come to this label if ecore_x_init() failed so there is no need to call ecore_x_shutdown(). We however, need to shutdown the previously intialized library (edje). See error_edje: 2 lines below, it calls ecore_evas_shutdown(), not edje_shutdown().

LGTM! Thanks.
Comment 8 Raphael Kubo da Costa (:rakuco) 2012-10-03 02:33:20 PDT
Comment on attachment 166816 [details]
Patch

Alright, looks fine, thanks!
Comment 9 WebKit Review Bot 2012-10-03 22:42:36 PDT
Comment on attachment 166816 [details]
Patch

Clearing flags on attachment: 166816

Committed r130363: <http://trac.webkit.org/changeset/130363>
Comment 10 WebKit Review Bot 2012-10-03 22:42:40 PDT
All reviewed patches have been landed.  Closing bug.