Add Xorg+dummy as alternative for Xvfb

remoting/host/linux/linux_me2me_host.py

 #!/usr/bin/python
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 # Virtual Me2Me implementation.  This script runs and manages the processes
 # required for a Virtual Me2Me desktop, which are: X server, X desktop
 # session, and Host process.
 # This script is intended to run continuously as a background daemon
 # process, running under an ordinary (non-root) user account.
@@ skipping 24 matching lines @@
 import time
 import uuid
 
 LOG_FILE_ENV_VAR = "CHROME_REMOTE_DESKTOP_LOG_FILE"
 
 # This script has a sensible default for the initial and maximum desktop size,
 # which can be overridden either on the command-line, or via a comma-separated
 # list of sizes in this environment variable.
 DEFAULT_SIZES_ENV_VAR = "CHROME_REMOTE_DESKTOP_DEFAULT_DESKTOP_SIZES"
 
+# By default, this script launches Xvfb as the virtual X display. When this
+# environment variable is set, the script will instead launch an instance of
+# Xorg using the dummy display driver and void input device.

[Jamie, 2016/05/07 00:05:47]

Add instructions here on how to install dummy and void (you can remove them from
the CL description if you like).

[rkjnsn, 2016/05/09 16:58:34]

It's obvious now that you mention it.

+USE_XORG_ENV_VAR = "CHROME_REMOTE_DESKTOP_USE_XORG"
+
 # By default, provide a maximum size that is large enough to support clients
 # with large or multiple monitors. This is a comma-separated list of
 # resolutions that will be made available if the X server supports RANDR. These
 # defaults can be overridden in ~/.profile.
 DEFAULT_SIZES = "1600x1200,3840x2560"
 
 # If RANDR is not available, use a smaller default size. Only a single
 # resolution is supported in this case.
 DEFAULT_SIZE_NO_RANDR = "1600x1200"
 
@@ skipping 40 matching lines @@
   # session file exist.
   if (os.path.isdir(CONFIG_DIR) or os.path.isfile(SESSION_FILE_PATH) or
       os.path.isfile(SYSTEM_SESSION_FILE_PATH)):
     return True
 
   # The host has been tested only on Ubuntu.
   distribution = platform.linux_distribution()
   return (distribution[0]).lower() == 'ubuntu'
 
 
-def get_randr_supporting_x_server():
-  """Returns a path to an X server that supports the RANDR extension, if this
-  is found on the system. Otherwise returns None."""
+def locate_xvfb_randr():
+  """Returns a path to our RANDR-supporting Xvfb server, if it is found on the
+  system. Otherwise returns None."""
 
   xvfb = "/usr/bin/Xvfb-randr"
   if os.path.exists(xvfb):
     return xvfb
 
   xvfb = os.path.join(SCRIPT_DIR, "Xvfb-randr")
   if os.path.exists(xvfb):
     return xvfb
 
   return None
@@ skipping 113 matching lines @@
 
 class Desktop:
   """Manage a single virtual desktop"""
 
   def __init__(self, sizes):
     self.x_proc = None
     self.session_proc = None
     self.host_proc = None
     self.child_env = None
     self.sizes = sizes
+    self.xorg_conf = None
     self.pulseaudio_pipe = None
     self.server_supports_exact_resize = False
     self.host_ready = False
     self.ssh_auth_sockname = None
     g_desktops.append(self)
 
   @staticmethod
   def get_unused_display_number():
     """Return a candidate display number for which there is currently no
     X Server lock file"""
@@ skipping 98 matching lines @@
     self.child_env["PULSE_STATE_PATH"] = pulse_path
     self.child_env["PULSE_SINK"] = sink_name
     self.pulseaudio_pipe = pipe_name
 
     return True
 
   def _setup_gnubby(self):
     self.ssh_auth_sockname = ("/tmp/chromoting.%s.ssh_auth_sock" %
                               os.environ["USER"])
 
+  def _launch_xvfb(self, display, x_auth_file, extra_x_args):
+    max_width = max([width for width, height in self.sizes])
+    max_height = max([height for width, height in self.sizes])
+
+    xvfb = locate_xvfb_randr()
+    if xvfb:
+      self.server_supports_exact_resize = True
+    else:
+      xvfb = "Xvfb"
+      self.server_supports_exact_resize = False
+
+    logging.info("Starting %s on display :%d" % (xvfb, display))
+    screen_option = "%dx%dx24" % (max_width, max_height)
+    self.x_proc = subprocess.Popen(
+        [xvfb, ":%d" % display,
+         "-auth", x_auth_file,
+         "-nolisten", "tcp",
+         "-noreset",
+         "-screen", "0", screen_option
+        ] + extra_x_args)
+    if not self.x_proc.pid:
+      raise Exception("Could not start Xvfb.")
+
+  def _launch_xorg(self, display, x_auth_file, extra_x_args):
+    config_file = tempfile.NamedTemporaryFile(
+        prefix="chrome_remote_desktop_", suffix=".conf", delete=False)
+    config_file.write(
+        # This causes X to load the default GLX module, even if a proprietary
+        # one is installed in a different directory.
+        'Section "Files"\n'
+        '  ModulePath "/usr/lib/xorg/modules"\n'
+        'EndSection\n'
+        '\n'
+        # Suppress device probing, which happens by default.
+        'Section "ServerFlags"\n'
+        '  Option "AutoAddDevices" "false"\n'
+        '  Option "AutoEnableDevices" "false"\n'
+        '  Option "DontVTSwitch" "true"\n'
+        '  Option "PciForceNone" "true"\n'
+        'EndSection\n'
+        '\n'
+        'Section "InputDevice"\n'
+        '  Identifier "Chrome Remote Desktop Input"\n'
+        '  Option "CoreKeyboard" "true"\n'
+        '  Option "CorePointer" "true"\n'
+        '  Driver "void"\n'
+        'EndSection\n'
+        '\n'
+        'Section "Device"\n'
+        '  Identifier "Chrome Remote Desktop Videocard"\n'
+        '  Driver "dummy"\n'
+        '  VideoRam 1048576\n'

[Jamie, 2016/05/07 00:05:47]

I think it's worth defining named constants for this (and other) magic values in
the config file, with a brief description of why they were chosen.

[rkjnsn, 2016/05/09 16:58:34]

That requires me to have a good reason for why they were chosen. :) Will do.

+        'EndSection\n'
+        '\n'
+        'Section "Monitor"\n'
+        '  Identifier "Chrome Remote Desktop Monitor"\n'
+        '  HorizSync   3.3\n'
+        '  VertRefresh 0.1\n'
+        '{modelines}'
+        'EndSection\n'
+        '\n'
+        'Section "Screen"\n'
+        '  Identifier "Chrome Remote Desktop Screen"\n'
+        '  Device "Chrome Remote Desktop Videocard"\n'
+        '  Monitor "Chrome Remote Desktop Monitor"\n'
+        '  DefaultDepth 24\n'
+        '  SubSection "Display"\n'
+        '    Viewport 0 0\n'
+        '    Depth 24\n'
+        '    Modes {modes}\n'
+        '  EndSubSection\n'
+        'EndSection\n'
+        '\n'
+        'Section "ServerLayout"\n'
+        '  Identifier   "Chrome Remote Desktop Layout"\n'
+        '  Screen       "Chrome Remote Desktop Screen"\n'
+        '  InputDevice  "Chrome Remote Desktop Input"\n'
+        'EndSection\n'.format(
+            modelines="".join(
+                '  Modeline "{0}x{1}" 108.9 {0} 32998 32999 33000 '
+                '{1} 32998 32999 33000\n'.format(w, h) for w, h in self.sizes),
+            modes=" ".join('"{0},{1}"'.format(w, h) for w, h in self.sizes)))
+    config_file.close()

[Jamie, 2016/05/07 00:05:47]

Would using "with" be more Pythonic than explicitly closing the file here?

[rkjnsn, 2016/05/09 16:58:33]

Probably. Will do.

+
+    # For now, we don't support exact resize with Xorg+dummy
+    self.server_supports_exact_resize = False
+    self.xorg_conf = config_file.name
+
+    logging.info("Starting Xorg on display :%d" % display)
+    # We use the child environment so the Xorg server picks up the Mesa libGL
+    # instead of any proprietary versions that may be installed, thanks to
+    # LD_LIBRARY_PATH.
+    # TODO(rkjnsn): Perhaps we want to pass the rest of our environment through
+    # instead of using the clean one we create for the session so that the user
+    # can pass environment variables to Xorg like they could for Xvfb.

[Jamie, 2016/05/07 00:05:47]

I don't think we need this TODO. It seems unlikely we'll ever do it unless it
proves to be a problem for end-users.

[rkjnsn, 2016/05/09 16:58:33]

Acknowledged.

+    self.x_proc = subprocess.Popen(
+        ["Xorg", ":%d" % display,
+         "-auth", x_auth_file,
+         "-nolisten", "tcp",
+         "-noreset",
+         # Log to stderr instead of a file so the output ends up in our main
+         # log file

[Jamie, 2016/05/07 00:05:47]

How does specifying the logfile as /dev/null log to stderr? I'd have thought it
effectively disabled logging.

[rkjnsn, 2016/05/09 16:58:33]

Ah, yes. I was worried this might not be clear. By default, Xorg logs to both a
file at level 3 and stderr at level 0. This is disabling logging to the file,
and bumping the stderr logging (which is already enabled) to level 3 so it
mirrors what would be written to the log file. I'll try to expand a bit more in
the comment.

[Jamie, 2016/05/10 00:37:09]

I completely missed the -verbose flag. Please move it to a separate line so that
you've got one option per line as per the other options.

+         "-logfile", "/dev/null", "-verbose", "3",
+         "-config", config_file.name
+        ] + extra_x_args, env=self.child_env)
+    if not self.x_proc.pid:
+      raise Exception("Could not start Xorg.")
+
   def _launch_x_server(self, extra_x_args):
     x_auth_file = os.path.expanduser("~/.Xauthority")
     self.child_env["XAUTHORITY"] = x_auth_file
     devnull = open(os.devnull, "r+")
     display = self.get_unused_display_number()
 
     # Run "xauth add" with |child_env| so that it modifies the same XAUTHORITY
     # file which will be used for the X session.
     ret_code = subprocess.call("xauth add :%d . `mcookie`" % display,
                                env=self.child_env, shell=True)
     if ret_code != 0:
       raise Exception("xauth failed with code %d" % ret_code)
 
-    max_width = max([width for width, height in self.sizes])
-    max_height = max([height for width, height in self.sizes])
-
-    xvfb = get_randr_supporting_x_server()
-    if xvfb:
-      self.server_supports_exact_resize = True
-    else:
-      xvfb = "Xvfb"
-      self.server_supports_exact_resize = False
-
     # Disable the Composite extension iff the X session is the default
     # Unity-2D, since it uses Metacity which fails to generate DAMAGE
     # notifications correctly. See crbug.com/166468.
     x_session = choose_x_session()
     if (len(x_session) == 2 and
         x_session[1] == "/usr/bin/gnome-session --session=ubuntu-2d"):
       extra_x_args.extend(["-extension", "Composite"])
 
-    logging.info("Starting %s on display :%d" % (xvfb, display))
-    screen_option = "%dx%dx24" % (max_width, max_height)
-    self.x_proc = subprocess.Popen(
-        [xvfb, ":%d" % display,
-         "-auth", x_auth_file,
-         "-nolisten", "tcp",
-         "-noreset",
-         "-screen", "0", screen_option
-        ] + extra_x_args)
-    if not self.x_proc.pid:
-      raise Exception("Could not start Xvfb.")
+    if USE_XORG_ENV_VAR in os.environ:
+      self._launch_xorg(display, x_auth_file, extra_x_args)
+    else:
+      self._launch_xvfb(display, x_auth_file, extra_x_args)
 
     self.child_env["DISPLAY"] = ":%d" % display
     self.child_env["CHROME_REMOTE_DESKTOP_SESSION"] = "1"
 
     # Use a separate profile for any instances of Chrome that are started in
     # the virtual session. Chrome doesn't support sharing a profile between
     # multiple DISPLAYs, but Chrome Sync allows for a reasonable compromise.
     chrome_profile = os.path.join(CONFIG_DIR, "chrome-profile")
     self.child_env["CHROME_USER_DATA_DIR"] = chrome_profile
 
     # Set SSH_AUTH_SOCK to the file name to listen on.
     if self.ssh_auth_sockname:
       self.child_env["SSH_AUTH_SOCK"] = self.ssh_auth_sockname
 
     # Wait for X to be active.
     for _test in range(20):
       retcode = subprocess.call("xdpyinfo", env=self.child_env, stdout=devnull)
       if retcode == 0:
         break
       time.sleep(0.5)
     if retcode != 0:
-      raise Exception("Could not connect to Xvfb.")
+      raise Exception("Could not connect to X server.")
     else:
-      logging.info("Xvfb is active.")
+      logging.info("X server is active.")
 
     # The remoting host expects the server to use "evdev" keycodes, but Xvfb
     # starts configured to use the "base" ruleset, resulting in XKB configuring
     # for "xfree86" keycodes, and screwing up some keys. See crbug.com/119013.
     # Reconfigure the X server to use "evdev" keymap rules.  The X server must
     # be started with -noreset otherwise it'll reset as soon as the command
     # completes, since there are no other X clients running yet.
     retcode = subprocess.call("setxkbmap -rules evdev", env=self.child_env,
                               shell=True)
     if retcode != 0:
@@ skipping 389 matching lines @@
   # Close the temporary file-descriptors.
   os.close(devnull_fd)
   os.close(log_fd)
 
 
 def cleanup():
   logging.info("Cleanup.")
 
   global g_desktops
   for desktop in g_desktops:
-    for proc, name in [(desktop.x_proc, "Xvfb"),
+    for proc, name in [(desktop.x_proc, "X server"),
                        (desktop.session_proc, "session"),
                        (desktop.host_proc, "host")]:
       if proc is not None:
         logging.info("Terminating " + name)
         try:
           psutil_proc = psutil.Process(proc.pid)
           psutil_proc.terminate()
 
           # Use a short timeout, to avoid delaying service shutdown if the
           # process refuses to die for some reason.
           psutil_proc.wait(timeout=10)
         except psutil.TimeoutExpired:
           logging.error("Timed out - sending SIGKILL")
           psutil_proc.kill()
         except psutil.Error:
           logging.error("Error terminating process")
+    if desktop.xorg_conf is not None:
+      os.remove(desktop.xorg_conf)
 
   g_desktops = []
   if ParentProcessLogger.instance():
     ParentProcessLogger.instance().release_parent(False)
 
 class SignalHandler:
   """Reload the config file on SIGHUP. Since we pass the configuration to the
   host processes via stdin, they can't reload it, so terminate them. They will
   be relaunched automatically with the new config."""
 
@@ skipping 180 matching lines @@
 def main():
   EPILOG = """This script is not intended for use by end-users.  To configure
 Chrome Remote Desktop, please install the app from the Chrome
 Web Store: https://chrome.google.com/remotedesktop"""
   parser = optparse.OptionParser(
       usage="Usage: %prog [options] [ -- [ X server options ] ]",
       epilog=EPILOG)
   parser.add_option("-s", "--size", dest="size", action="append",
                     help="Dimensions of virtual desktop. This can be specified "
                     "multiple times to make multiple screen resolutions "
-                    "available (if the Xvfb server supports this).")
+                    "available (if the X server supports this).")
   parser.add_option("-f", "--foreground", dest="foreground", default=False,
                     action="store_true",
                     help="Don't run as a background daemon.")
   parser.add_option("", "--start", dest="start", default=False,
                     action="store_true",
                     help="Start the host.")
   parser.add_option("-k", "--stop", dest="stop", default=False,
                     action="store_true",
                     help="Stop the daemon currently running.")
   parser.add_option("", "--get-status", dest="get_status", default=False,
@@ skipping 113 matching lines @@
     watch_for_resolution_changes(options.watch_resolution)
     return 0
 
   if not options.start:
     # If no modal command-line options specified, print an error and exit.
     print(EPILOG, file=sys.stderr)
     return 1
 
   # If a RANDR-supporting Xvfb is not available, limit the default size to
   # something more sensible.
-  if get_randr_supporting_x_server():
+  if USE_XORG_ENV_VAR not in os.environ and locate_xvfb_randr():
     default_sizes = DEFAULT_SIZES
   else:
     default_sizes = DEFAULT_SIZE_NO_RANDR
 
   # Collate the list of sizes that XRANDR should support.
   if not options.size:
     if DEFAULT_SIZES_ENV_VAR in os.environ:
       default_sizes = os.environ[DEFAULT_SIZES_ENV_VAR]
     options.size = default_sizes.split(",")
 
@@ skipping 180 matching lines @@
         else:
           logging.info("Host exited with status %s." % os.WEXITSTATUS(status))
       elif os.WIFSIGNALED(status):
         logging.info("Host terminated by signal %s." % os.WTERMSIG(status))
 
 
 if __name__ == "__main__":
   logging.basicConfig(level=logging.DEBUG,
                       format="%(asctime)s:%(levelname)s:%(message)s")
   sys.exit(main())