Bug 92719

Created attachment 155456 [details] Patch Remove extra checks for ecore_x_init initialisation

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.

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

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

Created attachment 155491 [details] Patch Use DISPLAY environment variable to identify if X server is running.

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.

(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 on attachment 155491 [details] Patch rubberstamping.

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

Created attachment 156017 [details] Patch Rebased to master

Comment on attachment 156017 [details] Patch Clearing flags on attachment: 156017 Committed r124445: <http://trac.webkit.org/changeset/124445>

All reviewed patches have been landed. Closing bug.