In run-webkit-tests, start xvfb if necessary.

third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/linux.py

 # Copyright (C) 2010 Google Inc. All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
 # met:
 #
 #     * Redistributions of source code must retain the above copyright
 # notice, this list of conditions and the following disclaimer.
 #     * Redistributions in binary form must reproduce the above
 # copyright notice, this list of conditions and the following disclaimer
@@ skipping 47 matching lines @@
 
     def __init__(self, host, port_name, **kwargs):
         super(LinuxPort, self).__init__(host, port_name, **kwargs)
         self._version = port_name[port_name.index('linux-') + len('linux-'):]
         self._architecture = 'x86_64'
         assert self._version in self.SUPPORTED_VERSIONS
 
         if not self.get_option('disable_breakpad'):
             self._dump_reader = DumpReaderLinux(host, self._build_path())
         self._original_home = None
+        self._xvfb_process = None
 
     def additional_driver_flag(self):
         flags = super(LinuxPort, self).additional_driver_flag()
         if not self.get_option('disable_breakpad'):
             flags += ['--enable-crash-reporter', '--crash-dumps-dir=%s' % self._dump_reader.crash_dumps_directory()]
         return flags
 
     def check_build(self, needs_http, printer):
         result = super(LinuxPort, self).check_build(needs_http, printer)
 
@@ skipping 19 matching lines @@
         # The Apache binary path can vary depending on OS and distribution
         # See http://wiki.apache.org/httpd/DistrosDefaultLayout
         for path in ['/usr/sbin/httpd', '/usr/sbin/apache2']:
             if self._filesystem.exists(path):
                 return path
         _log.error('Could not find apache. Not installed or unknown path.')
         return None
 
     def setup_test_run(self):
         super(LinuxPort, self).setup_test_run()
+        self._start_xvfb()
         self._setup_dummy_home_dir()
 
     def clean_up_test_run(self):
         super(LinuxPort, self).clean_up_test_run()
+        self._stop_xvfb()
         self._clean_up_dummy_home_dir()
 
     #
     # PROTECTED METHODS
     #
 
     def _setup_dummy_home_dir(self):
         """Creates a dummy home directory for running the test.
 
         This is a workaround for crbug.com/595504; see crbug.com/612730.
@@ skipping 14 matching lines @@
                 continue
             fs.copyfile(original_path, fs.join(dummy_home, filename))
 
     def _clean_up_dummy_home_dir(self):
         """Cleans up the dummy dir and resets the HOME environment variable."""
         dummy_home = self.host.environ['HOME']
         assert dummy_home != self._original_home
         self._filesystem.rmtree(dummy_home)
         self.host.environ['HOME'] = self._original_home
 
+    def _start_xvfb(self):
+        if self._has_x_server():
+            return
+        display = self.host.environ.get('DISPLAY') or ':20'

[qyearsley, 2017/04/14 18:25:40]

This choice of :20 is arbitrary.

[Dirk Pranke, 2017/04/14 21:44:08]

Don't use the existing DISPLAY value, that'll just confuse things. Also, you
shouldn't alter the existing environ, just make sure that the new value gets
propagated to the driver in setup_environ_for_server().

You might want to use :99 for compatibility w/ xvfb-run, and also be prepared to
loop to find a free display.

And, check the error code from the subprocess.

[qyearsley, 2017/04/18 01:02:39]

There's one complication here: a collection of new Port objects is created for
each Worker, so if the display found (e.g. :99 or :100) during setup is stored
on the original Port object itself that won't work, since new Ports are created
for each new Worker.

I think we could store the display found on the Host or somewhere else to ensure
that is available when creating and setting up Workers, but this might not be as
simple.

If we alter the run-webkit-tests process environment, though, then that also
works (all of the subprocesses get the new DISPLAY), and this wouldn't affect
the value of DISPLAY after run-webkit-tests exits.

Is it still a good idea to avoid setting the existing DISPLAY environment
variable?
Done.
Which subprocess? The Xvfb process,by calling self._xvfb_process.wait() after
self._xvfb_process.kill()?

[Dirk Pranke, 2017/04/18 02:16:18]

OK, good point.

I guess go w/ setting the exsting DISPLAY for now.
I meant, make sure that Xvfb started properly, so check to make sure the process
hasn't exited.

[qyearsley, 2017/04/18 17:58:35]

Alright, will do this for now. Added comment about this as well.
Done (by checking whether process.poll() is None).

+        self.host.environ['DISPLAY'] = display
+        _log.debug('Starting Xvfb with display location "%s".', display)
+        self._xvfb_process = self.host.executive.popen(
+            ['Xvfb', display, '-screen', '0', '1280x800x24', '-ac', '-dpi', '96'],

[qyearsley, 2017/04/14 18:25:40]

Not sure if all of these options are necessary.

 - `man Xvfb` says that if -screen is not given, then the default is screen 0,
1280x1024x8.
 - `man Xserver` says that if -dpi is not given, it's determined automatically
(or guessed?), and "-ac" disables host-based access control mechanisms, and
should be used with caution.

[Dirk Pranke, 2017/04/18 02:16:18]

You need -screen, because you don't want to conflict if a user is running this.
You might be able to get away without -ac, not sure. You want to set -dpi to be
sure you're getting the dpi you expect.

[qyearsley, 2017/04/18 17:58:35]

Alright, will keep these arguments, later we could possibly experiment with
removing -ac but it's probably not important.

+            stderr=self.host.executive.PIPE)
+
+    def _has_x_server(self):
+        display = self.host.environ.get('DISPLAY')
+        print display
+        _log.debug('Checking for X server on display location "%s".', display)
+        if not display:
+            return False
+        exit_code = self.host.executive.run_command(
+            ['xdpyinfo', '-display', display], return_exit_code=True)
+        return exit_code == 0
+
+    def _stop_xvfb(self):
+        if not self._xvfb_process:
+            return
+        _log.debug('Killing Xvfb process pid %d.', self._xvfb_process.pid)
+        self._xvfb_process.kill()

[qyearsley, 2017/04/14 18:25:40]

Not sure whether it would be prudent to do anything else here; presumably since
Xvfb is a subprocess of python run-webkit-tests, then it will be cleaned up even
if for some reason it fails to be killed here?

[Dirk Pranke, 2017/04/18 02:16:18]

I'd probably call self._xvfb_process.wait() just to be safe.

[qyearsley, 2017/04/18 17:58:35]

Done.

+
     def _path_to_driver(self, target=None):
         binary_name = self.driver_name()
         return self._build_path_with_target(target, binary_name)