[EFL][WK2] WTR is failing when X server is not running

• Component: WebKit2
• Status: RESOLVED FIXED

[Alexander Shalamov]

All layout tests are crashing on build bot, since the X server is not running.

[Alexander Shalamov]

Created attachment 155456 [details]
Patch

Remove extra checks for ecore_x_init initialisation

[Chris Dumez]

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.

[Alexander Shalamov]

(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?

[Chris Dumez]

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

[Alexander Shalamov]

Created attachment 155491 [details]
Patch

Use DISPLAY environment variable to identify if X server is running.

[Chris Dumez]

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.

[Chris Dumez]

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

[Hajime Morrita]

Comment on attachment 155491 [details]
Patch

rubberstamping.

[WebKit Review Bot]

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

[Alexander Shalamov]

Created attachment 156017 [details]
Patch

Rebased to master

[WebKit Review Bot]

Comment on attachment 156017 [details]
Patch

Clearing flags on attachment: 156017

Committed r124445: <http://trac.webkit.org/changeset/124445>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.