Make setScreenLayout() asynchronous

Allows more flexibility in how things are implemented and connected.

common/rfb/SDesktop.h

@@ -63,11 +63,8 @@ namespace rfb {
 
     // setScreenLayout() requests to reconfigure the framebuffer and/or
     // the layout of screens.
-    virtual unsigned int setScreenLayout(int /*fb_width*/,
-                                         int /*fb_height*/,
-                                         const ScreenSet& /*layout*/) {
-      return resultProhibited;
-    }
+    virtual void setScreenLayout(int fb_width, int fb_height,
+                                 const ScreenSet& layout) = 0;
 
   protected:
     virtual ~SDesktop() {}

common/rfb/VNCServer.h

@@ -87,6 +87,16 @@ namespace rfb {
     // the pixelbuffer. Clients will be notified of the new layout.
     virtual void setScreenLayout(const ScreenSet& layout) = 0;
 
+    // acceptScreenLayout() accepts a request by a client to change the
+    // screen layout and informs both the requesting client, and any
+    // other connected clients.
+    virtual void acceptScreenLayout(int fb_width, int fb_height,
+                                    const ScreenSet& layout) = 0;
+
+    // rejectScreenLayout() rejects a request by a client to change the
+    // screen layout.
+    virtual void rejectScreenLayout(unsigned int reason) = 0;
+
     // getPixelBuffer() returns a pointer to the PixelBuffer object.
     virtual const PixelBuffer* getPixelBuffer() const = 0;
 

common/rfb/VNCServerST.cxx

@@ -87,7 +87,7 @@ VNCServerST::VNCServerST(const char* name_, SDesktop* desktop_)
   : blHosts(&blacklist), desktop(desktop_), desktopStarted(false),
     blockCounter(0), pb(nullptr), ledState(ledUnknown),
     name(name_), pointerClient(nullptr), clipboardClient(nullptr),
-    pointerClientTime(0), comparer(nullptr),
+    pointerClientTime(0), layoutClient(nullptr), comparer(nullptr),
     cursor(new Cursor(0, 0, core::Point(), nullptr)),
     renderedCursorInvalid(false),
     keyRemapper(&KeyRemapper::defInstance),
@@ -220,6 +220,8 @@ void VNCServerST::removeSocket(network::Socket* sock) {
       if (clipboardClient == *ci)
         handleClipboardAnnounce(*ci, "", false);
       clipboardRequestors.remove(*ci);
+      if (layoutClient == *ci)
+        layoutClient = nullptr;
 
       std::string peer((*ci)->getPeerEndpoint());
 
@@ -389,6 +391,45 @@ void VNCServerST::setScreenLayout(const ScreenSet& layout)
     (*ci)->screenLayoutChangeOrClose(reasonServer);
 }
 
+void VNCServerST::acceptScreenLayout(int fb_width, int fb_height,
+                                     const ScreenSet& layout)
+{
+  if (layoutClient == nullptr) {
+    slog.debug("Unexpected or late acception of screen layout");
+    return;
+  }
+
+  // Sanity check
+  if (!pb)
+    throw Exception("acceptScreenLayout: no PixelBuffer");
+  if ((fb_width != pb->width()) || (fb_height != pb->height()))
+    throw Exception("Desktop configured a different screen layout than requested");
+  if (screenLayout != layout)
+    throw Exception("Desktop configured a different screen layout than requested");
+
+  // Notify other clients
+  std::list<VNCSConnectionST*>::iterator ci;
+  for (ci = clients.begin(); ci != clients.end(); ++ci) {
+    if ((*ci) == layoutClient)
+      continue;
+    (*ci)->screenLayoutChangeOrClose(reasonOtherClient);
+  }
+
+  layoutClient->screenLayoutChangeOrClose(reasonClient);
+  layoutClient = nullptr;
+}
+
+void VNCServerST::rejectScreenLayout(unsigned int reason)
+{
+  if (layoutClient == nullptr) {
+    slog.debug("Unexpected or late rejection of screen layout");
+    return;
+  }
+
+  layoutClient->setDesktopSizeFailureOrClose(reason);
+  layoutClient = nullptr;
+}
+
 void VNCServerST::requestClipboard()
 {
   if (!rfb::Server::acceptCutText)
@@ -585,7 +626,6 @@ void VNCServerST::setDesktopSize(VNCSConnectionST* requester,
                                  int fb_width, int fb_height,
                                  const ScreenSet& layout)
 {
-  unsigned int result;
   std::list<VNCSConnectionST*>::iterator ci;
 
   if (!rfb::Server::acceptSetDesktopSize) {
@@ -608,27 +648,20 @@ void VNCServerST::setDesktopSize(VNCSConnectionST* requester,
     return;
   }
 
-  // FIXME: the desktop will call back to VNCServerST and an extra set
-  // of ExtendedDesktopSize messages will be sent. This is okay
-  // protocol-wise, but unnecessary.
-  result = desktop->setScreenLayout(fb_width, fb_height, layout);
-  if (result != resultSuccess) {
-    requester->setDesktopSizeFailureOrClose(result);
+  // We don't bother with a queue, so just reject the second client if
+  // we happen to have overlapping requests
+  if (layoutClient != nullptr) {
+    slog.error("Rejecting concurrent screen layout requests");
+    requester->setDesktopSizeFailureOrClose(resultProhibited);
     return;
   }
 
-  // Sanity check
-  if (screenLayout != layout)
-    throw Exception("Desktop configured a different screen layout than requested");
-
-  // Notify other clients
-  for (ci = clients.begin(); ci != clients.end(); ++ci) {
-    if ((*ci) == requester)
-      continue;
-    (*ci)->screenLayoutChangeOrClose(reasonOtherClient);
-  }
+  layoutClient = requester;
 
-  requester->screenLayoutChangeOrClose(reasonClient);
+  // FIXME: the desktop will call back to VNCServerST and an extra set
+  // of ExtendedDesktopSize messages will be sent. This is okay
+  // protocol-wise, but unnecessary.
+  desktop->setScreenLayout(fb_width, fb_height, layout);
 }
 
 // Other public methods

common/rfb/VNCServerST.h

@@ -83,6 +83,9 @@ namespace rfb {
     void setPixelBuffer(PixelBuffer* pb, const ScreenSet& layout) override;
     void setPixelBuffer(PixelBuffer* pb) override;
     void setScreenLayout(const ScreenSet& layout) override;
+    void acceptScreenLayout(int fb_width, int fb_height,
+                            const ScreenSet& layout) override;
+    void rejectScreenLayout(unsigned int reason) override;
     const PixelBuffer* getPixelBuffer() const override { return pb; }
 
     void requestClipboard() override;
@@ -201,6 +204,8 @@ namespace rfb {
 
     time_t pointerClientTime;
 
+    VNCSConnectionST* layoutClient;
+
     ComparingUpdateTracker* comparer;
 
     core::Point cursorPos;

unix/x0vncserver/XDesktop.cxx

@@ -685,14 +685,15 @@ static void GetSmallerMode(XRRScreenResources *res,
 }
 #endif /* HAVE_XRANDR */
 
-unsigned int XDesktop::setScreenLayout(int fb_width, int fb_height,
-                                       const rfb::ScreenSet& layout)
+void XDesktop::setScreenLayout(int fb_width, int fb_height,
+                               const rfb::ScreenSet& layout)
 {
 #ifdef HAVE_XRANDR
   XRRScreenResources *res = XRRGetScreenResources(dpy, DefaultRootWindow(dpy));
   if (!res) {
     vlog.error("XRRGetScreenResources failed");
-    return rfb::resultProhibited;
+    server->rejectScreenLayout(rfb::resultProhibited);
+    return;
   }
   vncSetGlueContext(dpy, res);
 
@@ -739,26 +740,30 @@ unsigned int XDesktop::setScreenLayout(int fb_width, int fb_height,
     if (outputId == None) {
       vlog.debug("Resize adjust: Could not find corresponding screen");
       XRRFreeScreenResources(res);
-      return rfb::resultInvalid;
+      server->rejectScreenLayout(rfb::resultInvalid);
+      return;
     }
     XRROutputInfo *output = XRRGetOutputInfo(dpy, res, outputId);
     if (!output) {
       vlog.debug("Resize adjust: XRRGetOutputInfo failed");
       XRRFreeScreenResources(res);
-      return rfb::resultInvalid;
+      server->rejectScreenLayout(rfb::resultInvalid);
+      return;
     }
     if (!output->crtc) {
       vlog.debug("Resize adjust: Selected output has no CRTC");
       XRRFreeScreenResources(res);
       XRRFreeOutputInfo(output);
-      return rfb::resultInvalid;
+      server->rejectScreenLayout(rfb::resultInvalid);
+      return;
     }
     XRRCrtcInfo *crtc = XRRGetCrtcInfo(dpy, res, output->crtc);
     if (!crtc) {
       vlog.debug("Resize adjust: XRRGetCrtcInfo failed");
       XRRFreeScreenResources(res);
       XRRFreeOutputInfo(output);
-      return rfb::resultInvalid;
+      server->rejectScreenLayout(rfb::resultInvalid);
+      return;
     }
 
     unsigned int swidth = iter->dimensions.width();
@@ -795,7 +800,8 @@ unsigned int XDesktop::setScreenLayout(int fb_width, int fb_height,
     } else {
       vlog.error("Failed to find smaller or equal screen size");
       XRRFreeScreenResources(res);
-      return rfb::resultInvalid;
+      server->rejectScreenLayout(rfb::resultInvalid);
+      return;
     }
   }
 
@@ -825,20 +831,25 @@ unsigned int XDesktop::setScreenLayout(int fb_width, int fb_height,
      VNCSConnectionST::setDesktopSize. Another ExtendedDesktopSize
      with reason=0 will be sent in response to the changes seen by the
      event handler. */
-  if (adjustedLayout != layout)
-    return rfb::resultInvalid;
+  if (adjustedLayout != layout) {
+    server->rejectScreenLayout(rfb::resultInvalid);
+    return;
+  }
 
   // Explicitly update the server state with the result as there
   // can be corner cases where we don't get feedback from the X server
   server->setScreenLayout(computeScreenLayout());
 
-  return ret;
+  if (ret == rfb::resultSuccess)
+    server->acceptScreenLayout(fb_width, fb_height, layout);
+  else
+    server->rejectScreenLayout(rfb::resultInvalid);
 
 #else
   (void)fb_width;
   (void)fb_height;
   (void)layout;
-  return rfb::resultProhibited;
+  server->rejectScreenLayout(rfb::resultProhibited);
 #endif /* HAVE_XRANDR */
 }
 

unix/x0vncserver/XDesktop.h

@@ -57,8 +57,8 @@ class XDesktop : public rfb::SDesktop,
   bool isRunning();
   void queryConnection(network::Socket* sock,
                        const char* userName) override;
-  unsigned int setScreenLayout(int fb_width, int fb_height,
-                               const rfb::ScreenSet& layout) override;
+  void setScreenLayout(int fb_width, int fb_height,
+                       const rfb::ScreenSet& layout) override;
 
   // -=- TXGlobalEventHandler interface
   bool handleGlobalEvent(XEvent* ev) override;

unix/xserver/hw/vnc/XserverDesktop.cc

@@ -481,8 +481,8 @@ void XserverDesktop::pointerEvent(rfb::VNCServerST*, const char*,
   vncPointerButtonAction(event.buttonMask);
 }
 
-unsigned int XserverDesktop::setScreenLayout(int fb_width, int fb_height,
-                                             const rfb::ScreenSet& layout)
+void XserverDesktop::setScreenLayout(int fb_width, int fb_height,
+                                     const rfb::ScreenSet& layout)
 {
   unsigned int result;
 
@@ -493,7 +493,10 @@ unsigned int XserverDesktop::setScreenLayout(int fb_width, int fb_height,
   // can be corner cases where we don't get feedback from the X core
   refreshScreenLayout();
 
-  return result;
+  if (result == rfb::resultSuccess)
+    server->acceptScreenLayout(fb_width, fb_height, layout);
+  else
+    server->rejectScreenLayout(result);
 }
 
 void XserverDesktop::frameTick(rfb::VNCServerST*, const char*)

unix/xserver/hw/vnc/XserverDesktop.h

@@ -95,8 +95,8 @@ class XserverDesktop : public rfb::SDesktop,
   void init(rfb::VNCServer* vs) override;
   void queryConnection(network::Socket* sock,
                        const char* userName) override;
-  unsigned int setScreenLayout(int fb_width, int fb_height,
-                               const rfb::ScreenSet& layout) override;
+  void setScreenLayout(int fb_width, int fb_height,
+                       const rfb::ScreenSet& layout) override;
 
   // rfb::PixelBuffer callbacks
   void grabRegion(const core::Region& r) override;

win/rfb_win32/SDisplay.cxx

@@ -180,6 +180,14 @@ void SDisplay::queryConnection(network::Socket* sock,
 }
 
 
+void SDisplay::setScreenLayout(int /*fb_width*/, int /*fb_height*/,
+                               const ScreenSet& /*layout*/)
+{
+  assert(server != nullptr);
+  server->rejectScreenLayout(rfb::resultProhibited);
+}
+
+
 void SDisplay::startCore() {
 
   // Currently, we just check whether we're in the console session, and

win/rfb_win32/SDisplay.h

@@ -74,6 +74,8 @@ namespace rfb {
       void init(VNCServer* vs) override;
       void queryConnection(network::Socket* sock,
                            const char* userName) override;
+      void setScreenLayout(int fb_width, int fb_height,
+                           const ScreenSet& layout) override;
 
       // -=- Clipboard events