Implement resize-to-client for Xvfb.

BUG=145049


Review URL: https://chromiumcodereview.appspot.com/10996053

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@160239 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 399 insertions, 17 deletions

diff --git a/remoting/host/desktop_resizer_linux.cc b/remoting/host/desktop_resizer_linux.cc
index 66fe413..50f88a3 100644
--- a/remoting/host/desktop_resizer_linux.cc
+++ b/remoting/host/desktop_resizer_linux.cc
@@ -3,40 +3,290 @@
 // found in the LICENSE file.
 
 #include "remoting/host/desktop_resizer.h"
+#include "remoting/host/linux/x11_util.h"
 
+#include <string.h>
+#include <X11/extensions/Xrandr.h>
+#include <X11/Xlib.h>
+
+#include "base/command_line.h"
 #include "base/logging.h"
 
+// On Linux, we use the xrandr extension to change the desktop size. For now,
+// we only support resize-to-client for Xvfb-based servers that can match the
+// client size exactly. To support best-size matching, it would be necessary
+// to implement |GetSupportedSizes|, but it's not considered a priority now.
+//
+// Xrandr has a number of restrictions that make this code more complex:
+//
+//   1. It's not possible to change the size of an existing mode. Instead, the
+//      mode must be deleted and recreated.
+//   2. It's not possible to delete a mode that's in use.
+//   3. Errors are communicated via Xlib's spectacularly unhelpful mechanism
+//      of terminating the process unless you install an error handler.
+//
+// The basic approach is as follows:
+//
+//   1. Create a new mode with the correct size;
+//   2. Switch to the new mode;
+//   3. Delete the old mode.
+//
+// Since the new mode must have a different name, and we want the current mode
+// name to be consistent, we then additionally:
+//
+//   4. Recreate the old mode at the new size;
+//   5. Switch to the old mode;
+//   6. Delete the temporary mode.
+//
+// Name consistency will allow a future CL to disable resize-to-client if the
+// user has changed the mode to something other than "Chrome Remote Desktop
+// client size". It doesn't make the code significantly more complex.
+
 namespace remoting {
 
-namespace {
-class DesktopResizerLinux : public DesktopResizer {
+// Wrapper class for the XRRScreenResources struct.
+class ScreenResources {
  public:
-  DesktopResizerLinux() {
+  ScreenResources() : resources_(NULL) {
   }
 
-  virtual SkISize GetCurrentSize() OVERRIDE {
-    NOTIMPLEMENTED();
-    return SkISize::Make(0, 0);
+  ~ScreenResources() {
+    Release();
   }
 
-  virtual std::list<SkISize> GetSupportedSizes(
-      const SkISize& preferred) OVERRIDE {
-    NOTIMPLEMENTED();
-    return std::list<SkISize>();
+  bool Refresh(Display* display, Window window) {
+    Release();
+    resources_ = XRRGetScreenResources(display, window);
+    return resources_ != NULL;
+  }
+
+  void Release() {
+    if (resources_) {
+      XRRFreeScreenResources(resources_);
+      resources_ = NULL;
+    }
   }
 
-  virtual void SetSize(const SkISize& size) OVERRIDE {
-    NOTIMPLEMENTED();
+  RRMode GetIdForMode(const char* name) {
+    CHECK(resources_);
+    for (int i = 0; i < resources_->nmode; ++i) {
+      const XRRModeInfo& mode = resources_->modes[i];
+      if (strcmp(mode.name, name) == 0) {
+        return mode.id;
+      }
+    }
+    return 0;
   }
 
-  virtual void RestoreSize(const SkISize& original) OVERRIDE {
-    NOTIMPLEMENTED();
+  // For now, assume we're only ever interested in the first output.
+  RROutput GetOutput() {
+    CHECK(resources_);
+    return resources_->outputs[0];
   }
 
+  // For now, assume we're only ever interested in the first crtc.
+  RRCrtc GetCrtc() {
+    CHECK(resources_);
+    return resources_->crtcs[0];
+  }
+
+  XRROutputInfo* GetOutputInfo(Display* display, RROutput output_id) {
+    CHECK(resources_);
+    return XRRGetOutputInfo(display, resources_, output_id);
+  }
+
+  XRRScreenResources* get() { return resources_; }
+
+ private:
+  XRRScreenResources* resources_;
+};
+
+
+class DesktopResizerLinux : public DesktopResizer {
+ public:
+  DesktopResizerLinux();
+  virtual ~DesktopResizerLinux();
+
+  // DesktopResizer interface
+  virtual SkISize GetCurrentSize() OVERRIDE;
+  virtual std::list<SkISize> GetSupportedSizes(
+      const SkISize& preferred) OVERRIDE;
+  virtual void SetSize(const SkISize& size) OVERRIDE;
+  virtual void RestoreSize(const SkISize& original) OVERRIDE;
+
  private:
+  // Create a mode, and attach it to the primary output. If the mode already
+  // exists, it is left unchanged.
+  void CreateMode(const char* name, int width, int height);
+
+  // Remove the specified mode from the primary output, and delete it. If the
+  // mode is in use, it is not deleted.
+  void DeleteMode(const char* name);
+
+  // Switch the primary output to the specified mode. If name is NULL, the
+  // primary output is disabled instead, which is required before changing
+  // its size.
+  void SwitchToMode(const char* name);
+
+  Display* display_;
+  int screen_;
+  Window root_;
+  ScreenResources resources_;
+  bool exact_resize_;
+
   DISALLOW_COPY_AND_ASSIGN(DesktopResizerLinux);
 };
-}  // namespace
+
+DesktopResizerLinux::DesktopResizerLinux()
+    : display_(XOpenDisplay(NULL)),
+      screen_(DefaultScreen(display_)),
+      root_(RootWindow(display_, screen_)),
+      exact_resize_(CommandLine::ForCurrentProcess()->
+                    HasSwitch("server-supports-exact-resize")) {
+  XRRSelectInput (display_, root_, RRScreenChangeNotifyMask);
+}
+
+DesktopResizerLinux::~DesktopResizerLinux() {
+  XCloseDisplay(display_);
+}
+
+SkISize DesktopResizerLinux::GetCurrentSize() {
+  if (!exact_resize_) {
+    // TODO(jamiewalch): Remove this early return if we decide to support
+    // non-Xvfb servers.
+    return SkISize::Make(0, 0);
+  }
+
+  // TODO(lambroslambrou): Xrandr requires that we process RRScreenChangeNotify
+  // events, otherwise DisplayWidth and DisplayHeight do not return the current
+  // values. Normally, this would be done via a central X event loop, but we
+  // don't have one, hence this horrible hack.
+  //
+  // Note that the WatchFileDescriptor approach taken in XServerClipboard
+  // doesn't work here because resize events have already been read from the
+  // X server socket by the time the resize function returns, hence the
+  // file descriptor is never seen as readable.
+  while (XEventsQueued(display_, QueuedAlready)) {
+    XEvent event;
+    XNextEvent(display_, &event);
+    XRRUpdateConfiguration(&event);
+  }
+
+  SkISize result = SkISize::Make(
+      DisplayWidth (display_, DefaultScreen(display_)),
+      DisplayHeight(display_, DefaultScreen(display_)));
+  return result;
+}
+
+std::list<SkISize> DesktopResizerLinux::GetSupportedSizes(
+    const SkISize& preferred) {
+  std::list<SkISize> result;
+  if (exact_resize_) {
+    // Clamp the specified size to something valid for the X server.
+    int min_width = 0, min_height = 0, max_width = 0, max_height = 0;
+    XRRGetScreenSizeRange(display_, root_,
+                          &min_width, &min_height,
+                          &max_width, &max_height);
+    int width = std::min(std::max(preferred.width(), min_width), max_width);
+    int height = std::min(std::max(preferred.height(), min_height), max_height);
+    // Additionally impose a minimum size of 640x480, since anything smaller
+    // doesn't seem very useful.
+    SkISize actual = SkISize::Make(std::max(640, width), std::max(480, height));
+    result.push_back(actual);
+  } else {
+    // TODO(jamiewalch): Return the list of supported sizes if we can't support
+    // exact-size matching.
+  }
+  return result;
+}
+
+void DesktopResizerLinux::SetSize(const SkISize& size) {
+  if (!exact_resize_) {
+    // TODO(jamiewalch): Remove this early return if we decide to support
+    // non-Xvfb servers.
+    return;
+  }
+
+  // Ignore X errors encountered while resizing the display. We might hit an
+  // error, for example if xrandr has been used to add a mode with the same
+  // name as our temporary mode, or to remove the "client size" mode. We don't
+  // want to terminate the process if this happens.
+  ScopedXErrorHandler handler(ScopedXErrorHandler::Ignore());
+
+  // Grab the X server while we're changing the display size. This ensures
+  // that the display configuration doesn't change under our feet.
+  ScopedXGrabServer grabber(display_);
+
+  // The name of the mode representing the current client view size and the
+  // temporary mode used for the reasons described at the top of this file.
+  // The former should be localized if it's user-visible; the latter only
+  // exists briefly and does not need to localized.
+  const char* kModeName = "Chrome Remote Desktop client size";
+  const char* kTempModeName = "Chrome Remote Desktop temporary mode";
+
+  // Actually do the resize operation, preserving the current mode name. Note
+  // that we have to detach the output from any mode in order to resize it
+  // (strictly speaking, this is only required when reducing the size, but it
+  // seems safe to do it regardless).
+  LOG(INFO) << "Changing desktop size to " << size.width()
+            << "x" << size.height();
+  CreateMode(kTempModeName, size.width(), size.height());
+  SwitchToMode(NULL);
+  XRRSetScreenSize(display_, root_, size.width(), size.height(), -1, -1);
+  SwitchToMode(kTempModeName);
+  DeleteMode(kModeName);
+  CreateMode(kModeName, size.width(), size.height());
+  SwitchToMode(kModeName);
+  DeleteMode(kTempModeName);
+}
+
+void DesktopResizerLinux::RestoreSize(const SkISize& original) {
+  // Since the desktop is only visible via a remote connection, the original
+  // size of the desktop will never been seen and there's no point restoring
+  // it; if we did, we'd just risk messing up the user's window layout.
+}
+
+void DesktopResizerLinux::CreateMode(const char* name, int width, int height) {
+  XRRModeInfo mode;
+  memset(&mode, 0, sizeof(mode));
+  mode.width = width;
+  mode.height = height;
+  mode.name = const_cast<char*>(name);
+  mode.nameLength = strlen(name);
+  XRRCreateMode(display_, root_, &mode);
+
+  if (!resources_.Refresh(display_, root_)) {
+    return;
+  }
+  RRMode mode_id = resources_.GetIdForMode(name);
+  if (!mode_id) {
+    return;
+  }
+  XRRAddOutputMode(display_, resources_.GetOutput(), mode_id);
+}
+
+void DesktopResizerLinux::DeleteMode(const char* name) {
+  RRMode mode_id = resources_.GetIdForMode(name);
+  if (mode_id) {
+    XRRDeleteOutputMode(display_, resources_.GetOutput(), mode_id);
+    XRRDestroyMode(display_, mode_id);
+    resources_.Refresh(display_, root_);
+  }
+}
+
+void DesktopResizerLinux::SwitchToMode(const char* name) {
+  RRMode mode_id = None;
+  RROutput* outputs = NULL;
+  int number_of_outputs = 0;
+  if (name) {
+    mode_id = resources_.GetIdForMode(name);
+    CHECK(mode_id);
+    outputs = resources_.get()->outputs;
+    number_of_outputs = resources_.get()->noutput;
+  }
+  XRRSetCrtcConfig(display_, resources_.get(), resources_.GetCrtc(),
+                   CurrentTime, 0, 0, mode_id, 1, outputs, number_of_outputs);
+}
 
 scoped_ptr<DesktopResizer> DesktopResizer::Create() {
   return scoped_ptr<DesktopResizer>(new DesktopResizerLinux);
diff --git a/remoting/host/linux/x11_util.cc b/remoting/host/linux/x11_util.cc
new file mode 100644
index 0000000..88935e2
--- /dev/null
+++ b/remoting/host/linux/x11_util.cc
@@ -0,0 +1,57 @@
+// 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.
+
+#include "remoting/host/linux/x11_util.h"
+
+#include "base/bind.h"
+
+namespace remoting {
+
+static ScopedXErrorHandler* g_handler = NULL;
+
+ScopedXErrorHandler::ScopedXErrorHandler(const Handler& handler):
+    handler_(handler),
+    ok_(true) {
+  // This is a poor-man's check for incorrect usage. It doesn't handle the case
+  // where a mix of ScopedXErrorHandler and raw XSetErrorHandler calls are used,
+  // and it disallows nested ScopedXErrorHandlers on the same thread, despite
+  // these being perfectly safe.
+  DCHECK(g_handler == NULL);
+  g_handler = this;
+  previous_handler_ = XSetErrorHandler(HandleXErrors);
+}
+
+ScopedXErrorHandler::~ScopedXErrorHandler() {
+  g_handler = NULL;
+  XSetErrorHandler(previous_handler_);
+}
+
+namespace {
+void IgnoreXErrors(Display* display, XErrorEvent* error) {}
+}  // namespace
+
+// Static
+ScopedXErrorHandler::Handler ScopedXErrorHandler::Ignore() {
+  return base::Bind(IgnoreXErrors);
+}
+
+int ScopedXErrorHandler::HandleXErrors(Display* display, XErrorEvent* error) {
+  DCHECK(g_handler != NULL);
+  g_handler->ok_ = false;
+  g_handler->handler_.Run(display, error);
+  return 0;
+}
+
+
+ScopedXGrabServer::ScopedXGrabServer(Display* display)
+    : display_(display) {
+  XGrabServer(display_);
+}
+
+ScopedXGrabServer::~ScopedXGrabServer() {
+  XUngrabServer(display_);
+  XFlush(display_);
+}
+
+}  // namespace remoting
diff --git a/remoting/host/linux/x11_util.h b/remoting/host/linux/x11_util.h
new file mode 100644
index 0000000..af9ee31
--- /dev/null
+++ b/remoting/host/linux/x11_util.h
@@ -0,0 +1,63 @@
+// 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.
+
+#ifndef REMOTING_HOST_X11_UTILS_H_
+#define REMOTING_HOST_X11_UTILS_H_
+
+// Xlib.h defines XErrorEvent as an anonymous struct, so we can't forward-
+// declare it in this header. Since Xlib.h is not generally something you
+// should #include into arbitrary code, please refrain from #including this
+// header in another header.
+#include <X11/Xlib.h>
+
+#include "base/callback.h"
+
+namespace remoting {
+
+// Temporarily install an alternative handler for X errors. The default handler
+// exits the process, which is not what we want.
+//
+// Note that X error handlers are global, which means that this class is not
+// thread safe.
+class ScopedXErrorHandler {
+ public:
+  typedef base::Callback<void(Display*, XErrorEvent*)> Handler;
+
+  explicit ScopedXErrorHandler(const Handler& handler);
+  ~ScopedXErrorHandler();
+
+  // Return false if any X errors have been encountered in the scope of this
+  // handler.
+  bool ok() const { return ok_; }
+
+  // Basic handler that ignores X errors.
+  static Handler Ignore();
+
+ private:
+  static int HandleXErrors(Display* display, XErrorEvent* error);
+
+  Handler handler_;
+  int (*previous_handler_)(Display*, XErrorEvent*);
+  bool ok_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedXErrorHandler);
+};
+
+
+// Grab/release the X server within a scope. This can help avoid race
+// conditions that would otherwise lead to X errors.
+class ScopedXGrabServer {
+ public:
+  ScopedXGrabServer(Display* display);
+  ~ScopedXGrabServer();
+
+ private:
+  Display* display_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedXGrabServer);
+};
+
+}  // namespace remoting
+
+#endif  // REMOTING_HOST_X11_UTILS_H_
diff --git a/remoting/remoting.gyp b/remoting/remoting.gyp
index bb0deb8..eab1d26 100644
--- a/remoting/remoting.gyp
+++ b/remoting/remoting.gyp
@@ -1376,6 +1376,8 @@
         'host/it2me_host_user_interface.h',
         'host/json_host_config.cc',
         'host/json_host_config.h',
+        'host/linux/x11_util.cc',
+        'host/linux/x11_util.h',
         'host/linux/x_server_clipboard.cc',
         'host/linux/x_server_clipboard.h',
         'host/linux/x_server_pixel_buffer.cc',
diff --git a/remoting/tools/me2me_virtual_host.py b/remoting/tools/me2me_virtual_host.py
index 7a6565c..4787372 100755
--- a/remoting/tools/me2me_virtual_host.py
+++ b/remoting/tools/me2me_virtual_host.py
@@ -174,6 +174,7 @@ class Desktop:
     self.child_env = None
     self.sizes = sizes
     self.pulseaudio_pipe = None
+    self.server_supports_exact_resize = False
     g_desktops.append(self)
 
   @staticmethod
@@ -259,9 +260,16 @@ class Desktop:
     max_height = max([height for width, height in self.sizes])
 
     try:
-      xvfb = locate_executable("Xvfb-randr")
+      # TODO(jamiewalch): This script expects to be installed alongside
+      # Xvfb-randr, but that's no longer the case. Fix this once we have
+      # a Xvfb-randr package that installs somewhere sensible.
+      xvfb = "/usr/bin/Xvfb-randr"
+      if not os.path.exists(xvfb):
+        xvfb = locate_executable("Xvfb-randr")
+      self.server_supports_exact_resize = True
     except Exception:
       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)
@@ -353,6 +361,8 @@ class Desktop:
     args = [locate_executable(HOST_BINARY_NAME), "--host-config=/dev/stdin"]
     if self.pulseaudio_pipe:
       args.append("--audio-pipe-name=%s" % self.pulseaudio_pipe)
+    if self.server_supports_exact_resize:
+      args.append("--server-supports-exact-resize")
     self.host_proc = subprocess.Popen(args, env=self.child_env,
                                       stdin=subprocess.PIPE)
     logging.info(args)
@@ -737,7 +747,7 @@ def waitpid_handle_exceptions(pid, deadline):
 
 
 def main():
-  DEFAULT_SIZE = "1280x800"
+  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"""