Skip to content

Commit c73cb32

Browse files
CendioOssmanCendioOssman
committed
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.
1 parent c821783 commit c73cb32

File tree

2 files changed

+141
-31
lines changed

2 files changed

+141
-31
lines changed

‎core/rfb.js‎

Lines changed: 43 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,8 @@ export default class RFB extends EventTargetMixin {
149149
this._supportsSetDesktopSize = false;
150150
this._screenID = 0;
151151
this._screenFlags = 0;
152+
this._pendingRemoteResize = false;
153+
this._lastResize = 0;
152154

153155
this._qemuExtKeyEventSupported = false;
154156

@@ -736,15 +738,9 @@ export default class RFB extends EventTargetMixin {
736738
this._saveExpectedClientSize();
737739
});
738740

739-
if (this._resizeSession) {
740-
// Request changing the resolution of the remote display to
741-
// the size of the local browser viewport.
742-
743-
// In order to not send multiple requests before the browser-resize
744-
// is finished we wait 0.5 seconds before sending the request.
745-
clearTimeout(this._resizeTimeout);
746-
this._resizeTimeout = setTimeout(this._requestRemoteResize.bind(this), 500);
747-
}
741+
// Request changing the resolution of the remote display to
742+
// the size of the local browser viewport.
743+
this._requestRemoteResize();
748744
}
749745

750746
// Update state of clipping in Display object, and make sure the
@@ -794,16 +790,40 @@ export default class RFB extends EventTargetMixin {
794790
// Requests a change of remote desktop size. This message is an extension
795791
// and may only be sent if we have received an ExtendedDesktopSize message
796792
_requestRemoteResize() {
797-
clearTimeout(this._resizeTimeout);
798-
this._resizeTimeout = null;
799793

800-
if (!this._resizeSession || this._viewOnly ||
801-
!this._supportsSetDesktopSize) {
794+
if (!this._resizeSession) {
795+
return;
796+
}
797+
if (this._viewOnly) {
798+
return;
799+
}
800+
if (!this._supportsSetDesktopSize) {
801+
return;
802+
}
803+
804+
// Rate limit to one pending resize at a time
805+
if (this._pendingRemoteResize) {
802806
return;
803807
}
804808

809+
// And no more than once every 100ms
810+
if ((Date.now() - this._lastResize) < 100) {
811+
clearTimeout(this._resizeTimeout);
812+
this._resizeTimeout = setTimeout(this._requestRemoteResize.bind(this),
813+
100 - (Date.now() - this._lastResize));
814+
return;
815+
}
816+
this._resizeTimeout = null;
817+
805818
const size = this._screenSize();
806819

820+
// Do we actually change anything?
821+
if (size.w === this._fbWidth && size.h === this._fbHeight) {
822+
return;
823+
}
824+
825+
this._pendingRemoteResize = true;
826+
this._lastResize = Date.now();
807827
RFB.messages.setDesktopSize(this._sock,
808828
Math.floor(size.w), Math.floor(size.h),
809829
this._screenID, this._screenFlags);
@@ -2913,6 +2933,10 @@ export default class RFB extends EventTargetMixin {
29132933
* 2 - another client requested the resize
29142934
*/
29152935

2936+
if (this._FBU.x === 1) {
2937+
this._pendingRemoteResize = false;
2938+
}
2939+
29162940
// We need to handle errors when we requested the resize.
29172941
if (this._FBU.x === 1 && this._FBU.y !== 0) {
29182942
let msg = "";
@@ -2945,6 +2969,12 @@ export default class RFB extends EventTargetMixin {
29452969
this._requestRemoteResize();
29462970
}
29472971

2972+
if (this._FBU.x === 1 && this._FBU.y === 0) {
2973+
// We might have resized again whilst waiting for the
2974+
// previous request, so check if we are in sync
2975+
this._requestRemoteResize();
2976+
}
2977+
29482978
return true;
29492979
}
29502980

‎tests/test.rfb.js‎

Lines changed: 98 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -680,7 +680,6 @@ describe('Remote Frame Buffer protocol client', function () {
680680
// The resize will cause scrollbars on the container, this causes a
681681
// resize observation in the browsers
682682
fakeResizeObserver.fire();
683-
clock.tick(1000);
684683

685684
// FIXME: Display implicitly calls viewportChangeSize() when
686685
// resizing the framebuffer, hence calledTwice.
@@ -1042,7 +1041,6 @@ describe('Remote Frame Buffer protocol client', function () {
10421041
// The resize will cause scrollbars on the container, this causes a
10431042
// resize observation in the browsers
10441043
fakeResizeObserver.fire();
1045-
clock.tick(1000);
10461044

10471045
expect(client._display.autoscale).to.have.been.calledOnce;
10481046
expect(client._display.autoscale).to.have.been.calledWith(70, 80);
@@ -1079,6 +1077,7 @@ describe('Remote Frame Buffer protocol client', function () {
10791077
let height = RFB.messages.setDesktopSize.args[0][2];
10801078
sendExtendedDesktopSize(client, 1, 0, width, height, 0x7890abcd, 0x12345678);
10811079
RFB.messages.setDesktopSize.resetHistory();
1080+
clock.tick(10000);
10821081
}
10831082
});
10841083

@@ -1093,7 +1092,6 @@ describe('Remote Frame Buffer protocol client', function () {
10931092
container.style.width = '40px';
10941093
container.style.height = '50px';
10951094
fakeResizeObserver.fire();
1096-
clock.tick(1000);
10971095
expect(RFB.messages.setDesktopSize).to.not.have.been.called;
10981096

10991097
client.resizeSession = true;
@@ -1132,7 +1130,6 @@ describe('Remote Frame Buffer protocol client', function () {
11321130
container.style.width = '40px';
11331131
container.style.height = '50px';
11341132
fakeResizeObserver.fire();
1135-
clock.tick(1000);
11361133

11371134
expect(RFB.messages.setDesktopSize).to.have.been.calledOnce;
11381135
expect(RFB.messages.setDesktopSize).to.have.been.calledWith(
@@ -1143,21 +1140,19 @@ describe('Remote Frame Buffer protocol client', function () {
11431140
container.style.width = '40px';
11441141
container.style.height = '50px';
11451142
fakeResizeObserver.fire();
1146-
clock.tick(1000);
11471143

11481144
expect(RFB.messages.setDesktopSize).to.have.been.calledOnce;
11491145
expect(RFB.messages.setDesktopSize).to.have.been.calledWith(
11501146
sinon.match.object, 40, 50, 0x7890abcd, 0x12345678);
11511147

11521148
// Server responds with the requested size 40x50
11531149
sendExtendedDesktopSize(client, 1, 0, 40, 50, 0x7890abcd, 0x12345678);
1154-
clock.tick(1000);
11551150

11561151
RFB.messages.setDesktopSize.resetHistory();
11571152

11581153
// size is still 40x50
1159-
fakeResizeObserver.fire();
11601154
clock.tick(1000);
1155+
fakeResizeObserver.fire();
11611156

11621157
expect(RFB.messages.setDesktopSize).to.not.have.been.called;
11631158
});
@@ -1166,7 +1161,6 @@ describe('Remote Frame Buffer protocol client', function () {
11661161
container.style.width = '40px';
11671162
container.style.height = '50px';
11681163
fakeResizeObserver.fire();
1169-
clock.tick(1000);
11701164

11711165
expect(RFB.messages.setDesktopSize).to.have.been.calledOnce;
11721166
expect(RFB.messages.setDesktopSize).to.have.been.calledWith(
@@ -1175,45 +1169,135 @@ describe('Remote Frame Buffer protocol client', function () {
11751169
sendExtendedDesktopSize(client, 1, 0, 40, 50, 0x7890abcd, 0x12345678);
11761170
RFB.messages.setDesktopSize.resetHistory();
11771171

1172+
clock.tick(1000);
11781173
container.style.width = '70px';
11791174
container.style.height = '80px';
11801175
fakeResizeObserver.fire();
1181-
clock.tick(1000);
11821176

11831177
expect(RFB.messages.setDesktopSize).to.have.been.calledOnce;
11841178
expect(RFB.messages.setDesktopSize).to.have.been.calledWith(
11851179
sinon.match.object, 70, 80, 0x7890abcd, 0x12345678);
11861180
});
11871181

1188-
it('should not resize until the container size is stable', function () {
1182+
it('should rate limit resizes', function () {
11891183
container.style.width = '20px';
11901184
container.style.height = '30px';
11911185
fakeResizeObserver.fire();
1192-
clock.tick(400);
1186+
1187+
expect(RFB.messages.setDesktopSize).to.have.been.calledOnce;
1188+
expect(RFB.messages.setDesktopSize).to.have.been.calledWith(
1189+
sinon.match.object, 20, 30, 0x7890abcd, 0x12345678);
1190+
1191+
sendExtendedDesktopSize(client, 1, 0, 20, 30, 0x7890abcd, 0x12345678);
1192+
RFB.messages.setDesktopSize.resetHistory();
1193+
1194+
clock.tick(20);
1195+
1196+
container.style.width = '30px';
1197+
container.style.height = '40px';
1198+
fakeResizeObserver.fire();
11931199

11941200
expect(RFB.messages.setDesktopSize).to.not.have.been.called;
11951201

1202+
clock.tick(20);
1203+
11961204
container.style.width = '40px';
11971205
container.style.height = '50px';
11981206
fakeResizeObserver.fire();
1199-
clock.tick(400);
12001207

12011208
expect(RFB.messages.setDesktopSize).to.not.have.been.called;
12021209

1203-
clock.tick(200);
1210+
clock.tick(80);
12041211

12051212
expect(RFB.messages.setDesktopSize).to.have.been.calledOnce;
12061213
expect(RFB.messages.setDesktopSize).to.have.been.calledWith(
12071214
sinon.match.object, 40, 50, 0x7890abcd, 0x12345678);
12081215
});
12091216

1217+
it('should not have overlapping resize requests', function () {
1218+
container.style.width = '40px';
1219+
container.style.height = '50px';
1220+
fakeResizeObserver.fire();
1221+
1222+
expect(RFB.messages.setDesktopSize).to.have.been.calledOnce;
1223+
1224+
RFB.messages.setDesktopSize.resetHistory();
1225+
1226+
clock.tick(1000);
1227+
container.style.width = '20px';
1228+
container.style.height = '30px';
1229+
fakeResizeObserver.fire();
1230+
1231+
expect(RFB.messages.setDesktopSize).to.not.have.been.called;
1232+
});
1233+
1234+
it('should finalize any pending resizes', function () {
1235+
container.style.width = '40px';
1236+
container.style.height = '50px';
1237+
fakeResizeObserver.fire();
1238+
1239+
expect(RFB.messages.setDesktopSize).to.have.been.calledOnce;
1240+
1241+
RFB.messages.setDesktopSize.resetHistory();
1242+
1243+
clock.tick(1000);
1244+
container.style.width = '20px';
1245+
container.style.height = '30px';
1246+
fakeResizeObserver.fire();
1247+
1248+
expect(RFB.messages.setDesktopSize).to.not.have.been.called;
1249+
1250+
// Server responds with the requested size 40x50
1251+
sendExtendedDesktopSize(client, 1, 0, 40, 50, 0x7890abcd, 0x12345678);
1252+
1253+
expect(RFB.messages.setDesktopSize).to.have.been.calledOnce;
1254+
expect(RFB.messages.setDesktopSize).to.have.been.calledWith(
1255+
sinon.match.object, 20, 30, 0x7890abcd, 0x12345678);
1256+
});
1257+
1258+
it('should not finalize any pending resize if not needed', function () {
1259+
container.style.width = '40px';
1260+
container.style.height = '50px';
1261+
fakeResizeObserver.fire();
1262+
1263+
expect(RFB.messages.setDesktopSize).to.have.been.calledOnce;
1264+
1265+
RFB.messages.setDesktopSize.resetHistory();
1266+
1267+
// Server responds with the requested size 40x50
1268+
sendExtendedDesktopSize(client, 1, 0, 40, 50, 0x7890abcd, 0x12345678);
1269+
1270+
expect(RFB.messages.setDesktopSize).to.not.have.been.called;
1271+
});
1272+
1273+
it('should not finalize any pending resizes on errors', function () {
1274+
container.style.width = '40px';
1275+
container.style.height = '50px';
1276+
fakeResizeObserver.fire();
1277+
1278+
expect(RFB.messages.setDesktopSize).to.have.been.calledOnce;
1279+
1280+
RFB.messages.setDesktopSize.resetHistory();
1281+
1282+
clock.tick(1000);
1283+
container.style.width = '20px';
1284+
container.style.height = '30px';
1285+
fakeResizeObserver.fire();
1286+
1287+
expect(RFB.messages.setDesktopSize).to.not.have.been.called;
1288+
1289+
// Server failed the requested size 40x50
1290+
sendExtendedDesktopSize(client, 1, 1, 40, 50, 0x7890abcd, 0x12345678);
1291+
1292+
expect(RFB.messages.setDesktopSize).to.not.have.been.called;
1293+
});
1294+
12101295
it('should not resize when resize is disabled', function () {
12111296
client._resizeSession = false;
12121297

12131298
container.style.width = '40px';
12141299
container.style.height = '50px';
12151300
fakeResizeObserver.fire();
1216-
clock.tick(1000);
12171301

12181302
expect(RFB.messages.setDesktopSize).to.not.have.been.called;
12191303
});
@@ -1224,7 +1308,6 @@ describe('Remote Frame Buffer protocol client', function () {
12241308
container.style.width = '40px';
12251309
container.style.height = '50px';
12261310
fakeResizeObserver.fire();
1227-
clock.tick(1000);
12281311

12291312
expect(RFB.messages.setDesktopSize).to.not.have.been.called;
12301313
});
@@ -1235,7 +1318,6 @@ describe('Remote Frame Buffer protocol client', function () {
12351318
container.style.width = '40px';
12361319
container.style.height = '50px';
12371320
fakeResizeObserver.fire();
1238-
clock.tick(1000);
12391321

12401322
expect(RFB.messages.setDesktopSize).to.not.have.been.called;
12411323
});
@@ -1248,15 +1330,13 @@ describe('Remote Frame Buffer protocol client', function () {
12481330
sendExtendedDesktopSize(client, 0, 0, 100, 100, 0xabababab, 0x11223344);
12491331
// The scrollbars cause the ResizeObserver to fire
12501332
fakeResizeObserver.fire();
1251-
clock.tick(1000);
12521333

12531334
expect(RFB.messages.setDesktopSize).to.not.have.been.called;
12541335

12551336
// An actual size change must not be ignored afterwards
12561337
container.style.width = '120px';
12571338
container.style.height = '130px';
12581339
fakeResizeObserver.fire();
1259-
clock.tick(1000);
12601340

12611341
expect(RFB.messages.setDesktopSize).to.have.been.calledOnce;
12621342
expect(RFB.messages.setDesktopSize).to.have.been.calledWith(

0 commit comments

Comments
 (0)