Increase default maximum desktop size and make it configurable.

remoting/tools/me2me_virtual_host.py

Index: remoting/tools/me2me_virtual_host.py
diff --git a/remoting/tools/me2me_virtual_host.py b/remoting/tools/me2me_virtual_host.py
index 57688f2404d6e8e3a5342f1a025bb8480188dc7b..cfcd635863984df7c2437d2f13cda1eb58ec0287 100755
--- a/remoting/tools/me2me_virtual_host.py
+++ b/remoting/tools/me2me_virtual_host.py
@@ -34,6 +34,7 @@ import uuid
 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]
 
@@ -748,7 +749,6 @@ def waitpid_handle_exceptions(pid, deadline):
 
 
 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"""
@@ -756,10 +756,9 @@ Web Store: https://chrome.google.com/remotedesktop"""
       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.")
@@ -831,9 +830,16 @@ Web Store: https://chrome.google.com/remotedesktop"""
     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: