Ignore resize observation caused by server resizesresizeObserverScrollbars

If we increase the remote screen size from the server in such a way that
it no longer fits the browser window, the browser will probably want to
show scrollbars. The same happens if you enable 'clipping' while the
remote is larger than the browser window. These scrollbars do, in turn,
decrease the available space in the browser window. This causes our
ResizeObserver to trigger.

If the resize observation triggers a requestRemoteResize() we will
overwrite the size and request a new one just because scrollbars have
appeared. We don't want that.

We can save the expected client size after resizing, and then compare
the current client size with the expected one. If there is no change
compared to the expected size, we shouldn't send the request.

Fixes issue #1616.

2 files changed, 113 insertions, 2 deletions

diff --git a/core/rfb.js b/core/rfb.js
index fe9939f..970e03f 100644
--- a/core/rfb.js
+++ b/core/rfb.js
@@ -244,6 +244,8 @@ export default class RFB extends EventTargetMixin {
         this._sock.on('message', this._handleMessage.bind(this));
         this._sock.on('error', this._socketError.bind(this));
 
+        this._expectedClientWidth = null;
+        this._expectedClientHeight = null;
         this._resizeObserver = new ResizeObserver(this._eventHandlers.handleResize);
 
         // All prepared, kick off the connection
@@ -623,7 +625,26 @@ export default class RFB extends EventTargetMixin {
             { detail: { name: this._fbName } }));
     }
 
+    _saveExpectedClientSize() {
+        this._expectedClientWidth = this._screen.clientWidth;
+        this._expectedClientHeight = this._screen.clientHeight;
+    }
+
+    _currentClientSize() {
+        return [this._screen.clientWidth, this._screen.clientHeight];
+    }
+
+    _clientHasExpectedSize() {
+        const [currentWidth, currentHeight] = this._currentClientSize();
+        return currentWidth == this._expectedClientWidth &&
+            currentHeight == this._expectedClientHeight;
+    }
+
     _handleResize() {
+        // Don't change anything if the client size is already as expected
+        if (this._clientHasExpectedSize()) {
+            return;
+        }
         // If the window resized then our screen element might have
         // as well. Update the viewport dimensions.
         window.requestAnimationFrame(() => {
@@ -664,6 +685,12 @@ export default class RFB extends EventTargetMixin {
             this._display.viewportChangeSize(size.w, size.h);
             this._fixScrollbars();
         }
+
+        // When changing clipping we might show or hide scrollbars.
+        // This causes the expected client dimensions to change.
+        if (curClip !== newClip) {
+            this._saveExpectedClientSize();
+        }
     }
 
     _updateScale() {
@@ -688,6 +715,7 @@ export default class RFB extends EventTargetMixin {
         }
 
         const size = this._screenSize();
+
         RFB.messages.setDesktopSize(this._sock,
                                     Math.floor(size.w), Math.floor(size.h),
                                     this._screenID, this._screenFlags);
@@ -2507,6 +2535,9 @@ export default class RFB extends EventTargetMixin {
         this._updateScale();
 
         this._updateContinuousUpdates();
+
+        // Keep this size until browser client size changes
+        this._saveExpectedClientSize();
     }
 
     _xvpOp(ver, op) {
diff --git a/tests/test.rfb.js b/tests/test.rfb.js
index 1d806e7..c3aa74e 100644
--- a/tests/test.rfb.js
+++ b/tests/test.rfb.js
@@ -540,6 +540,10 @@ describe('Remote Frame Buffer Protocol Client', function () {
             sinon.spy(client._display, "viewportChangeSize");
 
             client._sock._websocket._receiveData(new Uint8Array(incoming));
+            // The resize will cause scrollbars on the container, this causes a
+            // resize observation in the browsers
+            fakeResizeObserver.fire();
+            clock.tick(1000);
 
             // FIXME: Display implicitly calls viewportChangeSize() when
             //        resizing the framebuffer, hence calledTwice.
@@ -571,6 +575,32 @@ describe('Remote Frame Buffer Protocol Client', function () {
             expect(client._display.viewportChangeSize).to.not.have.been.called;
         });
 
+        describe('Clipping and remote resize', function () {
+            beforeEach(function () {
+                // Given a remote (100, 100) larger than the container (70x80),
+                client._resize(100, 100);
+                client._supportsSetDesktopSize = true;
+                client.resizeSession = true;
+                sinon.spy(RFB.messages, "setDesktopSize");
+            });
+            afterEach(function () {
+                RFB.messages.setDesktopSize.restore();
+            });
+            it('should not change remote size when changing clipping', function () {
+                // When changing clipping the scrollbars of the container
+                // will appear and disappear and thus trigger resize observations
+                client.clipViewport = false;
+                fakeResizeObserver.fire();
+                clock.tick(1000);
+                client.clipViewport = true;
+                fakeResizeObserver.fire();
+                clock.tick(1000);
+
+                // Then no resize requests should be sent
+                expect(RFB.messages.setDesktopSize).to.not.have.been.called;
+            });
+        });
+
         describe('Dragging', function () {
             beforeEach(function () {
                 client.dragViewport = true;
@@ -734,6 +764,10 @@ describe('Remote Frame Buffer Protocol Client', function () {
             sinon.spy(client._display, "autoscale");
 
             client._sock._websocket._receiveData(new Uint8Array(incoming));
+            // The resize will cause scrollbars on the container, this causes a
+            // resize observation in the browsers
+            fakeResizeObserver.fire();
+            clock.tick(1000);
 
             expect(client._display.autoscale).to.have.been.calledOnce;
             expect(client._display.autoscale).to.have.been.calledWith(70, 80);
@@ -830,6 +864,35 @@ describe('Remote Frame Buffer Protocol Client', function () {
             expect(RFB.messages.setDesktopSize).to.have.been.calledWith(sinon.match.object, 40, 50, 0, 0);
         });
 
+        it('should not request the same size twice', function () {
+            container.style.width = '40px';
+            container.style.height = '50px';
+            fakeResizeObserver.fire();
+            clock.tick(1000);
+
+            expect(RFB.messages.setDesktopSize).to.have.been.calledOnce;
+            expect(RFB.messages.setDesktopSize).to.have.been.calledWith(
+                sinon.match.object, 40, 50, 0, 0);
+
+            // Server responds with the requested size 40x50
+            const incoming = [ 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00,
+                               0x00, 0x28, 0x00, 0x32, 0xff, 0xff, 0xfe, 0xcc,
+                               0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                               0x00, 0x00, 0x00, 0x00, 0x00, 0x28, 0x00, 0x32,
+                               0x00, 0x00, 0x00, 0x00];
+
+            client._sock._websocket._receiveData(new Uint8Array(incoming));
+            clock.tick(1000);
+
+            RFB.messages.setDesktopSize.resetHistory();
+
+            // size is still 40x50
+            fakeResizeObserver.fire();
+            clock.tick(1000);
+
+            expect(RFB.messages.setDesktopSize).to.not.have.been.called;
+        });
+
         it('should not resize until the container size is stable', function () {
             container.style.width = '20px';
             container.style.height = '30px';
@@ -885,16 +948,33 @@ describe('Remote Frame Buffer Protocol Client', function () {
         });
 
         it('should not try to override a server resize', function () {
-            // Simple ExtendedDesktopSize FBU message
+            // Simple ExtendedDesktopSize FBU message, new size: 100x100
             const incoming = [ 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00,
-                               0x00, 0x04, 0x00, 0x04, 0xff, 0xff, 0xfe, 0xcc,
+                               0x00, 0x64, 0x00, 0x64, 0xff, 0xff, 0xfe, 0xcc,
                                0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
                                0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x04,
                                0x00, 0x00, 0x00, 0x00 ];
 
+            // Note that this will cause the browser to display scrollbars
+            // since the framebuffer is 100x100 and the container is 70x80.
+            // The usable space (clientWidth/clientHeight) will be even smaller
+            // due to the scrollbars taking up space.
             client._sock._websocket._receiveData(new Uint8Array(incoming));
+            // The scrollbars cause the ResizeObserver to fire
+            fakeResizeObserver.fire();
+            clock.tick(1000);
 
             expect(RFB.messages.setDesktopSize).to.not.have.been.called;
+
+            // An actual size change must not be ignored afterwards
+            container.style.width = '120px';
+            container.style.height = '130px';
+            fakeResizeObserver.fire();
+            clock.tick(1000);
+
+            expect(RFB.messages.setDesktopSize).to.have.been.calledOnce;
+            expect(RFB.messages.setDesktopSize.firstCall.args[1]).to.equal(120);
+            expect(RFB.messages.setDesktopSize.firstCall.args[2]).to.equal(130);
         });
     });