Bug 92719 - [EFL][WK2] WTR is failing when X server is not running
Summary: [EFL][WK2] WTR is failing when X server is not running
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexander Shalamov
URL:
Keywords:
Depends on:
Blocks: 61838 92643
  Show dependency treegraph
 
Reported: 2012-07-31 00:25 PDT by Alexander Shalamov
Modified: 2012-08-02 05:56 PDT (History)
9 users (show)

See Also:


Attachments
Patch (1.18 KB, patch)
2012-07-31 00:37 PDT, Alexander Shalamov
no flags Details | Formatted Diff | Diff
Patch (2.96 KB, patch)
2012-07-31 04:36 PDT, Alexander Shalamov
morrita: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (2.99 KB, patch)
2012-08-02 02:21 PDT, Alexander Shalamov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Shalamov 2012-07-31 00:25:51 PDT
All layout tests are crashing on build bot, since the X server is not running.
Comment 1 Alexander Shalamov 2012-07-31 00:37:21 PDT
Created attachment 155456 [details]
Patch

Remove extra checks for ecore_x_init initialisation
Comment 2 Chris Dumez 2012-07-31 00:56:36 PDT
Comment on attachment 155456 [details]
Patch

This is hardly a fix. It will still fail / crash later when you call ecore_x functions I believe. For example, with use ecore_x to emit a system beep.
Comment 3 Alexander Shalamov 2012-07-31 01:04:44 PDT
(In reply to comment #2)
> (From update of attachment 155456 [details])
> This is hardly a fix. It will still fail / crash later when you call ecore_x functions I believe. For example, with use ecore_x to emit a system beep.

Yes it will crash in the same way as WK1 build bot. I could remove dependency to ecore_x calls completely, as it was done for WK1 DRT. Would it be better?
Comment 4 Chris Dumez 2012-07-31 01:12:59 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 155456 [details] [details])
> > This is hardly a fix. It will still fail / crash later when you call ecore_x functions I believe. For example, with use ecore_x to emit a system beep.
> 
> Yes it will crash in the same way as WK1 build bot. I could remove dependency to ecore_x calls completely, as it was done for WK1 DRT. Would it be better?

Hmm. I was wondering if we could find a way to to handle this better.

Basically, what we would need is a function in WebCore to tell us if we running X. And we should call this function before making any ecore_x call (there are not many). Maybe we could use the DISPLAY environment variable to do such check? We need to verify on the build bot that the DISPLAY variable is not set.
Comment 5 Alexander Shalamov 2012-07-31 04:36:41 PDT
Created attachment 155491 [details]
Patch

Use DISPLAY environment variable to identify if X server is running.
Comment 6 Chris Dumez 2012-07-31 04:44:51 PDT
Comment on attachment 155491 [details]
Patch

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

Don't forget to unskip system beep test.

> Tools/WebKitTestRunner/efl/main.cpp:36
> +    const char* display = getenv("DISPLAY");

I would move the display detection code to WebCore in platform/efl/ScreenUtilities.h since it needs to be used in other places (for example in SystemBeep before calling score_x_beep() function). BTW, this is missing from your patch so it will still crash.
Comment 7 Chris Dumez 2012-07-31 04:57:56 PDT
(In reply to comment #6)
> (From update of attachment 155491 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=155491&action=review
> 
> Don't forget to unskip system beep test.
> 
> > Tools/WebKitTestRunner/efl/main.cpp:36
> > +    const char* display = getenv("DISPLAY");
> 
> I would move the display detection code to WebCore in platform/efl/ScreenUtilities.h since it needs to be used in other places (for example in SystemBeep before calling score_x_beep() function). BTW, this is missing from your patch so it will still crash.

Nevermind, we can fix the crasher in a separate patch since the test is skipped.

LGTM.
Comment 8 Hajime Morrita 2012-08-02 00:40:18 PDT
Comment on attachment 155491 [details]
Patch

rubberstamping.
Comment 9 WebKit Review Bot 2012-08-02 00:42:17 PDT
Comment on attachment 155491 [details]
Patch

Rejecting attachment 155491 [details] from commit-queue.

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

Last 500 characters of output:
cceeded at 1 with fuzz 3.
patching file Tools/Scripts/webkitpy/layout_tests/port/efl.py
patching file Tools/WebKitTestRunner/efl/main.cpp
Hunk #1 FAILED at 21.
Hunk #2 succeeded at 35 (offset 3 lines).
Hunk #3 succeeded at 50 (offset 3 lines).
1 out of 3 hunks FAILED -- saving rejects to file Tools/WebKitTestRunner/efl/main.cpp.rej

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Hajime Mor..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue/

Full output: http://queues.webkit.org/results/13428090
Comment 10 Alexander Shalamov 2012-08-02 02:21:31 PDT
Created attachment 156017 [details]
Patch

Rebased to master
Comment 11 WebKit Review Bot 2012-08-02 05:56:33 PDT
Comment on attachment 156017 [details]
Patch

Clearing flags on attachment: 156017

Committed r124445: <http://trac.webkit.org/changeset/124445>
Comment 12 WebKit Review Bot 2012-08-02 05:56:39 PDT
All reviewed patches have been landed.  Closing bug.