From 0b5e968e14075519c8ef6ff7201f3355b4536d04 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Wed, 5 Feb 2025 16:33:13 +0100 Subject: [PATCH] Better resize rate limiting Be more aggressive with resizing, limiting it to once ever 100 ms instead of after a 500 ms idle period. This gives a more responsive user experience. --- core/rfb.js | 55 ++++++++++++++++------ tests/test.rfb.js | 116 +++++++++++++++++++++++++++++++++++++++------- 2 files changed, 140 insertions(+), 31 deletions(-) diff --git a/core/rfb.js b/core/rfb.js index 57f025814..e3266cc8d 100644 --- a/core/rfb.js +++ b/core/rfb.js @@ -149,6 +149,8 @@ export default class RFB extends EventTargetMixin { this._supportsSetDesktopSize = false; this._screenID = 0; this._screenFlags = 0; + this._pendingRemoteResize = false; + this._lastResize = 0; this._qemuExtKeyEventSupported = false; @@ -736,15 +738,9 @@ export default class RFB extends EventTargetMixin { this._saveExpectedClientSize(); }); - if (this._resizeSession) { - // Request changing the resolution of the remote display to - // the size of the local browser viewport. - - // In order to not send multiple requests before the browser-resize - // is finished we wait 0.5 seconds before sending the request. - clearTimeout(this._resizeTimeout); - this._resizeTimeout = setTimeout(this._requestRemoteResize.bind(this), 500); - } + // Request changing the resolution of the remote display to + // the size of the local browser viewport. + this._requestRemoteResize(); } // Update state of clipping in Display object, and make sure the @@ -794,16 +790,39 @@ export default class RFB extends EventTargetMixin { // Requests a change of remote desktop size. This message is an extension // and may only be sent if we have received an ExtendedDesktopSize message _requestRemoteResize() { - clearTimeout(this._resizeTimeout); - this._resizeTimeout = null; + if (!this._resizeSession) { + return; + } + if (this._viewOnly) { + return; + } + if (!this._supportsSetDesktopSize) { + return; + } + + // Rate limit to one pending resize at a time + if (this._pendingRemoteResize) { + return; + } - if (!this._resizeSession || this._viewOnly || - !this._supportsSetDesktopSize) { + // And no more than once every 100ms + if ((Date.now() - this._lastResize) < 100) { + clearTimeout(this._resizeTimeout); + this._resizeTimeout = setTimeout(this._requestRemoteResize.bind(this), + 100 - (Date.now() - this._lastResize)); return; } + this._resizeTimeout = null; const size = this._screenSize(); + // Do we actually change anything? + if (size.w === this._fbWidth && size.h === this._fbHeight) { + return; + } + + this._pendingRemoteResize = true; + this._lastResize = Date.now(); RFB.messages.setDesktopSize(this._sock, Math.floor(size.w), Math.floor(size.h), this._screenID, this._screenFlags); @@ -2913,6 +2932,10 @@ export default class RFB extends EventTargetMixin { * 2 - another client requested the resize */ + if (this._FBU.x === 1) { + this._pendingRemoteResize = false; + } + // We need to handle errors when we requested the resize. if (this._FBU.x === 1 && this._FBU.y !== 0) { let msg = ""; @@ -2945,6 +2968,12 @@ export default class RFB extends EventTargetMixin { this._requestRemoteResize(); } + if (this._FBU.x === 1 && this._FBU.y === 0) { + // We might have resized again whilst waiting for the + // previous request, so check if we are in sync + this._requestRemoteResize(); + } + return true; } diff --git a/tests/test.rfb.js b/tests/test.rfb.js index 9e792704d..2a7bbeaab 100644 --- a/tests/test.rfb.js +++ b/tests/test.rfb.js @@ -680,7 +680,6 @@ describe('Remote Frame Buffer protocol client', function () { // 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. @@ -1042,7 +1041,6 @@ describe('Remote Frame Buffer protocol client', function () { // 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); @@ -1079,6 +1077,7 @@ describe('Remote Frame Buffer protocol client', function () { let height = RFB.messages.setDesktopSize.args[0][2]; sendExtendedDesktopSize(client, 1, 0, width, height, 0x7890abcd, 0x12345678); RFB.messages.setDesktopSize.resetHistory(); + clock.tick(10000); } }); @@ -1093,7 +1092,6 @@ describe('Remote Frame Buffer protocol client', function () { container.style.width = '40px'; container.style.height = '50px'; fakeResizeObserver.fire(); - clock.tick(1000); expect(RFB.messages.setDesktopSize).to.not.have.been.called; client.resizeSession = true; @@ -1132,7 +1130,6 @@ describe('Remote Frame Buffer protocol client', 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( @@ -1143,7 +1140,6 @@ describe('Remote Frame Buffer protocol client', 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( @@ -1151,13 +1147,12 @@ describe('Remote Frame Buffer protocol client', function () { // Server responds with the requested size 40x50 sendExtendedDesktopSize(client, 1, 0, 40, 50, 0x7890abcd, 0x12345678); - clock.tick(1000); RFB.messages.setDesktopSize.resetHistory(); // size is still 40x50 - fakeResizeObserver.fire(); clock.tick(1000); + fakeResizeObserver.fire(); expect(RFB.messages.setDesktopSize).to.not.have.been.called; }); @@ -1166,7 +1161,6 @@ describe('Remote Frame Buffer protocol client', 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( @@ -1175,45 +1169,135 @@ describe('Remote Frame Buffer protocol client', function () { sendExtendedDesktopSize(client, 1, 0, 40, 50, 0x7890abcd, 0x12345678); RFB.messages.setDesktopSize.resetHistory(); + clock.tick(1000); container.style.width = '70px'; container.style.height = '80px'; fakeResizeObserver.fire(); - clock.tick(1000); expect(RFB.messages.setDesktopSize).to.have.been.calledOnce; expect(RFB.messages.setDesktopSize).to.have.been.calledWith( sinon.match.object, 70, 80, 0x7890abcd, 0x12345678); }); - it('should not resize until the container size is stable', function () { + it('should rate limit resizes', function () { container.style.width = '20px'; container.style.height = '30px'; fakeResizeObserver.fire(); - clock.tick(400); + + expect(RFB.messages.setDesktopSize).to.have.been.calledOnce; + expect(RFB.messages.setDesktopSize).to.have.been.calledWith( + sinon.match.object, 20, 30, 0x7890abcd, 0x12345678); + + sendExtendedDesktopSize(client, 1, 0, 20, 30, 0x7890abcd, 0x12345678); + RFB.messages.setDesktopSize.resetHistory(); + + clock.tick(20); + + container.style.width = '30px'; + container.style.height = '40px'; + fakeResizeObserver.fire(); expect(RFB.messages.setDesktopSize).to.not.have.been.called; + clock.tick(20); + container.style.width = '40px'; container.style.height = '50px'; fakeResizeObserver.fire(); - clock.tick(400); expect(RFB.messages.setDesktopSize).to.not.have.been.called; - clock.tick(200); + clock.tick(80); expect(RFB.messages.setDesktopSize).to.have.been.calledOnce; expect(RFB.messages.setDesktopSize).to.have.been.calledWith( sinon.match.object, 40, 50, 0x7890abcd, 0x12345678); }); + it('should not have overlapping resize requests', function () { + container.style.width = '40px'; + container.style.height = '50px'; + fakeResizeObserver.fire(); + + expect(RFB.messages.setDesktopSize).to.have.been.calledOnce; + + RFB.messages.setDesktopSize.resetHistory(); + + clock.tick(1000); + container.style.width = '20px'; + container.style.height = '30px'; + fakeResizeObserver.fire(); + + expect(RFB.messages.setDesktopSize).to.not.have.been.called; + }); + + it('should finalize any pending resizes', function () { + container.style.width = '40px'; + container.style.height = '50px'; + fakeResizeObserver.fire(); + + expect(RFB.messages.setDesktopSize).to.have.been.calledOnce; + + RFB.messages.setDesktopSize.resetHistory(); + + clock.tick(1000); + container.style.width = '20px'; + container.style.height = '30px'; + fakeResizeObserver.fire(); + + expect(RFB.messages.setDesktopSize).to.not.have.been.called; + + // Server responds with the requested size 40x50 + sendExtendedDesktopSize(client, 1, 0, 40, 50, 0x7890abcd, 0x12345678); + + expect(RFB.messages.setDesktopSize).to.have.been.calledOnce; + expect(RFB.messages.setDesktopSize).to.have.been.calledWith( + sinon.match.object, 20, 30, 0x7890abcd, 0x12345678); + }); + + it('should not finalize any pending resize if not needed', function () { + container.style.width = '40px'; + container.style.height = '50px'; + fakeResizeObserver.fire(); + + expect(RFB.messages.setDesktopSize).to.have.been.calledOnce; + + RFB.messages.setDesktopSize.resetHistory(); + + // Server responds with the requested size 40x50 + sendExtendedDesktopSize(client, 1, 0, 40, 50, 0x7890abcd, 0x12345678); + + expect(RFB.messages.setDesktopSize).to.not.have.been.called; + }); + + it('should not finalize any pending resizes on errors', function () { + container.style.width = '40px'; + container.style.height = '50px'; + fakeResizeObserver.fire(); + + expect(RFB.messages.setDesktopSize).to.have.been.calledOnce; + + RFB.messages.setDesktopSize.resetHistory(); + + clock.tick(1000); + container.style.width = '20px'; + container.style.height = '30px'; + fakeResizeObserver.fire(); + + expect(RFB.messages.setDesktopSize).to.not.have.been.called; + + // Server failed the requested size 40x50 + sendExtendedDesktopSize(client, 1, 1, 40, 50, 0x7890abcd, 0x12345678); + + expect(RFB.messages.setDesktopSize).to.not.have.been.called; + }); + it('should not resize when resize is disabled', function () { client._resizeSession = false; container.style.width = '40px'; container.style.height = '50px'; fakeResizeObserver.fire(); - clock.tick(1000); expect(RFB.messages.setDesktopSize).to.not.have.been.called; }); @@ -1224,7 +1308,6 @@ describe('Remote Frame Buffer protocol client', function () { container.style.width = '40px'; container.style.height = '50px'; fakeResizeObserver.fire(); - clock.tick(1000); expect(RFB.messages.setDesktopSize).to.not.have.been.called; }); @@ -1235,7 +1318,6 @@ describe('Remote Frame Buffer protocol client', function () { container.style.width = '40px'; container.style.height = '50px'; fakeResizeObserver.fire(); - clock.tick(1000); expect(RFB.messages.setDesktopSize).to.not.have.been.called; }); @@ -1248,7 +1330,6 @@ describe('Remote Frame Buffer protocol client', function () { sendExtendedDesktopSize(client, 0, 0, 100, 100, 0xabababab, 0x11223344); // The scrollbars cause the ResizeObserver to fire fakeResizeObserver.fire(); - clock.tick(1000); expect(RFB.messages.setDesktopSize).to.not.have.been.called; @@ -1256,7 +1337,6 @@ describe('Remote Frame Buffer protocol client', function () { 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).to.have.been.calledWith(