Increase default maximum desktop size and make it configurable.

remoting/tools/me2me_virtual_host.py

 #!/usr/bin/env 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 16 matching lines @@
 
 # By default this script will try to determine the most appropriate X session
 # command for the system.  To use a specific session instead, set this variable
 # to the executable filename, or a list containing the executable and any
 # arguments, for example:
 # XSESSION_COMMAND = "/usr/bin/gnome-session-fallback"
 # XSESSION_COMMAND = ["/usr/bin/gnome-session", "--session=ubuntu-2d"]
 XSESSION_COMMAND = None
 
 LOG_FILE_ENV_VAR = "CHROME_REMOTE_DESKTOP_LOG_FILE"
+DEFAULT_SIZES_ENV_VAR = "CHROME_REMOTE_DESKTOP_DEFAULT_DESKTOP_SIZES"

[Wez, 2012/12/14 19:49:03]

nit: Consider describing the format this variable must take, either here or
better still in the usage output.

[Jamie, 2012/12/14 21:13:17]

Done. The usage output doesn't mention this variable, and I consider it to be an
advanced option, so I think documenting it here is fine.

 
 SCRIPT_PATH = sys.path[0]
 
 DEFAULT_INSTALL_PATH = "/opt/google/chrome-remote-desktop"
 if SCRIPT_PATH == DEFAULT_INSTALL_PATH:
   HOST_BINARY_NAME = "chrome-remote-desktop-host"
 else:
   HOST_BINARY_NAME = "remoting_me2me_host"
 
 CHROME_REMOTING_GROUP_NAME = "chrome-remote-desktop"
@@ skipping 694 matching lines @@
           raise
         elif deadline > now:
           time.sleep(deadline - now)
         return (0, 0)
       else:
         # Anything else is an unexpected error.
         raise
 
 
 def main():
-  DEFAULT_SIZE = "2560x1600"
   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 (default: %s). "
-                    "This can be specified multiple times to make multiple "
-                    "screen resolutions available (if the Xvfb server "
-                    "supports this)" % DEFAULT_SIZE)
+                    help="Dimensions of virtual desktop. This can be specified "
+                    "multiple times to make multiple screen resolutions "
+                    "available (if the Xvfb 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("", "--check-running", dest="check_running", default=False,
@@ skipping 51 matching lines @@
 
   if options.host_version:
     # TODO(sergeyu): Also check RPM package version once we add RPM package.
     return os.system(locate_executable(HOST_BINARY_NAME) + " --version") >> 8
 
   if not options.start:
     # If no modal command-line options specified, print an error and exit.
     print >> sys.stderr, EPILOG
     return 1
 
-  # Collate the list of sizes that XRANDR should support.
+  # Collate the list of sizes that XRANDR should support. If no sizes were
+  # specified on the command-line, provide a relatively small size to handle

[Lambros, 2012/12/14 19:59:05]

In the non-RANDR case, we are taking the box-union of all the |sizes| and
passing that for the "-screen" X server option - see _launch_x_server(). That is
not "relatively small" :) We might have to revisit this logic if we really do
want a small-ish default size?

[Jamie, 2012/12/14 21:13:17]

I don't think that the non-RANDR case is something we need to optimize for,
given that it's by far the less common use-case. The user can always fix things
via a .profile tweak.

+  # the case where resize-to-client is disabled or not supported, and a much
+  # larger size to support clients with large or mulitple monitors. These
+  # defaults can be overridden by the user by defining a variable in .profile.
   if not options.size:
-    options.size = [DEFAULT_SIZE]
+    default_sizes = "1600x1200,3840x1600"

[Wez, 2012/12/14 19:49:03]

nit: This still feels like something that should be a constant at the top of the
file, aking to the X port number.

[Lambros, 2012/12/14 19:59:05]

It's a strange letter-boxy default. I'd be inclined to increase the final 1600
to, say, 2560 (in case someone wants to rotate a large monitor into portrait
mode, or wants to stack two smaller monitors vertically). WDYT?

[Jamie, 2012/12/14 21:13:17]

That increases Xvfb memory usage from 34 to 48Mb, which I think is starting to
get too big for a default setup. We have to strike a balance between making sure
that things "just work" for common use cases, without being too resource-hungry.
I don't think we can make this work OOtB for all users, hence the environment
variable.

[Jamie, 2012/12/14 21:13:17]

Done.

[Wez, 2012/12/14 21:19:20]

Linux will only actually commit pages that we touch - that doesn't really help
with the width, since pages are 4KB on x86, so the pages will fall on 1024 pixel
boundaires, but it should mean that pages containing pixels beyond the bottom of
the current dimensions just count against swap quota, in effect.

[Jamie, 2012/12/14 21:23:19]

Is this a vote for increasing the height or just a comment?

+    if os.environ.has_key(DEFAULT_SIZES_ENV_VAR):
+      default_sizes = os.environ[DEFAULT_SIZES_ENV_VAR]

[Wez, 2012/12/14 19:49:03]

Do we get enough of the user's environment for this to work as expected at boot
time?

[Jamie, 2012/12/14 21:13:17]

Yes, Lambros did a test and confirmed that .profile is sourced bu su --login.

+    options.size = default_sizes.split(",");
 
   sizes = []
   for size in options.size:
     size_components = size.split("x")
     if len(size_components) != 2:
       parser.error("Incorrect size format '%s', should be WIDTHxHEIGHT" % size)
 
     try:
       width = int(size_components[0])
       height = int(size_components[1])
@@ skipping 190 matching lines @@
         host_config.clear_auth()
         host_config.clear_host_info()
         host_config.save()
         return 0
 
 
 if __name__ == "__main__":
   logging.basicConfig(level=logging.DEBUG,
                       format="%(asctime)s:%(levelname)s:%(message)s")
   sys.exit(main())